Various fixes for OpenXR action map meta data and editing

1 files changed, 142 insertions, 56 deletions

diff --git a/modules/openxr/editor/openxr_action_map_editor.cpp b/modules/openxr/editor/openxr_action_map_editor.cpp
index b5223e5903..a2c33a91c4 100644
--- a/modules/openxr/editor/openxr_action_map_editor.cpp
+++ b/modules/openxr/editor/openxr_action_map_editor.cpp
@@ -40,14 +40,16 @@
 
 void OpenXRActionMapEditor::_bind_methods() {
 	ClassDB::bind_method("_add_action_set_editor", &OpenXRActionMapEditor::_add_action_set_editor);
-	ClassDB::bind_method("_update_action_sets", &OpenXRActionMapEditor::_update_action_sets);
-
 	ClassDB::bind_method("_add_interaction_profile_editor", &OpenXRActionMapEditor::_add_interaction_profile_editor);
-	ClassDB::bind_method("_update_interaction_profiles", &OpenXRActionMapEditor::_update_interaction_profiles);
 
 	ClassDB::bind_method(D_METHOD("_add_action_set", "name"), &OpenXRActionMapEditor::_add_action_set);
 	ClassDB::bind_method(D_METHOD("_set_focus_on_action_set", "action_set"), &OpenXRActionMapEditor::_set_focus_on_action_set);
 	ClassDB::bind_method(D_METHOD("_remove_action_set", "name"), &OpenXRActionMapEditor::_remove_action_set);
+
+	ClassDB::bind_method(D_METHOD("_do_add_action_set_editor", "action_set_editor"), &OpenXRActionMapEditor::_do_add_action_set_editor);
+	ClassDB::bind_method(D_METHOD("_do_remove_action_set_editor", "action_set_editor"), &OpenXRActionMapEditor::_do_remove_action_set_editor);
+	ClassDB::bind_method(D_METHOD("_do_add_interaction_profile_editor", "interaction_profile_editor"), &OpenXRActionMapEditor::_do_add_interaction_profile_editor);
+	ClassDB::bind_method(D_METHOD("_do_remove_interaction_profile_editor", "interaction_profile_editor"), &OpenXRActionMapEditor::_do_remove_interaction_profile_editor);
 }
 
 void OpenXRActionMapEditor::_notification(int p_what) {
@@ -63,8 +65,8 @@ void OpenXRActionMapEditor::_notification(int p_what) {
 		} break;
 
 		case NOTIFICATION_READY: {
-			_update_action_sets();
-			_update_interaction_profiles();
+			_create_action_sets();
+			_create_interaction_profiles();
 		} break;
 	}
 }
@@ -75,18 +77,13 @@ OpenXRActionSetEditor *OpenXRActionMapEditor::_add_action_set_editor(Ref<OpenXRA
 	OpenXRActionSetEditor *action_set_editor = memnew(OpenXRActionSetEditor(action_map, p_action_set));
 	action_set_editor->connect("remove", callable_mp(this, &OpenXRActionMapEditor::_on_remove_action_set));
 	action_set_editor->connect("action_removed", callable_mp(this, &OpenXRActionMapEditor::_on_action_removed));
+
 	actionsets_vb->add_child(action_set_editor);
 
 	return action_set_editor;
 }
 
-void OpenXRActionMapEditor::_update_action_sets() {
-	// out with the old...
-	while (actionsets_vb->get_child_count() > 0) {
-		memdelete(actionsets_vb->get_child(0));
-	}
-
-	// in with the new...
+void OpenXRActionMapEditor::_create_action_sets() {
 	if (action_map.is_valid()) {
 		Array action_sets = action_map->get_action_sets();
 		for (int i = 0; i < action_sets.size(); i++) {
@@ -116,22 +113,10 @@ OpenXRInteractionProfileEditorBase *OpenXRActionMapEditor::_add_interaction_prof
 	new_profile_editor->add_theme_style_override("panel", get_theme_stylebox(SNAME("panel"), SNAME("Tree")));
 	tabs->set_tab_button_icon(tabs->get_tab_count() - 1, get_theme_icon(SNAME("close"), SNAME("TabBar")));
 
-	interaction_profiles.push_back(new_profile_editor);
-
 	return new_profile_editor;
 }
 
-void OpenXRActionMapEditor::_update_interaction_profiles() {
-	// out with the old...
-	while (interaction_profiles.size() > 0) {
-		Node *interaction_profile = interaction_profiles[0];
-		interaction_profiles.remove_at(0);
-
-		tabs->remove_child(interaction_profile);
-		interaction_profile->queue_free();
-	}
-
-	// in with the new...
+void OpenXRActionMapEditor::_create_interaction_profiles() {
 	if (action_map.is_valid()) {
 		Array new_interaction_profiles = action_map->get_interaction_profiles();
 		for (int i = 0; i < new_interaction_profiles.size(); i++) {
@@ -150,9 +135,17 @@ OpenXRActionSetEditor *OpenXRActionMapEditor::_add_action_set(String p_name) {
 	new_action_set->set_name(p_name);
 	new_action_set->set_localized_name(p_name);
 	action_map->add_action_set(new_action_set);
+	action_map->set_edited(true);
 
 	// update our editor right away
-	return _add_action_set_editor(new_action_set);
+	OpenXRActionSetEditor *action_set_editor = _add_action_set_editor(new_action_set);
+
+	undo_redo->create_action(TTR("Add action set"));
+	undo_redo->add_do_method(this, "_do_add_action_set_editor", action_set_editor);
+	undo_redo->add_undo_method(this, "_do_remove_action_set_editor", action_set_editor);
+	undo_redo->commit_action(false);
+
+	return action_set_editor;
 }
 
 void OpenXRActionMapEditor::_remove_action_set(String p_name) {
@@ -160,13 +153,12 @@ void OpenXRActionMapEditor::_remove_action_set(String p_name) {
 	Ref<OpenXRActionSet> action_set = action_map->find_action_set(p_name);
 	ERR_FAIL_COND(action_set.is_null());
 
-	if (action_set->get_action_count() > 0) {
-		// we should remove these and add to our redo/undo step before calling _remove_action_set
-		WARN_PRINT("Action set still has associated actions before being removed!");
+	for (int i = 0; i < actionsets_vb->get_child_count(); i++) {
+		OpenXRActionSetEditor *action_set_editor = Object::cast_to<OpenXRActionSetEditor>(actionsets_vb->get_child(i));
+		if (action_set_editor && action_set_editor->get_action_set() == action_set) {
+			_on_remove_action_set(action_set_editor);
+		}
 	}
-
-	// now we remove it
-	action_map->remove_action_set(action_set);
 }
 
 void OpenXRActionMapEditor::_on_add_action_set() {
@@ -203,14 +195,23 @@ void OpenXRActionMapEditor::_on_remove_action_set(Object *p_action_set_editor) {
 	Ref<OpenXRActionSet> action_set = action_set_editor->get_action_set();
 	ERR_FAIL_COND(action_set.is_null());
 
-	action_map->remove_action_set(action_set);
-	actionsets_vb->remove_child(action_set_editor);
-	action_set_editor->queue_free();
+	action_set_editor->remove_all_actions();
+
+	undo_redo->create_action(TTR("Remove action set"));
+	undo_redo->add_do_method(this, "_do_remove_action_set_editor", action_set_editor);
+	undo_redo->add_undo_method(this, "_do_add_action_set_editor", action_set_editor);
+	undo_redo->commit_action(true);
+
+	action_map->set_edited(true);
 }
 
 void OpenXRActionMapEditor::_on_action_removed() {
-	// make sure our interaction profiles are updated
-	_update_interaction_profiles();
+	for (int i = 0; i < tabs->get_tab_count(); i++) {
+		// First tab won't be an interaction profile editor, but being thorough..
+		OpenXRInteractionProfileEditorBase *interaction_profile_editor = static_cast<OpenXRInteractionProfileEditorBase *>(tabs->get_tab_control(i));
+		if (interaction_profile_editor) {
+		}
+	}
 }
 
 void OpenXRActionMapEditor::_on_add_interaction_profile() {
@@ -232,20 +233,35 @@ void OpenXRActionMapEditor::_on_interaction_profile_selected(const String p_path
 	new_profile.instantiate();
 	new_profile->set_interaction_profile_path(p_path);
 	action_map->add_interaction_profile(new_profile);
+	action_map->set_edited(true);
 
-	_add_interaction_profile_editor(new_profile);
+	OpenXRInteractionProfileEditorBase *interaction_profile_editor = _add_interaction_profile_editor(new_profile);
+
+	undo_redo->create_action(TTR("Add interaction profile"));
+	undo_redo->add_do_method(this, "_do_add_interaction_profile_editor", interaction_profile_editor);
+	undo_redo->add_undo_method(this, "_do_remove_interaction_profile_editor", interaction_profile_editor);
+	undo_redo->commit_action(false);
 
 	tabs->set_current_tab(tabs->get_tab_count() - 1);
 }
 
 void OpenXRActionMapEditor::_load_action_map(const String p_path, bool p_create_new_if_missing) {
-	action_map = ResourceLoader::load(p_path, "", ResourceFormatLoader::CACHE_MODE_IGNORE);
-	if (action_map.is_null()) {
-		if (p_create_new_if_missing) {
+	Error err = OK;
+	action_map = ResourceLoader::load(p_path, "", ResourceFormatLoader::CACHE_MODE_IGNORE, &err);
+	if (err != OK) {
+		if ((err == ERR_FILE_NOT_FOUND || err == ERR_CANT_OPEN) && p_create_new_if_missing) {
 			action_map.instantiate();
 			action_map->create_default_action_sets();
+
+			// Save it immediately
+			err = ResourceSaver::save(action_map, p_path);
+			if (err != OK) {
+				// show warning but continue
+				EditorNode::get_singleton()->show_warning(vformat(TTR("Error saving file %s: %s"), edited_path, error_names[err]));
+			}
+
 		} else {
-			EditorNode::get_singleton()->show_warning(TTR("Invalid file, not an OpenXR action map."));
+			EditorNode::get_singleton()->show_warning(vformat(TTR("Error loading %s: %s."), edited_path, error_names[err]));
 
 			edited_path = "";
 			header_label->set_text("");
@@ -254,55 +270,123 @@ void OpenXRActionMapEditor::_load_action_map(const String p_path, bool p_create_
 	}
 
 	edited_path = p_path;
-	header_label->set_text(TTR("OpenXR Action map:") + " " + p_path.get_file());
+	header_label->set_text(TTR("OpenXR Action map:") + " " + edited_path.get_file());
 }
 
 void OpenXRActionMapEditor::_on_save_action_map() {
 	Error err = ResourceSaver::save(action_map, edited_path);
 	if (err != OK) {
-		EditorNode::get_singleton()->show_warning(vformat(TTR("Error saving file: %s"), edited_path));
+		EditorNode::get_singleton()->show_warning(vformat(TTR("Error saving file %s: %s"), edited_path, error_names[err]));
 		return;
 	}
 
-	_update_action_sets();
-	_update_interaction_profiles();
+	// TODO should clear undo/redo history
+
+	// out with the old
+	_clear_action_map();
+
+	_create_action_sets();
+	_create_interaction_profiles();
 }
 
 void OpenXRActionMapEditor::_on_reset_to_default_layout() {
+	// TODO should clear undo/redo history
+
+	// out with the old
+	_clear_action_map();
+
 	// create a new one
 	action_map.unref();
 	action_map.instantiate();
 	action_map->create_default_action_sets();
+	action_map->set_edited(true);
 
-	_update_action_sets();
-	_update_interaction_profiles();
+	_create_action_sets();
+	_create_interaction_profiles();
 }
 
 void OpenXRActionMapEditor::_on_tabs_tab_changed(int p_tab) {
 }
 
 void OpenXRActionMapEditor::_on_tab_button_pressed(int p_tab) {
-	OpenXRInteractionProfileEditorBase *profile_editor = static_cast<OpenXRInteractionProfileEditorBase *>(tabs->get_tab_control(p_tab));
-	ERR_FAIL_NULL(profile_editor);
+	OpenXRInteractionProfileEditorBase *interaction_profile_editor = static_cast<OpenXRInteractionProfileEditorBase *>(tabs->get_tab_control(p_tab));
+	ERR_FAIL_NULL(interaction_profile_editor);
+
+	undo_redo->create_action(TTR("Remove interaction profile"));
+	undo_redo->add_do_method(this, "_do_remove_interaction_profile_editor", interaction_profile_editor);
+	undo_redo->add_undo_method(this, "_do_add_interaction_profile_editor", interaction_profile_editor);
+	undo_redo->commit_action(true);
+
+	action_map->set_edited(true);
+}
+
+void OpenXRActionMapEditor::_do_add_action_set_editor(OpenXRActionSetEditor *p_action_set_editor) {
+	Ref<OpenXRActionSet> action_set = p_action_set_editor->get_action_set();
+	ERR_FAIL_COND(action_set.is_null());
+
+	action_map->add_action_set(action_set);
+	actionsets_vb->add_child(p_action_set_editor);
+}
+
+void OpenXRActionMapEditor::_do_remove_action_set_editor(OpenXRActionSetEditor *p_action_set_editor) {
+	Ref<OpenXRActionSet> action_set = p_action_set_editor->get_action_set();
+	ERR_FAIL_COND(action_set.is_null());
+
+	actionsets_vb->remove_child(p_action_set_editor);
+	action_map->remove_action_set(action_set);
+}
+
+void OpenXRActionMapEditor::_do_add_interaction_profile_editor(OpenXRInteractionProfileEditorBase *p_interaction_profile_editor) {
+	Ref<OpenXRInteractionProfile> interaction_profile = p_interaction_profile_editor->get_interaction_profile();
+	ERR_FAIL_COND(interaction_profile.is_null());
+
+	action_map->add_interaction_profile(interaction_profile);
+	tabs->add_child(p_interaction_profile_editor);
+	tabs->set_tab_button_icon(tabs->get_tab_count() - 1, get_theme_icon(SNAME("close"), SNAME("TabBar")));
+
+	tabs->set_current_tab(tabs->get_tab_count() - 1);
+}
 
-	Ref<OpenXRInteractionProfile> interaction_profile = profile_editor->get_interaction_profile();
+void OpenXRActionMapEditor::_do_remove_interaction_profile_editor(OpenXRInteractionProfileEditorBase *p_interaction_profile_editor) {
+	Ref<OpenXRInteractionProfile> interaction_profile = p_interaction_profile_editor->get_interaction_profile();
 	ERR_FAIL_COND(interaction_profile.is_null());
 
+	tabs->remove_child(p_interaction_profile_editor);
 	action_map->remove_interaction_profile(interaction_profile);
-	tabs->remove_child(profile_editor);
-	profile_editor->queue_free();
 }
 
 void OpenXRActionMapEditor::open_action_map(String p_path) {
 	EditorNode::get_singleton()->make_bottom_panel_item_visible(this);
 
+	// out with the old...
+	_clear_action_map();
+
+	// now load in our new action map
 	_load_action_map(p_path);
 
-	_update_action_sets();
-	_update_interaction_profiles();
+	_create_action_sets();
+	_create_interaction_profiles();
+}
+
+void OpenXRActionMapEditor::_clear_action_map() {
+	while (actionsets_vb->get_child_count() > 0) {
+		Node *child = actionsets_vb->get_child(0);
+		actionsets_vb->remove_child(child);
+		child->queue_free();
+	}
+
+	for (int i = 0; i < tabs->get_tab_count(); i++) {
+		// First tab won't be an interaction profile editor, but being thorough..
+		OpenXRInteractionProfileEditorBase *interaction_profile_editor = static_cast<OpenXRInteractionProfileEditorBase *>(tabs->get_tab_control(i));
+		if (interaction_profile_editor) {
+			tabs->remove_child(interaction_profile_editor);
+			interaction_profile_editor->queue_free();
+		}
+	}
 }
 
 OpenXRActionMapEditor::OpenXRActionMapEditor() {
+	undo_redo = EditorNode::get_undo_redo();
 	set_custom_minimum_size(Size2(0.0, 300.0));
 
 	top_hb = memnew(HBoxContainer);
@@ -364,7 +448,9 @@ OpenXRActionMapEditor::OpenXRActionMapEditor() {
 	select_interaction_profile_dialog->connect("interaction_profile_selected", callable_mp(this, &OpenXRActionMapEditor::_on_interaction_profile_selected));
 	add_child(select_interaction_profile_dialog);
 
-	_load_action_map(GLOBAL_GET("xr/openxr/default_action_map"));
+	// Our Action map editor is only shown if openxr is enabled in project settings
+	// So load our action map and if it doesn't exist, create it right away.
+	_load_action_map(GLOBAL_GET("xr/openxr/default_action_map"), true);
 }
 
 OpenXRActionMapEditor::~OpenXRActionMapEditor() {