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.

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

Physics tick counter is incorrect within frame #92903

Closed
lawnjelly opened this issue Jun 8, 2024 · 3 comments · Fixed by #94039
Closed

Physics tick counter is incorrect within frame #92903

lawnjelly opened this issue Jun 8, 2024 · 3 comments · Fixed by #94039

Comments

@lawnjelly
Copy link
Member

lawnjelly commented Jun 8, 2024

Tested versions

3.6 beta 5
(probably all earlier)
4.3 dev 6 and prior

System information

All

Issue description

While investigating an MRP, I realised that we are incrementing the physics tick counter in the incorrect place.

Currently the physics tick counter is incremented at the end of the physics tick, whereas the physics tick does not change logically until the next physics tick begins.

This means that, especially at low tick rates, the reported tick within _process() will be one higher than the current tick.

Steps to reproduce

func _physics_process(delta):
	print("physics process - starting tick " + str(Engine.get_physics_frames()))

func _process(delta):
	print("process - physics tick reported as " + str(Engine.get_physics_frames()))

Minimal reproduction project (MRP)

physics_tick_counter.zip

Output:

process - physics tick reported as 12
physics process - starting tick 12
physics process - starting tick 13
physics process - starting tick 14
physics process - starting tick 15
physics process - starting tick 16
physics process - starting tick 17
physics process - starting tick 18
process - physics tick reported as 19
physics process - starting tick 19
physics process - starting tick 20
physics process - starting tick 21
physics process - starting tick 22
physics process - starting tick 23
physics process - starting tick 24
physics process - starting tick 25
physics process - starting tick 26
process - physics tick reported as 27

Discussion

This may appear as a kind of "glass half empty / glass half full" kind of issue, but the problem is that input etc that occurs during the frame will report the wrong physics tick before the next one has started, which can be a problem if you e.g. position objects based on the physics tick.

It's fairly easy to solve by moving the count increment to the start of the physics tick, however, this will take some care to solve properly as it likely to cause regressions. Notably already I have confirmed that Input.is_action_just_pressed() would need adjustment.

This problem is especially notable at low tick rates (e.g. using physics interpolation). At 10 tps, and 60 fps, the reported tick is considerably out of sync.

e.g.

tick 0 begins
frame 0 reports tick 1
frame 1 reports tick 1
frame 2 reports tick 1
frame 3 reports tick 1
frame 4 reports tick 1
frame 5 reports tick 1
tick 1 begins
frame 6 reports tick 2
frame 7 reports tick 2
frame 8 reports tick 2
frame 9 reports tick 2
etc
@Calinou
Copy link
Member

Calinou commented Jun 13, 2024

This is what I see as of 4.3.beta 5833f59 (Linux):

process - physics tick reported as 0
physics process - starting tick 0
process - physics tick reported as 1
physics process - starting tick 1
physics process - starting tick 2
physics process - starting tick 3
physics process - starting tick 4
process - physics tick reported as 5
physics process - starting tick 5
physics process - starting tick 6
physics process - starting tick 7
physics process - starting tick 8
physics process - starting tick 9
physics process - starting tick 10
physics process - starting tick 11
physics process - starting tick 12
process - physics tick reported as 13
physics process - starting tick 13
physics process - starting tick 14
physics process - starting tick 15
physics process - starting tick 16
physics process - starting tick 17
physics process - starting tick 18
physics process - starting tick 19
physics process - starting tick 20
process - physics tick reported as 21
physics process - starting tick 21
physics process - starting tick 22
physics process - starting tick 23
physics process - starting tick 24
physics process - starting tick 25
physics process - starting tick 26
physics process - starting tick 27
physics process - starting tick 28
process - physics tick reported as 29
physics process - starting tick 29
physics process - starting tick 30
physics process - starting tick 31
physics process - starting tick 32
physics process - starting tick 33
physics process - starting tick 34
physics process - starting tick 35
physics process - starting tick 36
process - physics tick reported as 37
physics process - starting tick 37
physics process - starting tick 38
physics process - starting tick 39
physics process - starting tick 40
physics process - starting tick 41
physics process - starting tick 42
physics process - starting tick 43
physics process - starting tick 44
process - physics tick reported as 45
physics process - starting tick 45
physics process - starting tick 46
physics process - starting tick 47
physics process - starting tick 48
physics process - starting tick 49
physics process - starting tick 50
physics process - starting tick 51
physics process - starting tick 52
process - physics tick reported as 53

Does this mean it has the same issue as 3.x?

MRP updated for 4.x: physics_tick_counter_4.x.zip

@lawnjelly
Copy link
Member Author

Yes, also present in 4.x. Will add to issue.

@lawnjelly
Copy link
Member Author

Fixed by #92941 in 3.x, but still present in 4.x, will change milestone and look at fixing there.

@lawnjelly lawnjelly modified the milestones: 3.x, 4.x Jul 1, 2024
@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jul 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants