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

Timer Polishing #931

Merged
merged 3 commits into from
Nov 27, 2020
Merged

Timer Polishing #931

merged 3 commits into from
Nov 27, 2020

Conversation

CleanCut
Copy link
Member

@CleanCut CleanCut commented Nov 26, 2020

@amberkowalski @cart This is a followup to #914 -- I made pause functional, and polished up a couple other things I noticed:

  • Don't update the timer when tick() is called if currently paused.
  • Make getter method names consistent by naming them all after the fields without is_ prefixes.
  • Update tests to use getter methods.
  • Test pause functionality
  • Rename resume to unpause - this is somewhat arbitrary, but I feel it has better symmetry / discoverability when paired with pause. Thoughts?

@ambeeeeee
Copy link
Contributor

ambeeeeee commented Nov 27, 2020

This is what I get for not pushing my own fixes :P
Had them done already but wanted to polish

@CleanCut
Copy link
Member Author

@amberkowalski Oops, sorry! Did I miss anything? I'd be happy to add you as a collaborator on my fork, so you can push commits directly if you like.

@ambeeeeee
Copy link
Contributor

@amberkowalski Oops, sorry! Did I miss anything? I'd be happy to add you as a collaborator on my fork, so you can push commits directly if you like.

I can PR a timing example onto your fork if you don't mind, that's the main thing I was working on but had to fix some issues alongside it

@CleanCut
Copy link
Member Author

@amberkowalski Sounds great. I've invited you as a collaborator to my fork so you can push commits straight to this PR if you like, or you can chain your own PR. Whatever you like. 😄

@ambeeeeee
Copy link
Contributor

Just added my example and some notice in the changelog! Hopefully it all worked okay since I used it as an opportunity to try out some fancy git commands and features :)

@ambeeeeee
Copy link
Contributor

Interesting, so bevy requires that you develop on nightly...

@cart
Copy link
Member

cart commented Nov 27, 2020

Good work crew!

Rename resume to unpause - this is somewhat arbitrary, but I feel it has better symmetry / discoverability when paired with pause. Thoughts?

I dig it (for the reasons you mentioned).

@cart cart merged commit 12f29bd into bevyengine:master Nov 27, 2020
@fopsdev fopsdev mentioned this pull request Jan 24, 2021
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

Successfully merging this pull request may close these issues.

3 participants