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

Safely removing running sequences #7

Merged
merged 6 commits into from
May 25, 2024
Merged

Conversation

stiv-iv
Copy link
Contributor

@stiv-iv stiv-iv commented Nov 1, 2023

Fixed some cases where one sequence in callback stops another sequence. Previously, this could modify table _runningSequences in a loop of Sequence.update().

Fixed some cases where one sequence in callback stops another sequence.
Previously, this could modify table _runningSequences in a loop of Sequence.update().
Fixed that: if you used sequence.time or sequence:get() in callback of sequence, you got not actual information from sequence.
Update print function sequence.print()
Fixed some cases, when you could get two same called callbacks in sequence (when cbObject.timestamp==clampedStart)
Floating point calculations lead to inaccuracies (some callbacks could be not called)
@Whitebrim
Copy link

@NicMagnier please find the time to review this pull request. The author says that when using the library, rare but serious bugs appear, and that he solved them in this PR.

rename some functions to be clearer
triggerCallbacks() behavior does not depending of logic when it is called (not fixing dt past call)
better clarifty for edge case in triggerCallbacksClampedTimeRange
clearer conversion to milliseconds
@NicMagnier
Copy link
Owner

Some good fix in there. I've refactor some part of the class to be make the changes clearer.
@stiv-iv can you check if this is still working with your test cases you had problem with?

For the future I would also recommend to make different PR because that helps fixing issue one at a time

@stiv-iv
Copy link
Contributor Author

stiv-iv commented May 21, 2024

@NicMagnier I have checked the latest changes with my test cases. Everything is fine.

These issues have been fixed:
#8
#9
#10

@NicMagnier NicMagnier merged commit 233cbcb into NicMagnier:main May 25, 2024
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