-
Notifications
You must be signed in to change notification settings - Fork 24
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
Use Position and Rotation instead of GlobalTransform (Based off PR #61) #81
base: main
Are you sure you want to change the base?
Conversation
@@ -22,6 +22,20 @@ use bevy_tnua_physics_integration_layer::*; | |||
/// Add this plugin to use avian2d as a physics backend. | |||
/// | |||
/// This plugin should be used in addition to `TnuaControllerPlugin`. | |||
/// Note that you should make sure both of these plugins use the same schedule. | |||
/// This should usually be `PhysicsSchedule`, which by default is `FixedUpdate`. |
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.
Isn't the PhysicsSchedule
a schedule of its own, separate from the FixedUpdate
?
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.
Not sure what @janhohenheim meant by PhysicsSchedule being FixedUpdate by default
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.
Maybe that the default in Avian is FixedUpdate
but usually you'd want to use PhysicsSchedule
?
This is kind of important, both because the documentation needs to be correct and because it affects the ScheduleToUse::PhysicsSchedule
option in the demos. I'll try to ping him on Discord.
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.
Hey hey everyone. I was not active much in the Bevy space for some months due to academia, so what I'm writing now might not be accurate. But as I remember it, I was looking at PhysicsSchedule
as basically a generic label / wrapper / "variable" for the actual schedule. What the actual schedule was, I determined by looking at PhysicsSchedulePlugin .schedule
which back then was FixedUpdate
by default (now FixedPostUpdate
).
It's very very possible that I misunderstood / misunderstand how the schedule hierarchy precisely works in Bevy, so if you think this sounds somewhat wrong, it probably is :D
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.
That's weird. I thought the point of the PhysicsSchedule
was supposed to run in its own frequency, which means its runner was supposed to run it multiple times, or not at all, depending on how much time has passed since the last time it was ran. In other words - even if it runs in PostUpdate
the PhysicsSchedule
itself still gets fixed frame time.
But not it looks like PhysicsSchedule
is mostly about not running it if Avian is paused and about replacing the Time
resource while it runs?
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.
Pinging @Jondolf, he surely knows best :)
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.
@idanarye The point of the PhysicsSchedule
is just to advance the whole physics simulation by one step, using the delta time stored in the Time
resource. The schedule is independent of how often and in what way it is run; you could run it with a fixed time step, a variable time step, or even run it manually for e.g. networking purposes.
Avian 0.2 did indeed change how the default scheduling is set up. Previously, Avian managed its own fixed time step in PostUpdate
, but this had unnecessary complexity, footguns (ex: duplicate fixed time steps when running physics in FixedUpdate
, and having to use Time::fixed_once_hz
), several bugs, and it was overall strange to emulate a custom fixed time step when Bevy already has a first-party one. Running physics after Update
was also kind of strange; both Unity and Godot run it before, near the end of FixedUpdate
/_physics_process
.
With the new scheduling, the PhysicsSchedule
is run in FixedPostUpdate
by default, making FixedUpdate
the natural place for most user gameplay logic and movement systems, except of course for things like camera movement or interpolation which should typically run every frame for smooth and responsive results.
You can still run physics in e.g. PostUpdate
too, but it will use a variable time step with Time<Virtual>
since that is the active time resource there. And of course custom scheduling is possible too for additional control.
The new scheduling and rationale is covered in all of these:
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.
My scheduling goals are:
- I want Tnua's internal systems to get the correct frame duration - that is, the same one Avian uses - because it's crucial for their calculations.
- I prefer Tnua's internal systems to run in lockstep with Avian's physics simulation, so that they can respond to the position and velocity changes Avian make.
- User control systems don't need to use Avian's frame duration - but they do need to run in lockstep with Tnua's systems because of how actions work.
Base on these goals and your description, I'd say the recommended way is to have both TnuaControllerPlugin
and TnuaAvian#dPlugin
use PhysicsSchedule
and to put the user control systems under FixedUpdate
?
Also, regarding the demos, this means that ScheduleToUse::PhysicsSchedule
is meaningless. Instead, for Avian, ScheduleToUse::Update
should register Avian itself under PostUpdate
and the user control systems under Update
while ScheduleToUse::FixedUpdate
should register Avian itself under FixedPostUpdate
and the user control systems under FixedUpdate
- and either way the Tnua plugins should be registered under PhysicsSchedule
.
demos/src/bin/platformer_2d.rs
Outdated
ScheduleToUse::PhysicsSchedule => { | ||
app.add_plugins(TnuaControllerPlugin::new(PhysicsSchedule)); | ||
app.add_plugins(TnuaCrouchEnforcerPlugin::new(PhysicsSchedule)); | ||
app.add_plugins(TnuaControllerPlugin::new(FixedUpdate)); |
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.
Wait what?
I thought Avian's PhysicsSchedule
is a schedule of its own which has its frequency controlled by Avian. Was it changed to just be equivalent to the FixedUpdate
? I don't see anything about it in the release notes.
Also, if PhysicsSchedule
really does mean the exact same thing as FixedUpdate
- to the point where when we run the demo with PhysicsSchedule
it gets switched to FixedUpdate
behind our back - maybe we should just remove ScheduleToUse::PhysicsSchedule
?
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.
I think I messed up when I rebased, I reverted it
Fixed demos compilation errors
No stuttering when tested with lightyear