Skip to content

Add Scripts Loading/Unloading Events - #8816

Open
APickledWalrus wants to merge 2 commits into
dev/featurefrom
feature/scripts-events
Open

Add Scripts Loading/Unloading Events#8816
APickledWalrus wants to merge 2 commits into
dev/featurefrom
feature/scripts-events

Conversation

@APickledWalrus

@APickledWalrus APickledWalrus commented Aug 5, 2026

Copy link
Copy Markdown
Member

Problem

It is not currently possible to detect when any script load/unload occurs (i.e. not the script itself loading/unloading).

Solution

Adds two new ScriptLoader events for "bulk" loads/unloads. This includes the first script load (on server start).
I've also added a new ScriptModule with events to listen to these two ScriptLoader events, with support for obtaining the loaded scripts given the script reflection experiment is enabled.

Testing Completed

basic test + more extensive in-game testing

Supporting Information

There are some parts that need further consideration:

  • I don't think the syntax is particularly good. I would like suggestions.
    • One problem is that loaded scripts is not really possible as that already exists, and there needs to be a way to distinguish between the two.
  • Should this event occur before or after individual script load/unload events?
  • Should it be possible to get the loaded/unloaded scripts as strings if the experiment is not enabled?
  • These events are still using SkriptEvent. This is not good and should be changed (which I may do in this PR, though we might prefer a utility class in the future (e.g. SkriptEvent but for our API events)).

Completes:

Related: none
AI assistance: none

@APickledWalrus
APickledWalrus requested a review from a team as a code owner August 5, 2026 20:06
@APickledWalrus APickledWalrus added the feature Pull request adding a new feature. label Aug 5, 2026
@APickledWalrus
APickledWalrus removed the request for review from a team August 5, 2026 20:06
@skriptlang-automation skriptlang-automation Bot moved this to In Review in 2.17 Releases Aug 5, 2026
@skriptlang-automation skriptlang-automation Bot added the needs reviews A PR that needs additional reviews label Aug 5, 2026
@skriptlang-automation

skriptlang-automation Bot commented Aug 5, 2026

Copy link
Copy Markdown

🔍 Triage Checklist

Type

  • bug
  • feature
  • enhancement

Project

  • 2.16 Releases
  • 2.17 Releases

  • ✅ Mark as triaged

@APickledWalrus APickledWalrus linked an issue Aug 5, 2026 that may be closed by this pull request
1 task

@Efnilite Efnilite left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think the current order for calling individual/batch events makes sense, as long as it's documented. looks good 🥇

public static void register(SyntaxRegistry syntaxRegistry) {
syntaxRegistry.register(SyntaxRegistry.EXPRESSION,
SyntaxInfo.Expression.simple(ExprLoadingScripts.class, ExprLoadingScripts::new, Script.class,
"[all [[of] the]|the] (loading|initializing|enabling) scripts"));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe you could change the existing loaded script syntax class such that writing loaded scripts uses the scripts from this event?

@APickledWalrus APickledWalrus Aug 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah I just wasn't sure how to handle someone wanting to obtain all scripts too.

Comment on lines +35 to +36
}
@Override

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
}
@Override
}
@Override

Comment on lines +35 to +36
}
@Override

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
}
@Override
}
@Override

syntaxRegistry.register(BukkitSyntaxInfos.Event.KEY, BukkitSyntaxInfos.Event.builder(EvtScripts.class, "Scripts Loading")
.supplier(() -> new EvtScripts(true))
.addEvent(ScriptsLoadEvent.class)
.addPattern("scripts (loading|initializing|enabling)")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think adding a distinction between all scripts and a batch of scripts may increase clarity here. also removes the need for the user to check whether the event contains all scripts. this may also allow this class and EvtScript to be merged which might be nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Pull request adding a new feature. needs reviews A PR that needs additional reviews

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

Add event for when all scripts finish loading

2 participants