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

Prevent setting the repeat time on times to zero #8477

Open
alice-i-cecile opened this issue Apr 23, 2023 · 6 comments · May be fixed by #8591
Open

Prevent setting the repeat time on times to zero #8477

alice-i-cecile opened this issue Apr 23, 2023 · 6 comments · May be fixed by #8591
Labels
A-Time Involves time keeping and reporting C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@alice-i-cecile
Copy link
Member

          I would prefer to make the issue impossible by types, as this means the check will happen several times. Someone doing lots of timers and calling `percent` on every frame may notice an impact... but we can revisit that then

Originally posted by @mockersf in #8467 (review)

@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Time Involves time keeping and reporting labels Apr 23, 2023
@Connor-McMillin
Copy link
Contributor

Should this be a compile time check or a runtime check? I think either way it should be possible to make a NonZeroDuration to hold this logic

@alice-i-cecile
Copy link
Member Author

Ideally compile time :)

@Connor-McMillin
Copy link
Contributor

Connor-McMillin commented May 11, 2023

Here are two possible ways of solving the problem. In my opinion the compile time check is way too hacky to merge, but I thought I would throw it out there in case anyone has a clever way to resolve my hack and make compile time checks actually work. However I think that the runtime check is a net improvement. It does move the problem to NonZeroDuration construction, but it ensures that we do not continually check for zero duration in tick() and percent

@Connor-McMillin
Copy link
Contributor

Its also probably worth discussing what the default Duration should be in NonZeroDuration

@Connor-McMillin
Copy link
Contributor

@alice-i-cecile / @mockersf what are your thoughts on the runtime check PR?

@alice-i-cecile
Copy link
Member Author

Yeah, lemme do a review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Time Involves time keeping and reporting C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
2 participants