-
-
Notifications
You must be signed in to change notification settings - Fork 18.8k
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.
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
Stop user from being able to bind multiple identical events to the same action #91278
base: master
Are you sure you want to change the base?
Conversation
editor/action_map_editor.cpp
Outdated
@@ -58,7 +58,15 @@ void ActionMapEditor::_event_config_confirmed() { | |||
Array events = new_action["events"].duplicate(); | |||
|
|||
if (current_action_event_index == -1) { | |||
// Add new event | |||
// Add new event if not already bound | |||
for (const Variant& event : events) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should run clang format / the pre-commit hook on your code, this would fail the CI pipeline.
- Opening brace of for-loop and if-block must be on same line.
- Newly added comments should end with a period.
When updating your PR, make sure to amend and force-push so that you end up with one commit.
ab44b99
to
ece6426
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for letting me know - I ran clang-format on my changes and amended the PR accordingly, will make sure to do so going forward!
editor/action_map_editor.cpp
Outdated
for (const Variant &event : events) { | ||
Ref<InputEvent> event_to_check = Ref<InputEvent>(event); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for (const Variant &event : events) { | |
Ref<InputEvent> event_to_check = Ref<InputEvent>(event); | |
for (Ref<InputEvent> event_to_check : events) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, agreed that this is a bit more succinct. Also made the reference const just because we're not mutating these events while iterating over them.
ece6426
to
1271215
Compare
Fixes #91243
In
ActionMapEditor
, when we return fromInputEventConfigurationDialog
, and we have confirmed that this input relates to a new event being added, we loop through the events currently bound and early out if the new event matches any of the ones that have already been added.