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

Support Time<Real> (in bevy 0.12) #106

Open
SecretPocketCat opened this issue Oct 28, 2023 · 10 comments
Open

Support Time<Real> (in bevy 0.12) #106

SecretPocketCat opened this issue Oct 28, 2023 · 10 comments
Labels
enhancement New feature or request

Comments

@SecretPocketCat
Copy link
Contributor

This's is just a prep issue for the upcoming bevy 0.12 release.
Time scaling was added in bevyengine/bevy#8964 with Time<()> being the default. In normal systems that's Time<Virtual> (scaled) which makes sense, but it would be nice to be able to animate in unscaled time (Time<Real>) (e.g. for a pause UI).

Adding a field to Animator like use_real_time: bool and adding the appropriate builder functions might do the trick for the API.

@SecretPocketCat SecretPocketCat changed the title Support Time<Real> Support Time<Real> (in bevy 0.12) Oct 28, 2023
@djeedai
Copy link
Owner

djeedai commented Oct 28, 2023

I see how that can be useful, however I have a few questions (not necessarily to you):

  • The default setup in Bevy will use Time<Virtual> in Update. It's unclear to me what's the recommendation for systems which want to use another time. Should they still be inside Update and go against the mental model of "all systems in Update are on virtual time"? This is a general question beyond this library.
  • It's unclear to me whether using a bool is the best answer. What if someone wants to use another Time<T>? Should the animator instead be generic over its time source? Or use a trait for it?
  • Do we need that at all? The animator can already be paused etc. so maybe it should always use Time<Real>? That's less convenient for things like game pausing though, as you have to manually pause all animators.

@djeedai
Copy link
Owner

djeedai commented Oct 28, 2023

I should add that my experience with Unity has been that time scaling is seldomly used, as it's too broad. If you want to implement a "pause system" (generic term here for anything that needs to tick only sometimes) then it's often better to do that manually with a custom time source that you manage, as opposed to an engine built-in one which has various assumptions as to what does and doesn't pause. That being said the Bevy API is more extensible so maybe we'll see broader usage.

@djeedai djeedai added the enhancement New feature or request label Oct 28, 2023
@SecretPocketCat
Copy link
Contributor Author

  • The default setup in Bevy will use Time<Virtual> in Update. It's unclear to me what's the recommendation for systems which want to use another time. Should they still be inside Update and go against the mental model of "all systems in Update are on virtual time"? This is a general question beyond this library.

Time<Virtual> is just the default in update systems, but the system can still explicitly use Real, Virtual or whatever (Fixed or custom) other kind of Time<T> resource. Scaling the time by invoking set_relative_speed actually requires using Virtual time.

  • It's unclear to me whether using a bool is the best answer. What if someone wants to use another Time<T>? Should the animator instead be generic over its time source? Or use a trait for it?

The bool (or enum) was just a suggestion for the simplest approach with the least breaking changes that would still cover most use cases.
Being generic over Time<T> would be the most flexible, but the users would have to schedule more systems which could lead to some confusion. Then again, maybe the generic param could default to () same way Time<T = ()> and only using other kinds of time would require the extra system scheduling, so that might be OK and actually less breaking changes then adding a new field.

  • Do we need that at all? The animator can already be paused etc. so maybe it should always use Time<Real>? That's less convenient for things like game pausing though, as you have to manually pause all animators.

I'd venture a guess that would be confusing for end users if the default time for tweening would be different from default bevy time.

@SecretPocketCat
Copy link
Contributor Author

I should add that my experience with Unity has been that time scaling is seldomly used, as it's too broad. If you want to implement a "pause system" (generic term here for anything that needs to tick only sometimes) then it's often better to do that manually with a custom time source that you manage, as opposed to an engine built-in one which has various assumptions as to what does and doesn't pause. That being said the Bevy API is more extensible so maybe we'll see broader usage.

I've done just jam games and in those cases having the basic time scaling was good enough. For bevy jam games I've used a bare-bones resource wrapped in a system param, but I'm gonna switch to the new time API which is indeed much nicer than the Unity one.

I wouldn't mind tackling this, but only if you think it would make sense for this plugin and point me in the proper direction the impl should take (ie. generic vs enum/bool field) 🙂.

@SecretPocketCat
Copy link
Contributor Author

Since support for bevy 0.12 has been merged, I've messed around with the generic approach, but I've encountered an issue:
It's unclear whether to schedule animator systems with () or Virtual for default/virtual time and seems like it could cause a bit confusion.

@SecretPocketCat
Copy link
Contributor Author

SecretPocketCat commented Nov 9, 2023

I've settled on a marker trait to exclude (). Here's a WIP branch that implements this for Animator (no assets and docs have to be updated too and new tests added).

@djeedai Any thoughts? I can close this if you don't think it would be useful and I'll just use it in my fork.

@brandon-reinhart
Copy link

brandon-reinhart commented Jan 30, 2024

I'd like to support this change! I can't use bevy_tweening at the moment because my game uses Time Virtual for gameplay and Time Real for UI. I need the ability to continue to tween over Time Real even if the game is paused Time Virtual or be able to specify this on a per-tween basis.

fwiw, I don't think this is an enhancement, but a requirement given how Bevy's Time resource works in 0.12+

@SecretPocketCat
Copy link
Contributor Author

I'm still down to finish this if @djeedai would consider it a worthy addition. Otherwise I'm just gonna close it as a stale issue.

@djeedai
Copy link
Owner

djeedai commented Jan 30, 2024

I mean, to be clear it seems a very good addition to be supporting the various Time<T> for tweening. I have no objection on that, I'm convinced this is the right thing to do, and there's plenty of use cases. I'm just not sure about the best design to do that.

@SecretPocketCat I had a look at your WIP branch, that looks a priori like a good start. I think it's maybe just missing to confirm the default U = Virtual works, because I think all examples you changed explicitly specify the time (or I missed something)? Otherwise that looks good for a PR I think, thanks!

@SecretPocketCat
Copy link
Contributor Author

Great, I will finish the PR then including a couple new tests, docs & adding the same to assets. I'm a bit busy at the moment, but I'll come back to it when I get the chance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants