Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add a method to disable a hook's registration #4093

Merged
merged 4 commits into from
Jun 24, 2021

Conversation

APickledWalrus
Copy link
Member

Description

This PR adds a method to disable a hook's registration. This is mainly for addon developers looking to disable a hook.
This could be useful in the case of: skript-worldguard


Target Minecraft Versions: Any
Requirements: None
Related Issues: None

@APickledWalrus APickledWalrus added feature Pull request adding a new feature. hook Issues or Pull requests related to Skript's hook system labels Jun 20, 2021
@Romitou
Copy link
Member

Romitou commented Jun 20, 2021

Do you think it would be useful to throw an exception if we use the disableHookRegistration method to try to disable a hook after they have been registered?

@APickledWalrus
Copy link
Member Author

Do you think it would be useful to throw an exception if we use the disableHookRegistration method to try to disable a hook after they have been registered?

Possibly. This method will only be used in very specific cases so I am not sure if it's all too necessary as the developer should know what they're doing when using the method. I am open to hearing other thoughts on this too though.

@TPGamesNL
Copy link
Member

I think it'd be a good idea to throw an exception in that case: if an addon developer incorrectly uses the method (e.g. in onEnable instead of onLoad), the problem will be hard to debug without an error.

@Pikachu920
Copy link
Member

what about adding a corresponding option to the config? I think this is something that could be useful to expose to users too

src/main/java/ch/njol/skript/Skript.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/Skript.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/Skript.java Outdated Show resolved Hide resolved
@APickledWalrus
Copy link
Member Author

what about adding a corresponding option to the config? I think this is something that could be useful to expose to users too

Added. Please let me know if the config stuff is wrong as I haven't added any config options before. It did work when I tested it though.

Copy link
Contributor

@ShaneBeee ShaneBeee left a comment

Choose a reason for hiding this comment

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

looks good to me 👍

@FranKusmiruk FranKusmiruk merged commit 45f58fd into SkriptLang:master Jun 24, 2021
@APickledWalrus APickledWalrus deleted the feature/disable-hooks branch September 6, 2023 21:21
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
feature Pull request adding a new feature. hook Issues or Pull requests related to Skript's hook system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants