Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix InputEvent device id clash (reverted) #97707

Merged
merged 1 commit into from
Oct 21, 2024
Merged

Fix InputEvent device id clash (reverted) #97707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions core/config/project_settings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ bool ProjectSettings::_load_resource_pack(const String &p_pack, bool p_replace_f
}

void ProjectSettings::_convert_to_last_version(int p_from_version) {
#ifndef DISABLE_DEPRECATED
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) {
Expand All @@ -508,6 +509,22 @@ void ProjectSettings::_convert_to_last_version(int p_from_version) {
}
}
}
if (p_from_version <= 5) {
Copy link
Member

@KoBeWi KoBeWi Oct 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least wrap it in DISABLE_DEPRECATED check then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
}
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// Converts the device in events from -1 (emulated events) to -3 (all events).
for (KeyValue<StringName, ProjectSettings::VariantContainer> &E : props) {
if (String(E.key).begins_with("input/")) {
Dictionary action = E.value.variant;
Array events = action["events"];
for (int i = 0; i < events.size(); i++) {
Ref<InputEvent> x = events[i];
if (x->get_device() == -1) { // -1 was the previous value (GH-97707).
x->set_device(InputEvent::DEVICE_ID_ALL_DEVICES);
}
}
}
}
}
#endif // DISABLE_DEPRECATED
}

/*
Expand Down
3 changes: 0 additions & 3 deletions core/input/input_event.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@
#include "core/os/keyboard.h"
#include "core/os/os.h"

const int InputEvent::DEVICE_ID_EMULATION = -1;
const int InputEvent::DEVICE_ID_INTERNAL = -2;

void InputEvent::set_device(int p_device) {
device = p_device;
emit_changed();
Expand Down
5 changes: 3 additions & 2 deletions core/input/input_event.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ class InputEvent : public Resource {
static void _bind_methods();

public:
static const int DEVICE_ID_EMULATION;
static const int DEVICE_ID_INTERNAL;
inline static constexpr int DEVICE_ID_EMULATION = -1;
inline static constexpr int DEVICE_ID_INTERNAL = -2;
inline static constexpr int DEVICE_ID_ALL_DEVICES = -3; // Signify that a given Action can be triggered by any device.

void set_device(int p_device);
int get_device() const;
Expand Down
4 changes: 1 addition & 3 deletions core/input/input_map.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@

InputMap *InputMap::singleton = nullptr;

int InputMap::ALL_DEVICES = -1;

void InputMap::_bind_methods() {
ClassDB::bind_method(D_METHOD("has_action", "action"), &InputMap::has_action);
ClassDB::bind_method(D_METHOD("get_actions"), &InputMap::_get_actions);
Expand Down Expand Up @@ -162,7 +160,7 @@ List<Ref<InputEvent>>::Element *InputMap::_find_event(Action &p_action, const Re
int i = 0;
for (List<Ref<InputEvent>>::Element *E = p_action.inputs.front(); E; E = E->next()) {
int device = E->get()->get_device();
if (device == ALL_DEVICES || device == p_event->get_device()) {
if (device == InputEvent::DEVICE_ID_ALL_DEVICES || device == p_event->get_device()) {
if (E->get()->action_match(p_event, p_exact_match, p_action.deadzone, r_pressed, r_strength, r_raw_strength)) {
if (r_event_index) {
*r_event_index = i;
Expand Down
5 changes: 0 additions & 5 deletions core/input/input_map.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ class InputMap : public Object {
GDCLASS(InputMap, Object);

public:
/**
* A special value used to signify that a given Action can be triggered by any device
*/
static int ALL_DEVICES;

struct Action {
int id;
float deadzone;
Expand Down
2 changes: 1 addition & 1 deletion editor/event_listener_line_edit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ String EventListenerLineEdit::get_event_text(const Ref<InputEvent> &p_event, boo
}

String EventListenerLineEdit::get_device_string(int p_device) {
if (p_device == InputMap::ALL_DEVICES) {
if (p_device == InputEvent::DEVICE_ID_ALL_DEVICES) {
return TTR("All Devices");
}
return TTR("Device") + " " + itos(p_device);
Expand Down
15 changes: 8 additions & 7 deletions editor/input_event_configuration_dialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,18 +551,18 @@ void InputEventConfigurationDialog::_input_list_item_selected() {
}

void InputEventConfigurationDialog::_device_selection_changed(int p_option_button_index) {
// Subtract 1 as option index 0 corresponds to "All Devices" (value of -1)
// and option index 1 corresponds to device 0, etc...
event->set_device(p_option_button_index - 1);
// Option index 0 corresponds to "All Devices" (value of -3).
// Otherwise subtract 1 as option index 1 corresponds to device 0, etc...
event->set_device(p_option_button_index == 0 ? InputEvent::DEVICE_ID_ALL_DEVICES : p_option_button_index - 1);
event_as_text->set_text(EventListenerLineEdit::get_event_text(event, true));
}

void InputEventConfigurationDialog::_set_current_device(int p_device) {
device_id_option->select(p_device + 1);
device_id_option->select(p_device == InputEvent::DEVICE_ID_ALL_DEVICES ? 0 : p_device + 1);
}

int InputEventConfigurationDialog::_get_current_device() const {
return device_id_option->get_selected() - 1;
return device_id_option->get_selected() == 0 ? InputEvent::DEVICE_ID_ALL_DEVICES : device_id_option->get_selected() - 1;
}

void InputEventConfigurationDialog::_notification(int p_what) {
Expand Down Expand Up @@ -705,11 +705,12 @@ InputEventConfigurationDialog::InputEventConfigurationDialog() {

device_id_option = memnew(OptionButton);
device_id_option->set_h_size_flags(Control::SIZE_EXPAND_FILL);
for (int i = -1; i < 8; i++) {
device_id_option->add_item(EventListenerLineEdit::get_device_string(InputEvent::DEVICE_ID_ALL_DEVICES));
for (int i = 0; i < 8; i++) {
device_id_option->add_item(EventListenerLineEdit::get_device_string(i));
}
device_id_option->connect(SceneStringName(item_selected), callable_mp(this, &InputEventConfigurationDialog::_device_selection_changed));
_set_current_device(InputMap::ALL_DEVICES);
_set_current_device(InputEvent::DEVICE_ID_ALL_DEVICES);
device_container->add_child(device_id_option);

device_container->hide();
Expand Down
Loading