Merge pull request #98792 from MJacred/fix_remove_joy_mapping

Fix `Input::remove_joy_mapping`

core/input/input.cpp

@@ -1602,9 +1602,6 @@ void Input::parse_mapping(const String &p_mapping) {
 		return;
 	}
 
-	CharString uid;
-	uid.resize(17);
-
 	mapping.uid = entry[0];
 	mapping.name = entry[1];
 
@@ -1712,15 +1709,72 @@ void Input::add_joy_mapping(const String &p_mapping, bool p_update_existing) {
 }
 
 void Input::remove_joy_mapping(const String &p_guid) {
+	// One GUID can exist multiple times in `map_db`, and
+	// `add_joy_mapping` can choose not to update the existing mapping,
+	// so the indices can be all over the place. Therefore we need to remember them.
+	Vector<int> removed_idx;
+	int min_removed_idx = -1;
+	int max_removed_idx = -1;
+	int fallback_mapping_offset = 0;
+
 	for (int i = map_db.size() - 1; i >= 0; i--) {
 		if (p_guid == map_db[i].uid) {
 			map_db.remove_at(i);
+
+			if (max_removed_idx == -1) {
+				max_removed_idx = i;
+			}
+			min_removed_idx = i;
+			removed_idx.push_back(i);
+
+			if (i < fallback_mapping) {
+				fallback_mapping_offset++;
+			} else if (i == fallback_mapping) {
+				fallback_mapping = -1;
+				WARN_PRINT_ONCE(vformat("Removed fallback joypad input mapping \"%s\". This could lead to joypads not working as intended.", p_guid));
+			}
 		}
 	}
+
+	if (min_removed_idx == -1) {
+		return; // Nothing removed.
+	}
+
+	if (fallback_mapping > 0) {
+		// Fix the shifted index.
+		fallback_mapping -= fallback_mapping_offset;
+	}
+
+	int removed_idx_size = removed_idx.size();
+
+	// Update joypad mapping references: some
+	// * should use the fallback_mapping (if set; if not, they get unmapped), or
+	// * need their mapping reference fixed, because the deletion(s) offset them.
 	for (KeyValue<int, Joypad> &E : joy_names) {
 		Joypad &joy = E.value;
-		if (joy.uid == p_guid) {
-			_set_joypad_mapping(joy, -1);
+		if (joy.mapping < min_removed_idx) {
+			continue; // Not affected.
+		}
+
+		if (joy.mapping > max_removed_idx) {
+			_set_joypad_mapping(joy, joy.mapping - removed_idx_size);
+			continue; // Simple offset fix.
+		}
+
+		// removed_idx is in reverse order (ie. high to low), because the first loop is in reverse order.
+		for (int i = 0; i < removed_idx.size(); i++) {
+			if (removed_idx[i] == joy.mapping) {
+				// Set to fallback_mapping, if defined, else unmap the joypad.
+				// Currently, the fallback_mapping is only set internally, and only for Android.
+				_set_joypad_mapping(joy, fallback_mapping);
+				break;
+			}
+			if (removed_idx[i] < joy.mapping) {
+				// Complex offset fix:
+				// This mapping was shifted by `(removed_idx_size - i)` deletions.
+				_set_joypad_mapping(joy, joy.mapping - (removed_idx_size - i));
+				break;
+			}
 		}
 	}
 }

core/input/input.h

@@ -184,7 +184,7 @@ private:
 
 	HashSet<uint32_t> ignored_device_ids;
 
-	int fallback_mapping = -1;
+	int fallback_mapping = -1; // Index of the guid in map_db.
 
 	CursorShape default_shape = CURSOR_ARROW;
 

doc/classes/Input.xml

@@ -311,7 +311,8 @@
 			<return type="void" />
 			<param index="0" name="guid" type="String" />
 			<description>
-				Removes all mappings from the internal database that match the given GUID.
+				Removes all mappings from the internal database that match the given GUID. All currently connected joypads that use this GUID will become unmapped.
+				On Android, Godot will map to an internal fallback mapping.
 			</description>
 		</method>
 		<method name="set_accelerometer">