-
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
Open
Alpyg
wants to merge
15
commits into
idanarye:main
Choose a base branch
from
Alpyg:use-position-rotation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+197
−65
Open
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
a0743b8
Remove default schedule
janhohenheim d890cd9
Use physical transform instead of rendered
janhohenheim 31dafba
Patch to newest avian
janhohenheim 6afb0a6
Add docs
janhohenheim 2c955b5
Update 2D and examples
janhohenheim 3bc32f8
Fix various fixed timestep issues
janhohenheim a10433c
Revert removal of running demos in physics schedule
janhohenheim 5492484
Make examples a bit more idiomatic
janhohenheim 0f28eb6
Use direct into impl
janhohenheim e8425b0
Remove useless line
janhohenheim e4d1d3b
Fix compile error
janhohenheim 6099a73
Run cargo fmt
janhohenheim c4d7909
Merge branch 'main' into fix-minor-avian-stuff
janhohenheim 009e3d1
fix demos
Alpyg 152fce4
Use right schedule
Alpyg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 theFixedUpdate
?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 usePhysicsSchedule
?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 atPhysicsSchedulePlugin .schedule
which back then wasFixedUpdate
by default (nowFixedPostUpdate
).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 inPostUpdate
thePhysicsSchedule
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 theTime
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 theTime
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 inFixedUpdate
, and having to useTime::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 afterUpdate
was also kind of strange; both Unity and Godot run it before, near the end ofFixedUpdate
/_physics_process
.With the new scheduling, the
PhysicsSchedule
is run inFixedPostUpdate
by default, makingFixedUpdate
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 withTime<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:
FixedPostUpdate
by default and simplify scheduling Jondolf/avian#457There 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:
Base on these goals and your description, I'd say the recommended way is to have both
TnuaControllerPlugin
andTnuaAvian#dPlugin
usePhysicsSchedule
and to put the user control systems underFixedUpdate
?Also, regarding the demos, this means that
ScheduleToUse::PhysicsSchedule
is meaningless. Instead, for Avian,ScheduleToUse::Update
should register Avian itself underPostUpdate
and the user control systems underUpdate
whileScheduleToUse::FixedUpdate
should register Avian itself underFixedPostUpdate
and the user control systems underFixedUpdate
- and either way the Tnua plugins should be registered underPhysicsSchedule
.