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

Add on leave to quit event (#3789) #3799

Merged

Conversation

TheDGOfficial
Copy link
Contributor

Description

.. as an alternative to quit, disconnect and logout.

Will handle leave, leaving and their variants with "on" prefix. (ScriptLoader adds that, no need to specify here)

Not sure if tests are required for a change like this; not familiar with the testing system but can add one that ensures on quit, on disconnect, on logout and on leave parses successfully (if that is possible).


Target Minecraft Versions: any
Requirements: none
Related Issues: #3789

.. as an alternative to quit, disconnect and logout.

Will handle leave, leaving and their variants with "on" prefix. (ScriptLoader adds that, no need to specify here)
@TPGamesNL TPGamesNL added 2.5 enhancement Feature request, an issue about something that could be improved, or a PR improving something. labels Mar 8, 2021
@Pikachu920
Copy link
Member

Pikachu920 commented Mar 15, 2021

just thinking out loud I'm not sure if this is actually a good add because on connect and on login are separate things, so maybe it would be confusing to have on disconnect and on logout be the same? i'm thinking that this may be the reason that disconnect isn't an option in the first place.

edit: nevermind my reading comprehension failed me

@Whimsyturtle Whimsyturtle merged commit eff6134 into SkriptLang:master Mar 22, 2021
@TheDGOfficial TheDGOfficial deleted the add-on-leave-to-quit-event branch April 12, 2021 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants