UI: Fix Undo/Redo holding source references

Holding references to sources is never a good idea. Instead, save and
restore the scene and its subsources on removal.

Co-authored-by: Jim <[email protected]>

Closes obsproject/obs-studio#4462

UI/window-basic-main.cpp

@@ -19,6 +19,7 @@
 
 #include <cstddef>
 #include <ctime>
+#include <functional>
 #include <obs-data.h>
 #include <obs.h>
 #include <obs.hpp>
@@ -3674,6 +3675,17 @@ void OBSBasic::DuplicateSelectedScene()
 	}
 }
 
+static void save_undo_source_enum(obs_source_t *, obs_source_t *source, void *p)
+{
+	if (obs_obj_is_private(source) && !obs_source_removed(source))
+		return;
+
+	obs_data_array_t *array = (obs_data_array_t *)p;
+	obs_data_t *source_data = obs_save_source(source);
+	obs_data_array_push_back(array, source_data);
+	obs_data_release(source_data);
+}
+
 void OBSBasic::RemoveSelectedScene()
 {
 	OBSScene scene = GetCurrentScene();
@@ -3681,104 +3693,93 @@ void OBSBasic::RemoveSelectedScene()
 
 	OBSSource curProgramScene = OBSGetStrongRef(programScene);
 
-	if (source && QueryRemoveSource(source)) {
-		vector<std::string> item_ids;
-		obs_data_t *wrapper = obs_save_source(source);
-		obs_data_array_t *arr = obs_data_array_create();
-		struct wrap {
-			obs_data_array_t *arr;
-			vector<std::string> &items;
-		};
+	if (!source || !QueryRemoveSource(source)) {
+		return;
+	}
 
-		wrap passthrough = {arr, item_ids};
-		obs_scene_enum_items(
-			scene,
-			[](obs_scene_t *, obs_sceneitem_t *item,
-			   void *vp_wrap) {
-				wrap *passthrough = (wrap *)vp_wrap;
-				passthrough->items.push_back(obs_source_get_name(
-					obs_sceneitem_get_source(item)));
-				obs_data_array_t *arr = passthrough->arr;
-				obs_sceneitem_save(item, arr);
-				obs_source_addref(
-					obs_sceneitem_get_source(item));
-				return true;
-			},
-			(void *)&passthrough);
-		obs_data_array_t *list_order = SaveSceneListOrder();
-		obs_data_set_array(wrapper, "arr", arr);
-		obs_data_set_array(wrapper, "list_order", list_order);
-		obs_data_set_string(wrapper, "name",
-				    obs_source_get_name(source));
-
-		auto d = [item_ids](bool remove_ref) {
-			for (auto &item : item_ids) {
-				obs_source_t *source =
-					obs_get_source_by_name(item.c_str());
-				blog(LOG_INFO, "%s", item.c_str());
-				if (remove_ref) {
-					obs_source_release(source);
-					obs_source_release(source);
-				}
-			}
-		};
+	/* ------------------------------ */
+	/* save all sources in scene tree */
 
-		auto undo = [this, d](const std::string &data) {
-			obs_data_t *dat =
-				obs_data_create_from_json(data.c_str());
-			obs_source_release(obs_load_source(dat));
-			obs_data_array_t *arr = obs_data_get_array(dat, "arr");
-			obs_data_array_t *list_order =
-				obs_data_get_array(dat, "list_order");
-			const char *sname = obs_data_get_string(dat, "name");
-			obs_source_t *source = obs_get_source_by_name(sname);
-			obs_scene_t *scene = obs_scene_from_source(source);
+	obs_data_array_t *array = obs_data_array_create();
 
-			obs_sceneitems_add(scene, arr);
-			LoadSceneListOrder(list_order);
-			SetCurrentScene(source);
+	obs_source_enum_full_tree(source, save_undo_source_enum, array);
 
-			obs_data_release(dat);
-			obs_data_array_release(arr);
-			obs_data_array_release(list_order);
-			obs_source_release(source);
+	obs_data_t *scene_data = obs_save_source(source);
+	obs_data_array_push_back(array, scene_data);
+	obs_data_release(scene_data);
 
-			d(true);
-		};
-		obs_data_t *rwrapper = obs_data_create();
-		obs_data_set_string(rwrapper, "name",
-				    obs_source_get_name(source));
-		auto redo = [d](const std::string &data) {
-			obs_data_t *dat =
-				obs_data_create_from_json(data.c_str());
-			obs_source_t *source = obs_get_source_by_name(
-				obs_data_get_string(dat, "name"));
-			obs_source_remove(source);
-			obs_source_release(source);
-			obs_data_release(dat);
+	/* --------------------------- */
+	/* undo/redo                   */
 
-			d(false);
-		};
+	auto undo = [this](const std::string &json) {
+		obs_data_t *base = obs_data_create_from_json(json.c_str());
+		obs_data_array_t *array = obs_data_get_array(base, "array");
+		int savedIndex = (int)obs_data_get_int(base, "index");
+		std::vector<obs_source_t *> sources;
 
-		std::string undo_data = obs_data_get_json(wrapper);
-		std::string redo_data = obs_data_get_json(wrapper);
-		undo_s.add_action(
-			QTStr("Undo.Delete").arg(obs_source_get_name(source)),
-			undo, redo, undo_data, redo_data, [d](bool undo) {
-				if (undo) {
-					d(true);
-				}
-			});
+		/* create missing sources */
+		size_t count = obs_data_array_count(array);
+		sources.reserve(count);
+
+		for (size_t i = 0; i < count; i++) {
+			obs_data_t *data = obs_data_array_item(array, i);
+			const char *name = obs_data_get_string(data, "name");
 
+			obs_source_t *source = obs_get_source_by_name(name);
+			if (!source)
+				source = obs_load_source(data);
+			sources.push_back(source);
+
+			obs_data_release(data);
+		}
+
+		/* actually load sources now */
+		for (obs_source_t *source : sources)
+			obs_source_load2(source);
+
+		obs_source_t *scene_source = sources.back();
+		OBSScene scene = obs_scene_from_source(scene_source);
+		SetCurrentScene(scene);
+
+		/* set original index in list box */
+		ui->scenes->blockSignals(true);
+		int curIndex = ui->scenes->currentRow();
+		QListWidgetItem *item = ui->scenes->takeItem(curIndex);
+		ui->scenes->insertItem(savedIndex, item);
+		ui->scenes->setCurrentRow(savedIndex);
+		ui->scenes->blockSignals(false);
+
+		/* release sources */
+		for (obs_source_t *source : sources)
+			obs_source_release(source);
+
+		obs_data_array_release(array);
+		obs_data_release(base);
+	};
+
+	auto redo = [](const std::string &name) {
+		obs_source_t *source = obs_get_source_by_name(name.c_str());
 		obs_source_remove(source);
-		obs_data_release(wrapper);
-		obs_data_release(rwrapper);
-		obs_data_array_release(arr);
-		obs_data_array_release(list_order);
+		obs_source_release(source);
+	};
 
-		if (api)
-			api->on_event(OBS_FRONTEND_EVENT_SCENE_LIST_CHANGED);
-	}
+	obs_data_t *data = obs_data_create();
+	obs_data_set_array(data, "array", array);
+	obs_data_set_int(data, "index", ui->scenes->currentRow());
+
+	undo_s.add_action("Delete Scene", undo, redo, obs_data_get_json(data),
+			  obs_source_get_name(source), NULL);
+
+	obs_data_array_release(array);
+	obs_data_release(data);
+
+	/* --------------------------- */
+	/* remove                      */
+
+	obs_source_remove(source);
+
+	if (api)
+		api->on_event(OBS_FRONTEND_EVENT_SCENE_LIST_CHANGED);
 }
 
 void OBSBasic::RemoveSelectedSceneItem()
@@ -5496,12 +5497,17 @@ static bool remove_items(obs_scene_t *, obs_sceneitem_t *item, void *param)
 void OBSBasic::on_actionRemoveSource_triggered()
 {
 	vector<OBSSceneItem> items;
+	OBSScene scene = GetCurrentScene();
+	obs_source_t *scene_source = obs_scene_get_source(scene);
 
-	obs_scene_enum_items(GetCurrentScene(), remove_items, &items);
+	obs_scene_enum_items(scene, remove_items, &items);
 
 	if (!items.size())
 		return;
 
+	/* ------------------------------------- */
+	/* confirm action with user              */
+
 	bool confirmed = false;
 
 	if (items.size() > 1) {
@@ -5528,129 +5534,116 @@ void OBSBasic::on_actionRemoveSource_triggered()
 	if (!confirmed)
 		return;
 
-	struct source_save {
-		std::string name;
-		std::string scene_name;
-		int pos;
-		bool in_group = false;
-		int64_t group_id;
-	};
-	vector<source_save> item_save;
+	/* ----------------------------------------------- */
+	/* save undo data                                  */
 
-	obs_data_t *wrapper = obs_data_create();
-	obs_data_array_t *data = obs_data_array_create();
-	for (const auto &item : items) {
-		obs_sceneitem_save(item, data);
-		obs_source_t *source = obs_sceneitem_get_source(item);
-		obs_source_addref(source);
-		obs_source_set_hidden(source, true);
+	obs_data_array_t *undo_array = obs_data_array_create();
 
-		obs_sceneitem_t *grp =
-			obs_sceneitem_get_group(GetCurrentScene(), item);
-		obs_scene_t *scene = obs_sceneitem_get_scene(item);
-		source_save save = {
-			obs_source_get_name(source),
-			obs_source_get_name(obs_scene_get_source(scene)),
-			obs_sceneitem_get_order_position(item),
-			grp ? true : false, obs_sceneitem_get_id(grp)};
+	obs_source_enum_full_tree(scene_source, save_undo_source_enum,
+				  undo_array);
 
-		item_save.push_back(save);
-	}
+	obs_data_t *scene_data = obs_save_source(scene_source);
+	obs_data_array_push_back(undo_array, scene_data);
+	obs_data_release(scene_data);
 
-	obs_scene_t *scene = GetCurrentScene();
-	const char *name = obs_source_get_name(obs_scene_get_source(scene));
-	obs_data_set_array(wrapper, "data_array", data);
-	obs_data_set_string(wrapper, "name", name);
-	std::string undo_data(obs_data_get_json(wrapper));
+	obs_data_t *undo_data = obs_data_create();
+	obs_data_set_array(undo_data, "array", undo_array);
+	obs_data_array_release(undo_array);
 
-	auto undo_fn = [this, item_save](const std::string &data) {
-		obs_data_t *dat = obs_data_create_from_json(data.c_str());
-		obs_data_array_t *sources_data =
-			obs_data_get_array(dat, "data_array");
-		const char *name = obs_data_get_string(dat, "name");
-		obs_source_t *src = obs_get_source_by_name(name);
-		obs_scene_t *scene = obs_scene_from_source(src);
+	/* because undo_array and redo_array will share settings data,
+	 * generate json here */
+	const char *undo_json = obs_data_get_json(undo_data);
 
-		obs_sceneitems_add(scene, sources_data);
-		SetCurrentScene(scene);
+	/* ----------------------------------------------- */
+	/* remove items                                    */
 
-		for (const auto &save : item_save) {
-			obs_source_t *source =
-				obs_get_source_by_name(save.name.c_str());
-			obs_source_set_hidden(source, false);
-			if (save.in_group) {
-				obs_sceneitem_t *grp =
-					obs_scene_find_sceneitem_by_id(
-						scene, save.group_id);
-				obs_sceneitem_t *item =
-					obs_scene_sceneitem_from_source(scene,
-									source);
-				obs_sceneitem_group_add_item(grp, item);
-				obs_sceneitem_set_order_position(item,
-								 save.pos);
-
-				obs_sceneitem_release(item);
-			}
+	for (auto &item : items)
+		obs_sceneitem_remove(item);
 
-			obs_source_release(source);
-			obs_source_release(source);
-		}
+	/* ----------------------------------------------- */
+	/* save redo data                                  */
 
-		obs_source_release(src);
-		obs_data_array_release(sources_data);
-		obs_data_release(dat);
-	};
+	obs_data_array_t *redo_array = obs_data_array_create();
 
-	auto redo_fn = [item_save](const std::string &) {
-		for (const auto &save : item_save) {
-			obs_source_t *source =
-				obs_get_source_by_name(save.name.c_str());
-			obs_source_t *scene_source =
-				obs_get_source_by_name(save.scene_name.c_str());
-			obs_scene_t *scene =
-				obs_scene_from_source(scene_source);
-			if (!scene)
-				scene = obs_group_from_source(scene_source);
-
-			obs_sceneitem_t *item =
-				obs_scene_sceneitem_from_source(scene, source);
-			obs_sceneitem_remove(item);
-			obs_source_set_hidden(source, true);
+	obs_source_enum_full_tree(scene_source, save_undo_source_enum,
+				  redo_array);
+
+	scene_data = obs_save_source(scene_source);
+	obs_data_array_push_back(redo_array, scene_data);
+	obs_data_release(scene_data);
+
+	obs_data_t *redo_data = obs_data_create();
+	obs_data_set_array(redo_data, "array", redo_array);
+	obs_data_array_release(redo_array);
 
-			obs_sceneitem_release(item);
-			obs_source_release(scene_source);
-			/*  usually want to release source, but redo needs to add a reference to keep alive */
+	const char *redo_json = obs_data_get_json(redo_data);
+
+	/* ----------------------------------------------- */
+	/* undo/redo callback                              */
+
+	auto undo_redo = [this](const std::string &json) {
+		obs_data_t *base = obs_data_create_from_json(json.c_str());
+		obs_data_array_t *array = obs_data_get_array(base, "array");
+		std::vector<obs_source_t *> sources;
+
+		/* create missing sources */
+		size_t count = obs_data_array_count(array);
+		sources.reserve(count);
+
+		for (size_t i = 0; i < count; i++) {
+			obs_data_t *data = obs_data_array_item(array, i);
+			const char *name = obs_data_get_string(data, "name");
+
+			obs_source_t *source = obs_get_source_by_name(name);
+			if (!source)
+				source = obs_load_source(data);
+			sources.push_back(source);
+
+			/* update scene/group settings to restore their
+			 * contents to their saved settings */
+			if (obs_source_is_group(source) ||
+			    obs_source_is_scene(source)) {
+				obs_data_t *scene_settings =
+					obs_data_get_obj(data, "settings");
+				obs_source_update(source, scene_settings);
+				obs_data_release(scene_settings);
+			}
+
+			obs_data_release(data);
 		}
-	};
 
-	auto d = [item_save](bool is_undo) {
-		if (!is_undo)
-			return;
+		/* actually load sources now */
+		for (obs_source_t *source : sources)
+			obs_source_load2(source);
 
-		for (const auto &item : item_save) {
-			obs_source_t *source =
-				obs_get_source_by_name(item.name.c_str());
-			obs_source_release(source);
+		/* release sources */
+		for (obs_source_t *source : sources)
 			obs_source_release(source);
-		}
+
+		obs_data_array_release(array);
+		obs_data_release(base);
+
+		ui->sources->RefreshItems();
 	};
 
+	/* ----------------------------------------------- */
+	/* undo/redo                                       */
+
 	QString action_name;
-	if (items.size() > 1)
+	if (items.size() > 1) {
 		action_name = QTStr("Undo.Sources.Multi")
 				      .arg(QString::number(items.size()));
-	else
-		action_name =
-			QTStr("Undo.Delete")
-				.arg(QString(obs_source_get_name(
-					obs_sceneitem_get_source(items[0]))));
-	undo_s.add_action(action_name, undo_fn, redo_fn, undo_data, "", d);
+	} else {
+		QString str = QTStr("Undo.Delete");
+		action_name = str.arg(obs_source_get_name(
+			obs_sceneitem_get_source(items[0])));
+	}
 
-	obs_data_array_release(data);
-	obs_data_release(wrapper);
+	undo_s.add_action(action_name, undo_redo, undo_redo, undo_json,
+			  redo_json, nullptr);
 
-	for (auto &item : items)
-		obs_sceneitem_remove(item);
+	obs_data_release(undo_data);
+	obs_data_release(redo_data);
 }
 
 void OBSBasic::on_actionInteract_triggered()