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

Do not multiply delta_time by 60 in Sprite.update #2397

Open
bunny-therapist opened this issue Oct 8, 2024 · 5 comments
Open

Do not multiply delta_time by 60 in Sprite.update #2397

bunny-therapist opened this issue Oct 8, 2024 · 5 comments
Milestone

Comments

@bunny-therapist
Copy link

Enhancement request:

What should be added/changed?

Currently (I am on 3.0.0.dev36, but I see it still in master, delta_time is multiplied by 60 and there is a motivation:

        # NOTE: change_x and change_y (or velocity) are historically defined as
        # the change in position per frame. To convert to change in position per
        # second, we multiply by 60 (frames per second).
        # Users can define these values in any unit they want, but this breaks
        # compatibility with physics engines. Consider changing this in the future.
        delta_time *= 60

I would like this factor of 60 to be dropped so that e.g. speeds and similar can be defined in time units of seconds rather than frames.

I have been using arcade since major version 2, and there used to be one method update with fixed delta_time and one method on_update which took it as an argument, and I believe the docs encouraged use of on_update, so I did that. It became perfectly natural to define all speed etc with a time unit of second rather than a time unit of 1/60 second (otherwise I would have had to throw in an extra factor of 60 in every on_update). Now it seems like update and on_update have been merged into just update, with delta_time. I agree with just having one update method (and I am sure there was a reason for choosing the name update over on_update), but the choice of having this factor 60 means that user who used on_update (like me) run into problems. It also means that new users of arcade must use 1/60 s as the unit of time. It all seems very counter-intuitive and strange. There is a reference to "physics engines" breaking if this is changed, but I am not sure which physics engines are referred to (if it was just referring to physics engines in arcade, it should all be fixable?).

I am making this request to throw my vote in for dropping the factor of 60.

What would it help with?

I am currently handling this by overriding update with a fixed (no factor of 60) version. If I don't do this, I would have to divide all my speeds by 60 and all my accelerations by 60^2. I could do this, but using 1/60 s as the unit of time feels counter-intuitive. I also believe this is an extra complication for new users (unless 1/60 s is somehow a common unit of time in games? What does e.g. pygame do?)

@einarf einarf added this to the 3.1 milestone Oct 8, 2024
@einarf
Copy link
Member

einarf commented Oct 8, 2024

That is the goal in the long run, but not in 3.0. We still have to obey velocity being in "pixels per frame at a framerate of 60" due many factors. If you want to change this default just override the update() method. Assigning this to 3.1 and we'll see where things go. There is a lot of physics stuff that needs to be resolved here we well. Work in that will start soon.

@bunny-therapist
Copy link
Author

Ok, that's fine by me. (Though I worry that it sounds like a mild breaking change between 3.0 and 3.1 which might affect new users, so unless 3.1 comes quickly, there may be a need for some documentation or warnings. But I leave that to you.)

@einarf
Copy link
Member

einarf commented Oct 9, 2024

It's definitely not a perfect situation. We just need to figure it out later or we will never get this release out 😆

There is always 3.x and 4.x.

@alejcas
Copy link
Member

alejcas commented Oct 22, 2024

There's a problem with the current approach as if the frame rate decreases to 30 (for example) the sprite will move way further that it is intended to.
I run into this today and had to remove this delta_time *= 60 line and move to pixels per second.
You can rate limit the upper bound... not the lower one.

@einarf
Copy link
Member

einarf commented Oct 25, 2024

So tempted to do something about this asap.

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

No branches or pull requests

3 participants