Skip to content

Conversation

erenkarakal
Copy link
Member

Problem

The problem is explained in detail in #2028, but the general idea is that having a global options file makes it easier to change static values across lots of script files.

This PR also fixes another issue, using the code below:

on load:
  broadcast {@hello}

{@hello} is parsed as a variable, because the script doesn't contain the options structure.
Therefore, it doesn't throw the unknown option error.

Solution

Added a new global-options.sk file inside /plugins/Skript/ folder. The entries inside are loaded into a static map.
Whenever replaceOptions is called, it checks if the script has that option first and returns it. If it doesn't, it gets the option from the static map instead. This lets scripts override global options.
You can also use sections just like the structure version.

Testing Completed

Only manual testing, unit tests will be added after reviews when it's finalised

TODO (after finalised)

  • Unit test
  • Translations for other languages

Completes: #2028
Related: none

@erenkarakal erenkarakal requested review from a team as code owners September 16, 2025 09:37
@erenkarakal erenkarakal requested review from APickledWalrus and Absolutionism and removed request for a team September 16, 2025 09:37
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label Sep 16, 2025
@erenkarakal erenkarakal added enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature Pull request adding a new feature. and removed enhancement Feature request, an issue about something that could be improved, or a PR improving something. labels Sep 16, 2025
@erenkarakal erenkarakal linked an issue Sep 16, 2025 that may be closed by this pull request
Comment on lines +175 to +177
reloading(sender, "global options", logHandler);
GlobalOptions.load();
reloaded(sender, logHandler, timingLogHandler, "global options");
Copy link
Member

Choose a reason for hiding this comment

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

should also send a warning that the users should reload all scripts using global options.

Comment on lines +86 to +96
private static void loadOptions(SectionNode sectionNode, String prefix, Map<String, String> options) {
for (Node node : sectionNode) {
if (node instanceof EntryNode) {
options.put(prefix + node.getKey(), ((EntryNode) node).getValue());
} else if (node instanceof SectionNode) {
loadOptions((SectionNode) node, prefix + node.getKey() + ".", options);
} else {
Skript.error("Invalid line in options");
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

can we make this public so we don't have duplicated code between Struct and GlobalOptions

# This file lets you set options globally.
# Options here will be available across all scripts.
# Scripts can override a global option by creating an option with the same name as the global one.
# When you edit this file, you must reload global-options and all script files that use global options.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# When you edit this file, you must reload global-options and all script files that use global options.
# When you edit this file, you must first reload global-options and then reload all the script files that use global options.

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
None yet
Development

Successfully merging this pull request may close these issues.

[Suggestion] Global Options File
2 participants