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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix regression in set_timer, old events not removed #2769

Merged
merged 1 commit into from Oct 16, 2021

Conversation

ankith26
Copy link
Contributor

Fixes #2763

This was a minor regression introduced in my older event refactor PRs that got merged in 2.0.1

Setting a timer for an event should discard older timers for the same event. While this behavior was not previously documented, this was the standard behaviour for a long time until 2.0.1 when my changes inadvertently changed behaviour here. (Funnily, I had documented this behaviour in the same PR I broke it 馃槄, I remember having tested it, but it somehow worked back then due to some test weirdness/randomness in the broken behaviour)

While having multiple timers for the same event running independently sounds like a good idea, this fix is needed for backwards compatibility of older code using this function.

This PR also updates the tests for this function.

Also what do y'all think of a keyword argument for this function, something like remove_old_timer=True, which can be set to false if someone wants to place in an event on the timer and not remove the older one?

@ankith26 ankith26 added this to the 2.0.3 milestone Oct 12, 2021
@illume illume added the time pygame.time label Oct 16, 2021
Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. Especially for adding those tests :)

@illume illume merged commit bdd2c96 into main Oct 16, 2021
@illume illume deleted the ankith26-set-timer-fix branch October 16, 2021 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set_timer not working
2 participants