-
-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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
Input - prevent action missed input on Android #83438
Conversation
On Android, if input came in on a tick or frame after the physics_process or process, it could be missed when relying on is_action_just_pressed(). This is because by the time the action got around to being checked, the tick or frame would have passed. This PR changes the timestamp for the action to be for the next tick or next frame (preventing missed input), but only immediately prior to process and physics_process, in order to minimize latency.
I will extend my test project to measure the input latency. After that I will try this PR as well. My guess was on the queue_redraw function, maybe the latency can be reduced further? I think if |
uint64_t input_curr_tick = 0; | ||
uint64_t input_curr_process_frame = 0; |
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.
What do you think about using naming similar to Engine and Action: input_physics_frame
and input_process_frame
?
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.
The terminology of tick
and physics_frame
has been discussed before, I won't rehash here.
Upshot was to move over to the tick
terminology wherever possible. See for example the renaming to project setting physics/common/physics_ticks_per_second
.
This is used internally in timing code in a number of places, but it may take some time to move existing terminology, and authors often copy terminology from existing code, and there is backward compatibility to think about, so there may be some inconsistencies.
test projekt: test.zip all actions are captured 👍 PR_83438_Screen_recording_20231016_233508.mp4 |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
Closing this for now as it appears the bug in #83301 may have been responsible for #66318 (and explains why it only was reported on 4.x and not in 3.x). On Android currently buffered input should always be used so providing the buffer is flushed before |
On Android, if input came in on a tick or frame after the physics_process or process, it could be missed when relying on
is_action_just_pressed()
. This is because by the time the action got around to being checked, the action's tick or frame would have passed.This PR changes the timestamp for the action to be for the next tick or next frame (preventing missed input), but only immediately prior to
process
andphysics_process
, in order to minimize latency.Alternative to #83301
Fixes #66318
Discussion
There's likely several ways of fixing this (and different ways of doing the sync, e.g.
MainLoop
versusMain::iteration()
versusSceneTree
etc). There might be some better way of protecting people making customMainLoop
- welcome suggestions here, I've just used someDEV_ASSERTS
here, which (rightly or wrongly) is assuming they are building from source, as a customMainLoop
seems to be here there be dragons territory. CustomMainLoop
is really difficult in general once you get down to the subtleties of timing I guess.I was originally unsure about #83301 but it's actually not a bad approach, as essentially we want to pass any input that is occurring after the "read point" to the next tick / frame. But the latency is a bit of a downside, especially for input that could unnecessarily be shifted to the next tick which might be on a later frame.
I first experimented with storing a read tick on each action, such that if it had not been read so far on that tick, the registered tick would be placed on the current tick, but if it had been read, it would be
tick+1
. This would make latency pretty minimal, but I wasn't keen on the haphazard / stochastic nature, so am now of the opinion a fixed sync point might be better.Here I've placed the sync point just before the calls to
physics_process
andprocess
. It may not gain a massive amount over just usingtick+1
everywhere (particularly for idle frame), but it seems slightly better for reducing latency, which is something we should strive for.Notes
input_curr_tick
andinput_curr_process_frame
could possibly be atomic, I'll defer to @RandomShaper 's opinion on this.