`get_matching_event` selects last controller input instead of first

[yukinogatari]

Hey! I recently upgraded from v3.1.2 to the most recent commit and noticed one tiny regression in how the addon selects icons.

In get_matching_event, the use of push_front for ALL_DEVICE events means that if there are multiple controller mappings for the same event, the last one on the list will end up at the front, which is the opposite of how it behaved in v3.1.2.

There may be a nicer way to handle this, but here's the quick fix I made locally. Since the comments suggest we're prioritizing ALL_DEVICE, I just make two fallback arrays, one for ALL_DEVICE events and one for everything else, fill them up in order, then smash them together at the end.

func get_matching_event(path: String, input_type: InputType = _last_input_type, controller: int = _last_controller) -> InputEvent:
	var events : Array
	if _custom_input_actions.has(path):
		events = _custom_input_actions[path]
	else:
		events = InputMap.action_get_events(path)

	var fallbacks_all = []
	var fallbacks_other = []
	for event in events:
		if not is_instance_valid(event): continue

		match event.get_class():
			"InputEventKey", "InputEventMouse", "InputEventMouseMotion", "InputEventMouseButton":
				if input_type == InputType.KEYBOARD_MOUSE:
					return event
			"InputEventJoypadButton", "InputEventJoypadMotion":
				if input_type == InputType.CONTROLLER:
					# Use the first device specific mapping if there is one.
					if event.device == controller:
						return event
					# Otherwise, we create a fallback prioritizing events with 'ALL_DEVICE'
					if event.device < 0: # All-device event
						fallbacks_all.push_back(event)
					else:
						fallbacks_other.push_back(event)

	var fallbacks = fallbacks_all + fallbacks_other
	return fallbacks[0] if not fallbacks.is_empty() else null

[rsubtil]

cc @greenpixels

[greenpixels]

Hi, great point - thank you for reporting this. That would work as an overall fix - we could also iterate backwards, which should restore the old behavior.

for event_index in range(events.size() - 1, -1, -1):
    var event = events[event_index]

Any preferences? @rsubtil

[rsubtil]

That would work as an overall fix - we could also iterate backwards, which should restore the old behavior.

I'm not sure, wouldn't this then cause issues for the behavior of other events that are currently push_backed into the fallback array? Then those events would be picked in reverse order too.

I think @yukinogatari's solution might be simpler to grasp, even if it's more verbose. Although, at this point, there is no reason to have two separate arrays, knowing that only the first object is returned. It is sufficient to have two references instead, and return the first one if valid, otherwise return the second one:

func get_matching_event(path: String, input_type: InputType = _last_input_type, controller: int = _last_controller) -> InputEvent:
	var events : Array
	if _custom_input_actions.has(path):
		events = _custom_input_actions[path]
	else:
		events = InputMap.action_get_events(path)

	var fallback_all : InputEvent = null
	var fallback_other : InputEvent = null
	for event in events:
		if not is_instance_valid(event): continue

		match event.get_class():
			"InputEventKey", "InputEventMouse", "InputEventMouseMotion", "InputEventMouseButton":
				if input_type == InputType.KEYBOARD_MOUSE:
					return event
			"InputEventJoypadButton", "InputEventJoypadMotion":
				if input_type == InputType.CONTROLLER:
					# Use the first device specific mapping if there is one.
					if event.device == controller:
						return event
					# Otherwise, we create a fallback prioritizing events with 'ALL_DEVICE'
					if not fallback_all and event.device < 0: # All-device event
						fallback_all = event
					elif not fallback_other:
						fallback_other = event

	return fallback_all if fallback_all else fallback_other