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

[3.x] Fix Viewport interpolation mode #92152

Merged

Conversation

lawnjelly
Copy link
Member

Viewport interpolation mode is a special case, which should be set to ON instead of INHERIT.

Fixes #92150
Fixes #91918

Notes

  • Controls defaulting to OFF is a trade off - it makes adding controls easy to a physics interpolated game, but users need to be aware that nodes below the control will usually inherit this OFF setting.
  • Viewports are typically children of Controls, and previously would inherit OFF, which could be unexpected for physics interpolated games.
  • The simplest and most sensible solution seems to be to reset the physics interpolation mode and consider the Viewport as a new root of each subscene.
  • This is something that can eventually be added to the physics interpolation docs, along with other special cases. But it should now work in a more expected manner for most users.

@lawnjelly lawnjelly added this to the 3.6 milestone May 20, 2024
@lawnjelly lawnjelly requested a review from a team as a code owner May 20, 2024 08:05
scene/main/viewport.cpp Outdated Show resolved Hide resolved
@akien-mga
Copy link
Member

CC @rburing - I guess this might be relevant for 4.x too?

@lawnjelly lawnjelly force-pushed the fix_viewport_interpolation_mode branch from f94a80d to 2f93e6a Compare May 20, 2024 08:19
@lawnjelly
Copy link
Member Author

I guess this might be relevant for 4.x too?

Yes, this should be forward ported as it will affect 3D and 2D games using Viewports.

Viewport interpolation mode is a special case, which should be set to ON instead of INHERIT.
@lawnjelly lawnjelly force-pushed the fix_viewport_interpolation_mode branch from 2f93e6a to 0f5cadf Compare May 20, 2024 09:29
@lawnjelly lawnjelly requested a review from a team as a code owner May 20, 2024 09:29
@lawnjelly lawnjelly merged commit bb342cb into godotengine:3.x May 20, 2024
14 checks passed
@lawnjelly
Copy link
Member Author

Thanks!

@oeleo1
Copy link

oeleo1 commented May 21, 2024

Wow! That's pretty major - si it turns out I was tricked to believe we were using FTI for 2D while most things are running in a ViewportContainer without it? Hmm... We did have to fix visual artefacts with reset_physics_interpolation() so we were definitely running FTI but I can't recall right now whether this was inside or outside that ViewportContainer. Something to test with a new 3.6 beta in a not so distant future hopefully :-)

Thanks!

@lawnjelly lawnjelly deleted the fix_viewport_interpolation_mode branch May 21, 2024 10:44
@lawnjelly
Copy link
Member Author

Something to test with a new 3.6 beta in a not so distant future hopefully :-)

Yes we should have an RC soon as this is just bug fixing phase.

As I say in the docs, for anything physics interpolation related, you should be regularly switching your physics tick rate down to e.g. 10 tps so that things are super obvious, e.g. need for resets. You can't rely on noticing whether things are operating correctly by eye at 60tps.

@oeleo1
Copy link

oeleo1 commented May 21, 2024

As I say in the docs, for anything physics interpolation related, you should be regularly switching your physics tick rate down to e.g. 10 tps so that things are super obvious, e.g. need for resets. You can't rely on noticing whether things are operating correctly by eye at 60tps.

I wish that was true. It is not. We can't operate the game at anything below 60tps. It is a real-time action game. This is due to an unimaginable set of factors for the Godot developers. They include plenty of physics interactions, barrier hits, penetration thresholds, increased gravities to prevent sporadic jumps on curved rigid bodies, etc., etc., etc. If we lower physics below 60 fps, say 20fps, nothing moves as expected. At 30 fps - barely acceptable but clearly unplayable, 40 and 50 - almost playable, but really regularily irritating. Whatever cool things the docs say, belive it or not, people apply to them a x16 Anisotropic filter right away, according to their own project. :-) Long live the next Beta!

@lawnjelly
Copy link
Member Author

Yup sure, you will tune a game to a particular physics tick rate, and you will definitely start getting crazy behaviour below 20 tps with physics engine 😀 , but it only needs to run enough temporarily to help identify interpolation problems.

But sure I do take your point that in some games the logic gets mucked up too much to test easily at very low tick rates. It is usually a good technique that will help troubleshoot.

Thinking about it, It's actually likely very possible to just jerry rig the OS timer to test this kind of thing without disrupting gameplay (although certain things like audio would be difficult to timeshift).

@oeleo1
Copy link

oeleo1 commented Aug 8, 2024

Thinking about it, It's actually likely very possible to just jerry rig the OS timer to test this kind of thing without disrupting gameplay (although certain things like audio would be difficult to timeshift).

Yes. Engine.set_time_scale() is the way to go. Audio put aside, values like 0.1 allow for testing the sequences of interest and FTI is doing its smoothing job very well without disturbing physics. Actually this is much better than lowering artificially the physics FPS setting.

@lawnjelly
Copy link
Member Author

Yes. Engine.set_time_scale() is the way to go. Audio put aside, values like 0.1 allow for testing the sequences of interest and FTI is doing its smoothing job very well without disturbing physics. Actually this is much better than lowering artificially the physics FPS setting.

I'll try and test this out and add to the docs, it could be a better suggestion than lowering the tick rate (in some situations at least).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants