Merge pull request #51308 from EricEzaM/improved-builting-override-handling

Improve implementation of builtin action overrides

core/input/input_map.cpp

@@ -33,6 +33,7 @@
 #include "core/config/project_settings.h"
 #include "core/input/input.h"
 #include "core/os/keyboard.h"
+#include "core/os/os.h"
 
 InputMap *InputMap::singleton = nullptr;
 
@@ -699,34 +700,57 @@ const OrderedHashMap<String, List<Ref<InputEvent>>> &InputMap::get_builtins() {
 	return default_builtin_cache;
 }
 
-void InputMap::load_default() {
+const OrderedHashMap<String, List<Ref<InputEvent>>> &InputMap::get_builtins_with_feature_overrides_applied() {
+	if (default_builtin_with_overrides_cache.size() > 0) {
+		return default_builtin_with_overrides_cache;
+	}
+
 	OrderedHashMap<String, List<Ref<InputEvent>>> builtins = get_builtins();
 
-	// List of Builtins which have an override for macOS.
-	Vector<String> macos_builtins;
+	// Get a list of all built in inputs which are valid overrides for the OS
+	// Key = builtin name (e.g. ui_accept)
+	// Value = override/feature names (e.g. macos, if it was defined as "ui_accept.macos" and the platform supports that feature)
+	Map<String, Vector<String>> builtins_with_overrides;
 	for (OrderedHashMap<String, List<Ref<InputEvent>>>::Element E = builtins.front(); E; E = E.next()) {
-		if (String(E.key()).ends_with(".macos")) {
-			// Strip .macos from name: some_input_name.macos -> some_input_name
-			macos_builtins.push_back(String(E.key()).split(".")[0]);
+		String fullname = E.key();
+
+		Vector<String> split = fullname.split(".");
+		String name = split[0];
+		String override_for = split.size() > 1 ? split[1] : String();
+
+		if (override_for != String() && OS::get_singleton()->has_feature(override_for)) {
+			builtins_with_overrides[name].push_back(override_for);
 		}
 	}
 
 	for (OrderedHashMap<String, List<Ref<InputEvent>>>::Element E = builtins.front(); E; E = E.next()) {
 		String fullname = E.key();
-		String name = fullname.split(".")[0];
-		String override_for = fullname.split(".").size() > 1 ? fullname.split(".")[1] : "";
 
-#ifdef APPLE_STYLE_KEYS
-		if (macos_builtins.has(name) && override_for != "macos") {
-			// Name has `macos` builtin but this particular one is for non-macOS systems - so skip.
+		Vector<String> split = fullname.split(".");
+		String name = split[0];
+		String override_for = split.size() > 1 ? split[1] : String();
+
+		if (builtins_with_overrides.has(name) && override_for == String()) {
+			// Builtin has an override but this particular one is not an override, so skip.
 			continue;
 		}
-#else
-		if (override_for == "macos") {
-			// Override for macOS - not needed on non-macOS platforms.
+
+		if (override_for != String() && !OS::get_singleton()->has_feature(override_for)) {
+			// OS does not support this override - skip.
 			continue;
 		}
-#endif
+
+		default_builtin_with_overrides_cache.insert(name, E.value());
+	}
+
+	return default_builtin_with_overrides_cache;
+}
+
+void InputMap::load_default() {
+	OrderedHashMap<String, List<Ref<InputEvent>>> builtins = get_builtins_with_feature_overrides_applied();
+
+	for (OrderedHashMap<String, List<Ref<InputEvent>>>::Element E = builtins.front(); E; E = E.next()) {
+		String name = E.key();
 
 		add_action(name);
 

core/input/input_map.h

@@ -56,6 +56,7 @@ private:
 
 	mutable OrderedHashMap<StringName, Action> input_map;
 	OrderedHashMap<String, List<Ref<InputEvent>>> default_builtin_cache;
+	OrderedHashMap<String, List<Ref<InputEvent>>> default_builtin_with_overrides_cache;
 
 	List<Ref<InputEvent>>::Element *_find_event(Action &p_action, const Ref<InputEvent> &p_event, bool p_exact_match = false, bool *p_pressed = nullptr, float *p_strength = nullptr, float *p_raw_strength = nullptr) const;
 
@@ -93,6 +94,7 @@ public:
 	String get_builtin_display_name(const String &p_name) const;
 	// Use an Ordered Map so insertion order is preserved. We want the elements to be 'grouped' somewhat.
 	const OrderedHashMap<String, List<Ref<InputEvent>>> &get_builtins();
+	const OrderedHashMap<String, List<Ref<InputEvent>>> &get_builtins_with_feature_overrides_applied();
 
 	InputMap();
 	~InputMap();

editor/editor_settings.cpp

@@ -1409,7 +1409,7 @@ Ref<Shortcut> EditorSettings::get_shortcut(const String &p_name) const {
 
 	// If there was no override, check the default builtins to see if it has an InputEvent for the provided name.
 	if (sc.is_null()) {
-		const OrderedHashMap<String, List<Ref<InputEvent>>>::ConstElement builtin_default = InputMap::get_singleton()->get_builtins().find(p_name);
+		const OrderedHashMap<String, List<Ref<InputEvent>>>::ConstElement builtin_default = InputMap::get_singleton()->get_builtins_with_feature_overrides_applied().find(p_name);
 		if (builtin_default) {
 			sc.instantiate();
 			sc->set_event(builtin_default.get().front()->get());
@@ -1502,15 +1502,23 @@ void EditorSettings::set_builtin_action_override(const String &p_name, const Arr
 	// Check if the provided event array is same as built-in. If it is, it does not need to be added to the overrides.
 	// Note that event order must also be the same.
 	bool same_as_builtin = true;
-	OrderedHashMap<String, List<Ref<InputEvent>>>::ConstElement builtin_default = InputMap::get_singleton()->get_builtins().find(p_name);
+	OrderedHashMap<String, List<Ref<InputEvent>>>::ConstElement builtin_default = InputMap::get_singleton()->get_builtins_with_feature_overrides_applied().find(p_name);
 	if (builtin_default) {
 		List<Ref<InputEvent>> builtin_events = builtin_default.get();
 
-		if (p_events.size() == builtin_events.size()) {
+		// In the editor we only care about key events.
+		List<Ref<InputEventKey>> builtin_key_events;
+		for (Ref<InputEventKey> iek : builtin_events) {
+			if (iek.is_valid()) {
+				builtin_key_events.push_back(iek);
+			}
+		}
+
+		if (p_events.size() == builtin_key_events.size()) {
 			int event_idx = 0;
 
 			// Check equality of each event.
-			for (const Ref<InputEvent> &E : builtin_events) {
+			for (const Ref<InputEventKey> &E : builtin_key_events) {
 				if (!E->is_match(p_events[event_idx])) {
 					same_as_builtin = false;
 					break;

editor/settings_config_dialog.cpp

@@ -268,7 +268,7 @@ void EditorSettingsDialog::_update_shortcuts() {
 		Array events; // Need to get the list of events into an array so it can be set as metadata on the item.
 		Vector<String> event_strings;
 
-		List<Ref<InputEvent>> all_default_events = InputMap::get_singleton()->get_builtins().find(action_name).value();
+		List<Ref<InputEvent>> all_default_events = InputMap::get_singleton()->get_builtins_with_feature_overrides_applied().find(action_name).value();
 		List<Ref<InputEventKey>> key_default_events;
 		// Remove all non-key events from the defaults. Only check keys, since we are in the editor.
 		for (List<Ref<InputEvent>>::Element *I = all_default_events.front(); I; I = I->next()) {
@@ -404,7 +404,7 @@ void EditorSettingsDialog::_shortcut_button_pressed(Object *p_item, int p_column
 		switch (button_idx) {
 			case SHORTCUT_REVERT: {
 				Array events;
-				List<Ref<InputEvent>> defaults = InputMap::get_singleton()->get_builtins()[current_action];
+				List<Ref<InputEvent>> defaults = InputMap::get_singleton()->get_builtins_with_feature_overrides_applied()[current_action];
 
 				// Convert the list to an array, and only keep key events as this is for the editor.
 				for (const Ref<InputEvent> &k : defaults) {