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

Internal Events API #5552

Merged

Conversation

APickledWalrus
Copy link
Member

@APickledWalrus APickledWalrus commented Mar 25, 2023

Introduction

This PR aims to refactor and expand upon the existing Script Event API.

This PR does NOT implement backwards compatibility of the existing Script Event API. The API was marked as experimental, and I do not believe it has widely been used. However, backwards compatibility may be implemented if we see it as important.

Event Registry

A generic container for event instances. Events may be registered/unregistered. This simply serves for standardizing the event registration process across Skript. Below is an example of registering an event on ScriptLoader:

ScriptLoader.eventRegistry().register(ScriptLoader.ScriptUnloadEvent, (parser, script) -> {
		myLogger.log("The script '" + script + "' has been unloaded");
});

Events

Events have been added that can have handlers registered with ScriptLoader (global) and specific Script instances. To explain further, the example above registered an event with the ScriptLoader that logs when a Script has been unloaded. It would also be possible to register a handler for that event on a specific Script instance (to detect when that Script has been unloaded):

someScript.eventRegistry().register(ScriptLoader.ScriptUnloadEvent, (parser, script) -> {
		// logic for when this script is unloaded (e.g. cleaning up some data)
});

ScriptPreInitEvent

This event is a replacement for the Bukkit-based PreScriptLoadEvent. This event triggers right before the collection of Configs (pre-parsed script files) are loaded into actual Script instances. The collection is modifiable which allows the prevention of certain Configs from loading into Scripts.

ScriptInitEvent

This event is triggered right after a Script has been loaded into the ScriptLoader, but not yet fully initialized.

ScriptLoadEvent

This event is triggered right after a Script has finished loading. That is, all of its Structures have finished loading.

ScriptUnloadEvent

This event is triggered right before a Script is unloaded in the ScriptLoader. All data/Structures are still available.

ScriptActivityChangeEvent

This event is triggered when the activity status of a ParserInstance changes (resulting in a Script being made active/inactive). This would trigger through the usage of ParserInstance#setActive(Script) and ParserInstance#setInactive(). This could be useful for managing data on Scripts that should reset when it is not active in a ParserInstance.


Target Minecraft Versions: Any
Requirements: None
Related Issues: N/A

@APickledWalrus APickledWalrus added feature Pull request adding a new feature. 2.7 Targeting a 2.7.X version release labels Mar 25, 2023
@TheLimeGlass
Copy link
Collaborator

2.7?

Copy link
Contributor

@kiip1 kiip1 left a comment

Choose a reason for hiding this comment

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

What if we allow for EventRegisters to define a parent EventRegister? This would let Script's event register to define ScriptLoader's event register as parent (= get events will include the parent, so duplicate code can be prevented). This would require ScriptLoaderEvent to extend ScriptEvent (because of generics).

Example:

// Old
eventRegister.getEvents(ScriptLoaderEvent.ScriptUnloadEvent.class)
		.forEach(eventHandler -> eventHandler.onUnload(parser, script));
script.getEventRegister().getEvents(ScriptLoaderEvent.ScriptUnloadEvent.class)
		.forEach(eventHandler -> eventHandler.onUnload(parser, script));

// New
script.getEventRegister().getEvents(ScriptLoaderEvent.ScriptUnloadEvent.class)
		.forEach(eventHandler -> eventHandler.onUnload(parser, script));

Let me know if you have any questions :)

@APickledWalrus
Copy link
Member Author

What if we allow for EventRegisters to define a parent EventRegister?

Discussed on Discord a little, but this might be difficult for ScriptEvents as ScriptLoader will eventually get unique events that only apply to it. This class will be extendable by developers though if they would have a case for this kind of functionality.

@APickledWalrus APickledWalrus added 2.8 Targeting a 2.8.X version release and removed 2.7 Targeting a 2.7.X version release labels Apr 18, 2023
@APickledWalrus APickledWalrus marked this pull request as draft July 17, 2023 18:19
@APickledWalrus APickledWalrus marked this pull request as ready for review July 19, 2023 18:10
@Moderocky Moderocky force-pushed the master branch 3 times, most recently from bd134d0 to 3f08853 Compare September 16, 2023 16:59
@APickledWalrus APickledWalrus changed the base branch from master to dev/feature September 16, 2023 18:07
sovdeeth
sovdeeth previously approved these changes Sep 18, 2023
@APickledWalrus APickledWalrus removed the 2.8 Targeting a 2.8.X version release label Dec 30, 2023
@APickledWalrus APickledWalrus added the 2.9 Targeting a 2.9.X version release label Feb 9, 2024
@APickledWalrus APickledWalrus removed the 2.9 Targeting a 2.9.X version release label Jul 1, 2024
@APickledWalrus APickledWalrus marked this pull request as draft July 14, 2024 05:04
@APickledWalrus APickledWalrus changed the title Add EventRegister API Internal Event API Jul 14, 2024
@APickledWalrus APickledWalrus added the breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) label Jul 14, 2024
The event classes have been refactored as inner classes of the classes they concern. This is to help accomdate future API changes (e.g. to ParserInstance, ScriptLoader, etc.) where the events may need to change.
@APickledWalrus APickledWalrus marked this pull request as ready for review July 14, 2024 21:21
@APickledWalrus APickledWalrus dismissed stale reviews from TheLimeGlass and sovdeeth July 14, 2024 21:22

stale

@APickledWalrus APickledWalrus requested a review from sovdeeth July 14, 2024 21:22
@APickledWalrus APickledWalrus changed the title Internal Event API Internal Events API Jul 14, 2024
@sovdeeth sovdeeth added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Sep 15, 2024
@sovdeeth
Copy link
Member

conflicts need resolving!

@sovdeeth sovdeeth merged commit 751c102 into SkriptLang:dev/feature Oct 13, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) feature Pull request adding a new feature. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants