Fix InputEvent device id clash (reverted)

[Sauermann]

InputMap::ALL_DEVICES and InputEvent::DEVICE_ID_EMULATION have the same value -1, but since both are used for device ids of input events, there could potentially be cases, where they are clashing.

Change value of InputMap::All_DEVICES so that it's different from InputEvent::DEVICE_ID_EMULATION. InputEvent::DEVICE_ID_EMULATION is part of the API and can't be changed without potentially breaking projects.

Gather all special device constants in a single location inside InputEvent.

Add a converter to project settings, that takes care of adjusting project files during loading.

Relevant PRs:

• Allows to map an action to all devices. #17967
• Fix fake null-motion mouse event flood #26851

[RadiantUwU]

Correctly implemented

[RandomShaper]

Makes sense. I have a little concern though. Is the former value for "all devices" stored somewhere? In that case, it'd be appropriate to leave -1 for that.

[Sauermann]

The only place, where it is used to set something, is within the Editor.
In InputEventConfigurationDialog it is used to configure an OptionButton.

godot/editor/input_event_configuration_dialog.cpp

Line 712 in f4af820

_set_current_device(InputMap::ALL_DEVICES);

I didn't find any indication, that this OptionButton is saved anywhere.

[Sauermann]

While the OptionButton doesn't get saved, using "All Devices" in the project input map will store -1 within the project.godot file.

devicealltest={
"deadzone": 0.5,
"events": [Object(InputEventMouseButton,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"button_mask":0,"position":Vector2(0, 0),"global_position":Vector2(0, 0),"factor":1.0,"button_index":8,"canceled":false,"pressed":false,"double_click":false,"script":null)
]

Projects that are loaded with the change in its current state will interpret previous ALL_DEVICE input map as DEVICE_ID_EMULATION which could cause problems.

One option to work around this would be:

• Adjust the device value from -1 to the new value at the time, when the Input Map is loaded from the project.godot file.
• This could work, because currently only the values -1, ..., 7 can appear there, so that the mapping can happen in an unambiguous way.
• This compatibility conversion would need to be removed at a time in the future, when compatibility with old project files can be broken.
• Before trying to implement this change, I would like feedback if this is a sensible way to deal with this situation.

[RandomShaper]

Why don't just leave -1 for "all devices" and use, say, -2 for emulation?

[Sauermann]

Same problem, because DEVICE_ID_EMULATION is exposed to GDScript and part of the API, so changing the value would need to be considered as breaking compatibility. I would like to try to avoid this.

[RandomShaper]

So, being that the case, your proposed workaround seems to be the most sensible thing to do.

[Sauermann]

It was necessary to add additional conversion code to InputEventConfigurationDialog.

[KoBeWi]

You need to increment project version, otherwise this code will run every time.

godot/core/config/project_settings.h

Line 152 in 44fa552

static const int CONFIG_VERSION = 5;

[Sauermann]

I didn't want to unnecessarily increment the project version number.

This conversion can be implemented without incrementing the project version.
Before this PR, possible values of device id in the project file are: -1, 0, ..., 7
With this PR, possible values of device id in the project file are: -3, 0, ..., 7
So the conversion doesn't affect projects, that are saved with this PR. The conversion just makes sure, that all projects use -3 for "all devices" after they are loaded.

Also it is compatible with future project version increments.

The conversion happens only once during loading, so there shouldn't be any noticeable delay.

[KoBeWi]

At least wrap it in DISABLE_DEPRECATED check then.

[Sauermann]

Should I wrap the section above also in a DISABLE_DEPRECATED check?

godot/core/config/project_settings.cpp

Lines 497 to 509 in 44fa552

	if (p_from_version <= 3) {
	// Converts the actions from array to dictionary (array of events to dictionary with deadzone + events)
	for (KeyValue<StringName, ProjectSettings::VariantContainer> &E : props) {
	Variant value = E.value.variant;
	if (String(E.key).begins_with("input/") && value.get_type() == Variant::ARRAY) {
	Array array = value;
	Dictionary action;
	action["deadzone"] = Variant(0.5f);
	action["events"] = array;
	E.value.variant = action;
	}
	}
	}

[KoBeWi]

tbh the other sections is irrelevant at this point. Version 5 is used since 4.0.
But yeah, it should be wrapped too.

[RandomShaper]

5 approvals! 😃

[Repiteo]

Thanks!

[passivestar]

Hey, this change breaks compat 🙂

I'm storing keymaps in InputEvent resources because project settings aren't portable. Keyboard stopped working for me, had to figure out why and remap. Not a big deal, just fyi in case you want to add this to a list of breaking changes

[mgiuca]

This also causes a pretty large change to the project.godot file (every action changes its device ID when you open a project and save it in 4.4-dev4). Might be worth calling out in the release notes as well.