OpenXR: change bindings to 'flatten' source paths

modules/openxr/action_map/openxr_action_map.cpp

@@ -576,20 +576,15 @@ PackedStringArray OpenXRActionMap::get_top_level_paths(const Ref<OpenXRAction> p
 		const OpenXRInteractionProfileMetadata::InteractionProfile *profile = OpenXRInteractionProfileMetadata::get_singleton()->get_profile(ip->get_interaction_profile_path());
 
 		if (profile != nullptr) {
-			for (int j = 0; j < ip->get_binding_count(); j++) {
-				Ref<OpenXRIPBinding> binding = ip->get_binding(j);
-				if (binding->get_action() == p_action) {
-					PackedStringArray paths = binding->get_paths();
-
-					for (int k = 0; k < paths.size(); k++) {
-						const OpenXRInteractionProfileMetadata::IOPath *io_path = profile->get_io_path(paths[k]);
-						if (io_path != nullptr) {
-							String top_path = io_path->top_level_path;
-
-							if (!arr.has(top_path)) {
-								arr.push_back(top_path);
-							}
-						}
+			Vector<Ref<OpenXRIPBinding>> bindings = ip->get_bindings_for_action(p_action);
+			for (const Ref<OpenXRIPBinding> &binding : bindings) {
+				String binding_path = binding->get_binding_path();
+				const OpenXRInteractionProfileMetadata::IOPath *io_path = profile->get_io_path(binding_path);
+				if (io_path != nullptr) {
+					String top_path = io_path->top_level_path;
+
+					if (!arr.has(top_path)) {
+						arr.push_back(top_path);
 					}
 				}
 			}

modules/openxr/action_map/openxr_interaction_profile.cpp

@@ -35,23 +35,30 @@ void OpenXRIPBinding::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("get_action"), &OpenXRIPBinding::get_action);
 	ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "action", PROPERTY_HINT_RESOURCE_TYPE, "OpenXRAction"), "set_action", "get_action");
 
-	ClassDB::bind_method(D_METHOD("get_path_count"), &OpenXRIPBinding::get_path_count);
+	ClassDB::bind_method(D_METHOD("set_binding_path", "binding_path"), &OpenXRIPBinding::set_binding_path);
+	ClassDB::bind_method(D_METHOD("get_binding_path"), &OpenXRIPBinding::get_binding_path);
+	ADD_PROPERTY(PropertyInfo(Variant::STRING, "binding_path"), "set_binding_path", "get_binding_path");
+
+	// Deprecated
+#ifndef DISABLE_DEPRECATED
 	ClassDB::bind_method(D_METHOD("set_paths", "paths"), &OpenXRIPBinding::set_paths);
 	ClassDB::bind_method(D_METHOD("get_paths"), &OpenXRIPBinding::get_paths);
-	ADD_PROPERTY(PropertyInfo(Variant::PACKED_STRING_ARRAY, "paths"), "set_paths", "get_paths");
+	ADD_PROPERTY(PropertyInfo(Variant::PACKED_STRING_ARRAY, "paths", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NONE), "set_paths", "get_paths");
 
+	ClassDB::bind_method(D_METHOD("get_path_count"), &OpenXRIPBinding::get_path_count);
 	ClassDB::bind_method(D_METHOD("has_path", "path"), &OpenXRIPBinding::has_path);
 	ClassDB::bind_method(D_METHOD("add_path", "path"), &OpenXRIPBinding::add_path);
 	ClassDB::bind_method(D_METHOD("remove_path", "path"), &OpenXRIPBinding::remove_path);
+#endif // DISABLE_DEPRECATED
 }
 
-Ref<OpenXRIPBinding> OpenXRIPBinding::new_binding(const Ref<OpenXRAction> p_action, const char *p_paths) {
+Ref<OpenXRIPBinding> OpenXRIPBinding::new_binding(const Ref<OpenXRAction> p_action, const String &p_binding_path) {
 	// This is a helper function to help build our default action sets
 
 	Ref<OpenXRIPBinding> binding;
 	binding.instantiate();
 	binding->set_action(p_action);
-	binding->parse_paths(String(p_paths));
+	binding->set_binding_path(p_binding_path);
 
 	return binding;
 }
@@ -65,42 +72,68 @@ Ref<OpenXRAction> OpenXRIPBinding::get_action() const {
 	return action;
 }
 
-int OpenXRIPBinding::get_path_count() const {
-	return paths.size();
+void OpenXRIPBinding::set_binding_path(const String &path) {
+	binding_path = path;
+	emit_changed();
 }
 
-void OpenXRIPBinding::set_paths(const PackedStringArray p_paths) {
-	paths = p_paths;
-	emit_changed();
+String OpenXRIPBinding::get_binding_path() const {
+	return binding_path;
 }
 
-PackedStringArray OpenXRIPBinding::get_paths() const {
+#ifndef DISABLE_DEPRECATED
+
+void OpenXRIPBinding::set_paths(const PackedStringArray p_paths) { // Deprecated, but needed for loading old action maps.
+	// Fallback logic, this should ONLY be called when loading older action maps.
+	// We'll parse this momentarily and extract individual bindings.
+	binding_path = "";
+	for (const String &path : p_paths) {
+		if (!binding_path.is_empty()) {
+			binding_path += ",";
+		}
+		binding_path += path;
+	}
+}
+
+PackedStringArray OpenXRIPBinding::get_paths() const { // Deprecated, but needed for converting old action maps.
+	// Fallback logic, return an array.
+	// If we just loaded an old action map from disc, this will be a comma separated list of actions.
+	// Once parsed there should be only one path in our array.
+	PackedStringArray paths = binding_path.split(",", false);
+
 	return paths;
 }
 
-void OpenXRIPBinding::parse_paths(const String p_paths) {
-	paths = p_paths.split(",", false);
-	emit_changed();
+int OpenXRIPBinding::get_path_count() const { // Deprecated.
+	// Fallback logic, we only have one entry.
+	return binding_path.is_empty() ? 0 : 1;
 }
 
-bool OpenXRIPBinding::has_path(const String p_path) const {
-	return paths.has(p_path);
+bool OpenXRIPBinding::has_path(const String p_path) const { // Deprecated.
+	// Fallback logic, return true if this is our path.
+	return binding_path == p_path;
 }
 
-void OpenXRIPBinding::add_path(const String p_path) {
-	if (!paths.has(p_path)) {
-		paths.push_back(p_path);
+void OpenXRIPBinding::add_path(const String p_path) { // Deprecated.
+	// Fallback logic, only assign first time this is called.
+	if (binding_path != p_path) {
+		ERR_FAIL_COND_MSG(!binding_path.is_empty(), "Method add_path has been deprecated. A binding path was already set, create separate binding resources for each path and use set_binding_path instead.");
+
+		binding_path = p_path;
 		emit_changed();
 	}
 }
 
-void OpenXRIPBinding::remove_path(const String p_path) {
-	if (paths.has(p_path)) {
-		paths.erase(p_path);
-		emit_changed();
-	}
+void OpenXRIPBinding::remove_path(const String p_path) { // Deprecated.
+	ERR_FAIL_COND_MSG(binding_path != p_path, "Method remove_path has been deprecated. Attempt at removing a different binding path, remove the correct binding record from the interaction profile instead.");
+
+	// Fallback logic, clear if this is our path.
+	binding_path = p_path;
+	emit_changed();
 }
 
+#endif // DISABLE_DEPRECATED
+
 OpenXRIPBinding::~OpenXRIPBinding() {
 	action.unref();
 }
@@ -151,9 +184,18 @@ Ref<OpenXRIPBinding> OpenXRInteractionProfile::get_binding(int p_index) const {
 }
 
 void OpenXRInteractionProfile::set_bindings(Array p_bindings) {
-	// TODO add check here that our bindings don't contain duplicate actions
+	bindings.clear();
+
+	for (Ref<OpenXRIPBinding> binding : p_bindings) {
+		String binding_path = binding->get_binding_path();
+		if (binding_path.find_char(',') >= 0) {
+			// Convert old binding approach to new...
+			add_new_binding(binding->get_action(), binding_path);
+		} else {
+			add_binding(binding);
+		}
+	}
 
-	bindings = p_bindings;
 	emit_changed();
 }
 
@@ -161,10 +203,9 @@ Array OpenXRInteractionProfile::get_bindings() const {
 	return bindings;
 }
 
-Ref<OpenXRIPBinding> OpenXRInteractionProfile::get_binding_for_action(const Ref<OpenXRAction> p_action) const {
-	for (int i = 0; i < bindings.size(); i++) {
-		Ref<OpenXRIPBinding> binding = bindings[i];
-		if (binding->get_action() == p_action) {
+Ref<OpenXRIPBinding> OpenXRInteractionProfile::find_binding(const Ref<OpenXRAction> p_action, const String &p_binding_path) const {
+	for (Ref<OpenXRIPBinding> binding : bindings) {
+		if (binding->get_action() == p_action && binding->get_binding_path() == p_binding_path) {
 			return binding;
 		}
 	}
@@ -172,11 +213,23 @@ Ref<OpenXRIPBinding> OpenXRInteractionProfile::get_binding_for_action(const Ref<
 	return Ref<OpenXRIPBinding>();
 }
 
+Vector<Ref<OpenXRIPBinding>> OpenXRInteractionProfile::get_bindings_for_action(const Ref<OpenXRAction> p_action) const {
+	Vector<Ref<OpenXRIPBinding>> ret_bindings;
+
+	for (Ref<OpenXRIPBinding> binding : bindings) {
+		if (binding->get_action() == p_action) {
+			ret_bindings.push_back(binding);
+		}
+	}
+
+	return ret_bindings;
+}
+
 void OpenXRInteractionProfile::add_binding(Ref<OpenXRIPBinding> p_binding) {
 	ERR_FAIL_COND(p_binding.is_null());
 
 	if (!bindings.has(p_binding)) {
-		ERR_FAIL_COND_MSG(get_binding_for_action(p_binding->get_action()).is_valid(), "There is already a binding for this action in this interaction profile");
+		ERR_FAIL_COND_MSG(find_binding(p_binding->get_action(), p_binding->get_binding_path()).is_valid(), "There is already a binding for this action and binding path in this interaction profile.");
 
 		bindings.push_back(p_binding);
 		emit_changed();
@@ -191,11 +244,15 @@ void OpenXRInteractionProfile::remove_binding(Ref<OpenXRIPBinding> p_binding) {
 	}
 }
 
-void OpenXRInteractionProfile::add_new_binding(const Ref<OpenXRAction> p_action, const char *p_paths) {
+void OpenXRInteractionProfile::add_new_binding(const Ref<OpenXRAction> p_action, const String &p_paths) {
 	// This is a helper function to help build our default action sets
 
-	Ref<OpenXRIPBinding> binding = OpenXRIPBinding::new_binding(p_action, p_paths);
-	add_binding(binding);
+	PackedStringArray paths = p_paths.split(",", false);
+
+	for (const String &path : paths) {
+		Ref<OpenXRIPBinding> binding = OpenXRIPBinding::new_binding(p_action, path);
+		add_binding(binding);
+	}
 }
 
 void OpenXRInteractionProfile::remove_binding_for_action(const Ref<OpenXRAction> p_action) {

modules/openxr/action_map/openxr_interaction_profile.h

@@ -41,26 +41,29 @@ class OpenXRIPBinding : public Resource {
 
 private:
 	Ref<OpenXRAction> action;
-	PackedStringArray paths;
+	String binding_path;
 
 protected:
 	static void _bind_methods();
 
 public:
-	static Ref<OpenXRIPBinding> new_binding(const Ref<OpenXRAction> p_action, const char *p_paths); // Helper function for adding a new binding
+	static Ref<OpenXRIPBinding> new_binding(const Ref<OpenXRAction> p_action, const String &p_binding_path); // Helper function for adding a new binding.
 
-	void set_action(const Ref<OpenXRAction> p_action); // Set the action for this binding
-	Ref<OpenXRAction> get_action() const; // Get the action for this binding
+	void set_action(const Ref<OpenXRAction> p_action); // Set the action for this binding.
+	Ref<OpenXRAction> get_action() const; // Get the action for this binding.
 
-	int get_path_count() const; // Get the number of io paths
-	void set_paths(const PackedStringArray p_paths); // Set our paths (for loading from resource)
-	PackedStringArray get_paths() const; // Get our paths (for saving to resource)
+	void set_binding_path(const String &path);
+	String get_binding_path() const;
 
-	void parse_paths(const String p_paths); // Parse a comma separated string of io paths.
-
-	bool has_path(const String p_path) const; // Has this io path
-	void add_path(const String p_path); // Add an io path
-	void remove_path(const String p_path); // Remove an io path
+	// Deprecated.
+#ifndef DISABLE_DEPRECATED
+	void set_paths(const PackedStringArray p_paths); // Set our paths (for loading from resource), needed for loading old action maps.
+	PackedStringArray get_paths() const; // Get our paths (for saving to resource), needed for converted old action maps.
+	int get_path_count() const; // Get the number of io paths.
+	bool has_path(const String p_path) const; // Has this io path.
+	void add_path(const String p_path); // Add an io path.
+	void remove_path(const String p_path); // Remove an io path.
+#endif // DISABLE_DEPRECATED
 
 	// TODO add validation that we can display in the interface that checks if no two paths belong to the same top level path
 
@@ -88,11 +91,12 @@ public:
 	void set_bindings(Array p_bindings); // Set the bindings (for loading from a resource)
 	Array get_bindings() const; // Get the bindings (for saving to a resource)
 
-	Ref<OpenXRIPBinding> get_binding_for_action(const Ref<OpenXRAction> p_action) const; // Get our binding record for a given action
+	Ref<OpenXRIPBinding> find_binding(const Ref<OpenXRAction> p_action, const String &p_binding_path) const; // Get our binding record
+	Vector<Ref<OpenXRIPBinding>> get_bindings_for_action(const Ref<OpenXRAction> p_action) const; // Get our binding record for a given action
 	void add_binding(Ref<OpenXRIPBinding> p_binding); // Add a binding object
 	void remove_binding(Ref<OpenXRIPBinding> p_binding); // Remove a binding object
 
-	void add_new_binding(const Ref<OpenXRAction> p_action, const char *p_paths); // Create a new binding for this profile
+	void add_new_binding(const Ref<OpenXRAction> p_action, const String &p_paths); // Create a new binding for this profile
 	void remove_binding_for_action(const Ref<OpenXRAction> p_action); // Remove all bindings for this action
 	bool has_binding_for_action(const Ref<OpenXRAction> p_action); // Returns true if we have a binding for this action
 

modules/openxr/doc_classes/OpenXRIPBinding.xml

@@ -4,32 +4,32 @@
 		Defines a binding between an [OpenXRAction] and an XR input or output.
 	</brief_description>
 	<description>
-		This binding resource binds an [OpenXRAction] to inputs or outputs. As most controllers have left hand and right versions that are handled by the same interaction profile we can specify multiple bindings. For instance an action "Fire" could be bound to both "/user/hand/left/input/trigger" and "/user/hand/right/input/trigger".
+		This binding resource binds an [OpenXRAction] to an input or output. As most controllers have left hand and right versions that are handled by the same interaction profile we can specify multiple bindings. For instance an action "Fire" could be bound to both "/user/hand/left/input/trigger" and "/user/hand/right/input/trigger". This would require two binding entries.
 	</description>
 	<tutorials>
 	</tutorials>
 	<methods>
-		<method name="add_path">
+		<method name="add_path" deprecated="Binding is for a single path.">
 			<return type="void" />
 			<param index="0" name="path" type="String" />
 			<description>
 				Add an input/output path to this binding.
 			</description>
 		</method>
-		<method name="get_path_count" qualifiers="const">
+		<method name="get_path_count" qualifiers="const" deprecated="Binding is for a single path.">
 			<return type="int" />
 			<description>
 				Get the number of input/output paths in this binding.
 			</description>
 		</method>
-		<method name="has_path" qualifiers="const">
+		<method name="has_path" qualifiers="const" deprecated="Binding is for a single path.">
 			<return type="bool" />
 			<param index="0" name="path" type="String" />
 			<description>
 				Returns [code]true[/code] if this input/output path is part of this binding.
 			</description>
 		</method>
-		<method name="remove_path">
+		<method name="remove_path" deprecated="Binding is for a single path.">
 			<return type="void" />
 			<param index="0" name="path" type="String" />
 			<description>
@@ -39,9 +39,13 @@
 	</methods>
 	<members>
 		<member name="action" type="OpenXRAction" setter="set_action" getter="get_action">
-			[OpenXRAction] that is bound to these paths.
+			[OpenXRAction] that is bound to [member binding_path].
 		</member>
-		<member name="paths" type="PackedStringArray" setter="set_paths" getter="get_paths" default="PackedStringArray()">
+		<member name="binding_path" type="String" setter="set_binding_path" getter="get_binding_path" default="&quot;&quot;">
+			Binding path that defines the input or output bound to [member action].
+			[b]Note:[/b] Binding paths are suggestions, an XR runtime may choose to bind the action to a different input or output emulating this input or output.
+		</member>
+		<member name="paths" type="PackedStringArray" setter="set_paths" getter="get_paths" deprecated="Use [member binding_path] instead.">
 			Paths that define the inputs or outputs bound on the device.
 		</member>
 	</members>

modules/openxr/editor/openxr_interaction_profile_editor.cpp

@@ -73,17 +73,19 @@ void OpenXRInteractionProfileEditorBase::_add_binding(const String p_action, con
 	Ref<OpenXRAction> action = action_map->get_action(p_action);
 	ERR_FAIL_COND(action.is_null());
 
-	Ref<OpenXRIPBinding> binding = interaction_profile->get_binding_for_action(action);
+	Ref<OpenXRIPBinding> binding = interaction_profile->find_binding(action, p_path);
 	if (binding.is_null()) {
 		// create a new binding
 		binding.instantiate();
 		binding->set_action(action);
+		binding->set_binding_path(p_path);
+
+		// add it to our interaction profile
 		interaction_profile->add_binding(binding);
 		interaction_profile->set_edited(true);
-	}
 
-	binding->add_path(p_path);
-	binding->set_edited(true);
+		binding->set_edited(true);
+	}
 
 	// Update our toplevel paths
 	action->set_toplevel_paths(action_map->get_top_level_paths(action));
@@ -98,15 +100,10 @@ void OpenXRInteractionProfileEditorBase::_remove_binding(const String p_action,
 	Ref<OpenXRAction> action = action_map->get_action(p_action);
 	ERR_FAIL_COND(action.is_null());
 
-	Ref<OpenXRIPBinding> binding = interaction_profile->get_binding_for_action(action);
+	Ref<OpenXRIPBinding> binding = interaction_profile->find_binding(action, p_path);
 	if (binding.is_valid()) {
-		binding->remove_path(p_path);
-		binding->set_edited(true);
-
-		if (binding->get_path_count() == 0) {
-			interaction_profile->remove_binding(binding);
-			interaction_profile->set_edited(true);
-		}
+		interaction_profile->remove_binding(binding);
+		interaction_profile->set_edited(true);
 
 		// Update our toplevel paths
 		action->set_toplevel_paths(action_map->get_top_level_paths(action));
@@ -116,21 +113,22 @@ void OpenXRInteractionProfileEditorBase::_remove_binding(const String p_action,
 }
 
 void OpenXRInteractionProfileEditorBase::remove_all_bindings_for_action(Ref<OpenXRAction> p_action) {
-	Ref<OpenXRIPBinding> binding = interaction_profile->get_binding_for_action(p_action);
-	if (binding.is_valid()) {
+	Vector<Ref<OpenXRIPBinding>> bindings = interaction_profile->get_bindings_for_action(p_action);
+	if (bindings.size() > 0) {
 		String action_name = p_action->get_name_with_set();
 
 		// for our undo/redo we process all paths
 		undo_redo->create_action(TTR("Remove action from interaction profile"));
-		PackedStringArray paths = binding->get_paths();
-		for (const String &path : paths) {
-			undo_redo->add_do_method(this, "_remove_binding", action_name, path);
-			undo_redo->add_undo_method(this, "_add_binding", action_name, path);
+		for (const Ref<OpenXRIPBinding> &binding : bindings) {
+			undo_redo->add_do_method(this, "_remove_binding", action_name, binding->get_binding_path());
+			undo_redo->add_undo_method(this, "_add_binding", action_name, binding->get_binding_path());
 		}
 		undo_redo->commit_action(false);
 
 		// but we take a shortcut here :)
-		interaction_profile->remove_binding(binding);
+		for (const Ref<OpenXRIPBinding> &binding : bindings) {
+			interaction_profile->remove_binding(binding);
+		}
 		interaction_profile->set_edited(true);
 
 		// Update our toplevel paths
@@ -228,9 +226,8 @@ void OpenXRInteractionProfileEditor::_add_io_path(VBoxContainer *p_container, co
 	if (interaction_profile.is_valid()) {
 		String io_path = String(p_io_path->openxr_path);
 		Array bindings = interaction_profile->get_bindings();
-		for (int i = 0; i < bindings.size(); i++) {
-			Ref<OpenXRIPBinding> binding = bindings[i];
-			if (binding->has_path(io_path)) {
+		for (Ref<OpenXRIPBinding> binding : bindings) {
+			if (binding->get_binding_path() == io_path) {
 				Ref<OpenXRAction> action = binding->get_action();
 
 				HBoxContainer *action_hb = memnew(HBoxContainer);

modules/openxr/openxr_interface.cpp

@@ -300,10 +300,7 @@ void OpenXRInterface::_load_action_map() {
 						continue;
 					}
 
-					PackedStringArray paths = xr_binding->get_paths();
-					for (int k = 0; k < paths.size(); k++) {
-						openxr_api->interaction_profile_add_binding(ip, action->action_rid, paths[k]);
-					}
+					openxr_api->interaction_profile_add_binding(ip, action->action_rid, xr_binding->get_binding_path());
 				}
 
 				// Now submit our suggestions