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

Throw error if beforeSave or beforeClose yield #64

Merged
merged 3 commits into from
Aug 21, 2024

Conversation

nezuo
Copy link
Owner

@nezuo nezuo commented Aug 21, 2024

This fixes an edge case where one of the callbacks could yield right before or during game:BindToClose which means the save won't have started and AutoSave won't wait for the document to close.

This could be fixed without restricting yielding but there is another awkward API issue this fixes: For example, if beforeSave yields and :close is called during the yield, the close will throw an error since :close can't be called during the beforeSave callback. This could also technically be fixed by checking the coroutine.status of the beforeSave thread before throwing this error. However, if the :close didn't yield in its callbacks the close request would happen before the save and that would result in a save happening after the close which is not good.

We may find solutions to these edge cases but I feel that it's more correct to not allow yielding.

@nezuo nezuo merged commit 324e1bb into master Aug 21, 2024
@nezuo nezuo deleted the no-yield-before-callbacks branch August 21, 2024 06:02
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.

1 participant