Skip to content

Structure API #4108

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

Merged
merged 140 commits into from
Oct 3, 2022
Merged

Structure API #4108

merged 140 commits into from
Oct 3, 2022

Conversation

TPGamesNL
Copy link
Member

@TPGamesNL TPGamesNL commented Jun 24, 2021

Description

This PR adds a new Structure API that is used for creating and registering top-level elements. It also includes a Structure-specific entry parsing system to simplify the usage of entries (like in command structures).

A priority system is also included to determine the order in which structures should initially load. This is particularly useful for things like function definitions.

Also included is a new Script object, with the goal of creating a consistent object to be used across Skript's loading and unloading processes. The Script object includes things such as events and script-specific data values. The "script warning" system has also been moved to this object. Many places across Skript where Config or File were used have been refactored to use this new Script object.

ScriptLoader and SkriptEventHandler have had methods reduced and removed in an effort to promote system isolation / more "softcoding" of systems.

It converts and improves Skript's current structures:

  • Aliases
    • Makes use of the script-data system to store script aliases
  • Commands (makes use of the new Structure entry system)
    • Makes use of the new Structure entry system
    • Command reloading handling has been moved/isolated to the new Command Structure. Commands are now synced on load AND unload (also fixes an issue that commands wouldn't be synced when a script was unloaded)
  • Functions
    • Function unloading has been moved/isolated to the new Function Structure
  • Options
    • Makes use of the script-data system to store script options
    • Moved/isolated as much of the system as possible into the new Option Structure. An additional PR will be needed in the future (see TODO comment on ScriptLoader#replaceOptions)
  • Variables

Other notable changes:

  • Better documentation and javadocs across updated areas
  • SkriptEventHandler is truly only for handling events now. It has been removed from classes like ScriptLoader. Yay for isolation!
  • Improved error handling and printing in ScriptLoader
  • Doc annotations for all the new Structures
  • ScriptInfo has been significantly reduced, as it only tracks files and structures now. All lang messages have been updated accordingly.
  • SkriptEvent has also been converted into a Structure. All event parsing, registration, and unregistration logic has been moved to SkriptEvent. SelfRegisteringTrigger methods in SkriptEventHandler have been deprecated as they are no longer necessary. The registerBukkitEvents() method has been replaced with more specific methods to be called by SkriptEvent during the registration process.
  • ParserInstance#getCurrentScript now returns Script instead of Config.
  • ParserInstance#getCurrentOptions has been deprecated. The method will still function properly though.
  • `ParserInstance#onCurrentScriptChange(Config) has been deprecated.
  • Functions#clearFunctions has been deprecated.
  • The class ScriptOptions has been removed as it was integrated into Script.
  • Trigger#getScript now returns Script instead of File.
  • SkriptParser#parseEvent(String, String) has been removed.
  • Commands#loadCommand(SectionNode) and Commands#loadCommand(SectionNode, boolean) have been removed.
  • Commands#unregisterCommands(Script) has been deprecated.
  • ScriptLoader has had unloading and reloading methods that use File parameters deprecated.
  • ScriptLoader has had its loading methods reworked and additional ones added. The method used to load all scripts within the scripts folder has been generalized. Behavior has been moved within the Skript class itself.
  • A method has been added to the Skript class to get a File object representing the scripts folder.
  • SkriptCommand and EffScriptFile have been reworked to use proper ScriptLoader methods.
  • Added static fields to ScriptLoader representing the disabled scripts prefix (currently -)
  • Made loadStructure methods within ScriptLoader private
  • Improved SkriptCommandTabCompleter to use better ScriptLoader methods & autocompletion
  • Reworked the currentEvents system to always require an event. A new ContextlessEvent has been added for when there is no specific event(s) to take into account when parsing. Essentially, instead of using null, ContextlessEvent should be used.
  • Deprecated SelfRegisteringSkriptEvent#afterParse(Config) as Structure#postLoad now exists.
  • Added new ParserInstance Activity Status - this is to promote the idea that ParserInstance is meant for loading/parsing/unloading
  • ... and a whole bunch of other changes! it's kind of impossible to list everything, so please feel free to read the commits!

Target Minecraft Versions: any
Requirements: none
Related Issues:

Fixing:

Non-Fixing:

@TPGamesNL TPGamesNL marked this pull request as ready for review June 24, 2021 13:27
@TPGamesNL TPGamesNL added the feature Pull request adding a new feature. label Jun 24, 2021
APickledWalrus
APickledWalrus previously approved these changes Jun 24, 2021
Copy link
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

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

Just the two questions. Implementation looks pretty good to me overall. Nice job ;D

@APickledWalrus
Copy link
Member

I just realized - would it be possible to create a Structure for events? I suppose their pattern would kind of match everything though - so maybe it would not be worth the trouble.

@TPGamesNL
Copy link
Member Author

It would be possible, but I don't think it would improve anything, so I did not do it.

Copy link
Contributor

@Mwexim Mwexim left a comment

Choose a reason for hiding this comment

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

Very neat changes! This is something I will probably adapt in skript-parser as well! The system is quite flexible without being too clunky.

@TPGamesNL TPGamesNL marked this pull request as draft July 14, 2021 13:46
@TPGamesNL TPGamesNL marked this pull request as ready for review July 14, 2021 16:02
@TPGamesNL
Copy link
Member Author

I've added a priority system to preloading structures, and also fixed #2492.
@Mwexim could you look at it?

Copy link
Contributor

@Mwexim Mwexim left a comment

Choose a reason for hiding this comment

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

There are still some things that need to be addressed since the current system is not powerful enough to support add-on developers.

@TPGamesNL
Copy link
Member Author

SkriptEvents are now also Structures

@TPGamesNL TPGamesNL marked this pull request as draft November 4, 2021 13:35
@TheLimeGlass
Copy link
Contributor

This issue was directed to this pull request #4246

Has this been addressed in this pull request? Yes -> This issue can be referenced. No -> The PR available tag can be removed on the issue in question.

@TPGamesNL
Copy link
Member Author

See #4252 (comment) and 58d70d2, it's been included, I've updated the description

@APickledWalrus APickledWalrus mentioned this pull request Aug 21, 2022
- Ensures entry data constructors funnel to 1 main constructor
- Fixes some minor formatting errors
- Fixes an issue where Literals did not work with ExpressionStructureEntryData and adds the ability to specify the SkriptParser flags to use
@APickledWalrus
Copy link
Member

The entry data system has been made to work with all SectionNode's - not just those representing a Structure (top-level SectionNodes)

There wasn't any real reason to enforce this, as the system has no real dependency on Structure. This makes it more accessible for developers who may wish to combine it with something like the Section API

Copy link
Member

@AyhamAl-Ali AyhamAl-Ali left a comment

Choose a reason for hiding this comment

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

Reviewed all the changes after my last RC and this looks 🔥
LGTM STICKER

@APickledWalrus APickledWalrus dismissed Pikachu920’s stale review October 3, 2022 19:37

Concerns were addressed

@APickledWalrus APickledWalrus merged commit aa62e89 into master Oct 3, 2022
@APickledWalrus APickledWalrus deleted the feature/structure-api branch October 3, 2022 19:47
Anarchick added a commit to Anarchick/skript-packet that referenced this pull request Jun 4, 2023
Since Skript 2.7.0-beta1 , getCurrentScript return Script instead of Config
SkriptLang/Skript#4108
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants