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

feat: Use OS time for tick loop #225

Merged
merged 5 commits into from
Jun 3, 2024
Merged

Conversation

elementbound
Copy link
Contributor

@elementbound elementbound commented May 29, 2024

Use OS time for tick loop to avoid time drifting between clients. Also supports pausing both from editor and via SceneTree.pause.

Refs #177

Copy link
Contributor

@albertok albertok left a comment

Choose a reason for hiding this comment

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

Works well, the clocks stay in sync from my tests even when high process load occurs.

Something seems bit off on the tick-interpolators in my game now though, I'm seeing slight jitter in movement where previously it was pretty smooth.

I've confirmed it only happens with the new tick system.
I don't see this issue when I run Forest Brawler though.

My game runs at:

tick rate: 30
fps: ~60 fps ( vsync enabled )
physics: 60

@elementbound
Copy link
Contributor Author

@albertok interesting, I have the same - Forest Brawl is fine, my own game jitters. My first thought was that it's because Forest Brawl runs at a solid 240 fps, while the other game can vary at 50-60 fps, so maybe the factor was capped before the next tick could be simulated. This doesn't seem to be the case.

I'll look into it more, I'm suspecting it's something around NetworkTime.tick_factor.

@albertok
Copy link
Contributor

albertok commented May 30, 2024

@elementbound

Worked it out. Were skipping tiny bits of time by sampling the clock as interpolation is about to happen which leads to the jitter. Because interpolation is meant to calculate the difference between two rendering frames we need to pin the time at the start of the render frame and calculate from there consistently.

@elementbound
Copy link
Contributor Author

@albertok Nice catch! After some testing, I've noticed on my own game that there's still a small amount of jittering. I think this is due to my camera not being tied to the tick loop. If instead I do the same as in Forest Brawl ( update pos in _tick and use TickInterpolator ), everything is silky smooth. I think this is why we didn't see the issue in Forest Brawl originally.

The other issue is if you pause the game for a while - since we're using OS time - netfox will think the game is multiple seconds behind so it will simulate things as fast as it can. I'll see if we can catch the pause and do something about it.

@elementbound elementbound marked this pull request as ready for review June 1, 2024 20:53
@elementbound
Copy link
Contributor Author

Pinned the OS time at the start of the frame as suggested. Also added support for paused games through get_tree().paused, and a hacky way to detect if the game was paused from the editor. The latter only works in the editor, so hopefully the false positives shouldn't affect final builds.

For testing, I've tied my own game's camera to the tick loop and it works smooth!

@albertok would love to have your review on this!

@elementbound elementbound changed the title refactor: Use OS time for tick loop feat: Use OS time for tick loop Jun 1, 2024
@albertok
Copy link
Contributor

albertok commented Jun 3, 2024

@elementbound

Initial testing feels really good. There's no sense of lag/delay on local matches at all now and higher latency is also improved.

addons/netfox/network-time.gd Outdated Show resolved Hide resolved
addons/netfox/network-time.gd Show resolved Hide resolved
@elementbound elementbound merged commit c971417 into main Jun 3, 2024
@elementbound elementbound deleted the refactor/os-clock-timing branch June 3, 2024 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants