Skip to content

Fix action state when multiple events are assigned #80859

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

Merged
akien-mga merged 1 commit into from
Aug 29, 2023
Merged

Fix action state when multiple events are assigned #80859

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
57 changes: 36 additions & 21 deletions core/input/input.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,9 @@ bool Input::is_joy_button_pressed(int p_device, JoyButton p_button) const {

bool Input::is_action_pressed(const StringName &p_action, bool p_exact) const {
ERR_FAIL_COND_V_MSG(!InputMap::get_singleton()->has_action(p_action), false, InputMap::get_singleton()->suggest_actions(p_action));
return action_state.has(p_action) && action_state[p_action].pressed && (p_exact ? action_state[p_action].exact : true);
if (p_action == "move_right")
print_line(action_state.has(p_action), action_state[p_action].pressed, action_state[p_action].pressed > 0, (p_exact ? action_state[p_action].exact : true));
return action_state.has(p_action) && action_state[p_action].pressed > 0 && (p_exact ? action_state[p_action].exact : true);
}

bool Input::is_action_just_pressed(const StringName &p_action, bool p_exact) const {
Expand Down Expand Up @@ -697,23 +699,30 @@ void Input::_parse_input_event_impl(const Ref<InputEvent> &p_event, bool p_is_em
for (const KeyValue<StringName, InputMap::Action> &E : InputMap::get_singleton()->get_action_map()) {
if (InputMap::get_singleton()->event_is_action(p_event, E.key)) {
Action &action = action_state[E.key];
// If not echo and action pressed state has changed
if (!p_event->is_echo() && is_action_pressed(E.key, false) != p_event->is_action_pressed(E.key)) {
bool is_pressed = false;

if (!p_event->is_echo()) {
if (p_event->is_action_pressed(E.key)) {
action.pressed = true;
action.pressed_physics_frame = Engine::get_singleton()->get_physics_frames();
action.pressed_process_frame = Engine::get_singleton()->get_process_frames();
action.pressed++;
is_pressed = true;
if (action.pressed == 1) {
action.pressed_physics_frame = Engine::get_singleton()->get_physics_frames();
action.pressed_process_frame = Engine::get_singleton()->get_process_frames();
}
} else {
action.pressed = false;
action.released_physics_frame = Engine::get_singleton()->get_physics_frames();
action.released_process_frame = Engine::get_singleton()->get_process_frames();
if (action.pressed == 1) {
action.released_physics_frame = Engine::get_singleton()->get_physics_frames();
action.released_process_frame = Engine::get_singleton()->get_process_frames();
}
action.pressed = MAX(action.pressed - 1, 0);
}
action.strength = 0.0f;
action.raw_strength = 0.0f;
action.exact = InputMap::get_singleton()->event_is_action(p_event, E.key, true);
}
action.strength = p_event->get_action_strength(E.key);
action.raw_strength = p_event->get_action_raw_strength(E.key);

if (is_pressed || action.pressed == 0) {
action.strength = p_event->get_action_strength(E.key);
action.raw_strength = p_event->get_action_raw_strength(E.key);
Comment on lines 723 to 724
Copy link
Copy Markdown
Contributor

@Sauermann Sauermann Aug 25, 2023

Choose a reason for hiding this comment

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

This implementation changes the action.strength on every event, which might lead to alternating values, if two axis inputs are mapped to the same action and both axis are moved.

The comment #45628 (comment) suggests to use as strength the maximum strength of all related actions. Perhaps that would be a better solution?

Copy link
Copy Markdown
Member Author

@KoBeWi KoBeWi Aug 25, 2023

Choose a reason for hiding this comment

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

But then what happens on release? It should assign max strength of still pressed actions, but they are not tracked. This means that if you use analog and then press a key, your analog strength will be ignored until it's released.
Maybe it's fine, idk.

Copy link
Copy Markdown
Contributor

@Sauermann Sauermann Aug 25, 2023

Choose a reason for hiding this comment

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

It should assign max strength of still pressed actions, but they are not tracked

Perhaps we could track it with the following, but I haven thought it through yet.

diff --git a/core/input/input_map.h b/core/input/input_map.h
index b4d5beacb3..b59e898730 100644
--- a/core/input/input_map.h
+++ b/core/input/input_map.h
@@ -48,10 +48,17 @@ public:
         */
        static int ALL_DEVICES;
 
+       struct EventWithStrength {
+               Ref<InputEvent> event;
+               int pressed = 0;
+               float strength = 0.0f;
+               float raw_strength = 0.0f;
+       };
+
        struct Action {
                int id;
                float deadzone;
-               List<Ref<InputEvent>> inputs;
+               List<EventWithStrength> inputs;
        };
 
 private:

Copy link
Copy Markdown
Member Author

@KoBeWi KoBeWi Aug 29, 2023

Choose a reason for hiding this comment

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

This can be addressed later if someone opens an issue. Let's see if the current behavior is ok for now.

Copy link
Copy Markdown
Contributor

@Sauermann Sauermann Aug 29, 2023

Choose a reason for hiding this comment

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

Sounds good to me.

}
}
}

Expand Down Expand Up @@ -831,9 +840,11 @@ void Input::action_press(const StringName &p_action, float p_strength) {
// Create or retrieve existing action.
Action &action = action_state[p_action];

action.pressed_physics_frame = Engine::get_singleton()->get_physics_frames();
action.pressed_process_frame = Engine::get_singleton()->get_process_frames();
action.pressed = true;
action.pressed++;
if (action.pressed == 1) {
action.pressed_physics_frame = Engine::get_singleton()->get_physics_frames();
action.pressed_process_frame = Engine::get_singleton()->get_process_frames();
}
action.strength = p_strength;
action.raw_strength = p_strength;
action.exact = true;
Expand All @@ -843,9 +854,11 @@ void Input::action_release(const StringName &p_action) {
// Create or retrieve existing action.
Action &action = action_state[p_action];

action.released_physics_frame = Engine::get_singleton()->get_physics_frames();
action.released_process_frame = Engine::get_singleton()->get_process_frames();
action.pressed = false;
action.pressed--;
if (action.pressed == 0) {
action.released_physics_frame = Engine::get_singleton()->get_physics_frames();
action.released_process_frame = Engine::get_singleton()->get_process_frames();
}
action.strength = 0.0f;
action.raw_strength = 0.0f;
action.exact = true;
Expand Down Expand Up @@ -1006,8 +1019,10 @@ void Input::release_pressed_events() {
joy_buttons_pressed.clear();
_joy_axis.clear();

for (const KeyValue<StringName, Input::Action> &E : action_state) {
if (E.value.pressed) {
for (KeyValue<StringName, Input::Action> &E : action_state) {
if (E.value.pressed > 0) {
// Make sure the action is really released.
E.value.pressed = 1;
action_release(E.key);
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/input/input.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class Input : public Object {
uint64_t pressed_process_frame = UINT64_MAX;
uint64_t released_physics_frame = UINT64_MAX;
uint64_t released_process_frame = UINT64_MAX;
bool pressed = false;
int pressed = 0;
bool exact = true;
float strength = 0.0f;
float raw_strength = 0.0f;
Expand Down
2 changes: 1 addition & 1 deletion doc/classes/Input.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@
<param index="0" name="action" type="StringName" />
<param index="1" name="exact_match" type="bool" default="false" />
<description>
Returns [code]true[/code] if you are pressing the action event. Note that if an action has multiple buttons assigned and more than one of them is pressed, releasing one button will release the action, even if some other button assigned to this action is still pressed.
Returns [code]true[/code] if you are pressing the action event.
If [param exact_match] is [code]false[/code], it ignores additional input modifiers for [InputEventKey] and [InputEventMouseButton] events, and the direction for [InputEventJoypadMotion] events.
[b]Note:[/b] Due to keyboard ghosting, [method is_action_pressed] may return [code]false[/code] even if one of the action's keys is pressed. See [url=$DOCS_URL/tutorials/inputs/input_examples.html#keyboard-events]Input examples[/url] in the documentation for more information.
</description>
Expand Down