-
-
Notifications
You must be signed in to change notification settings - Fork 354
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
New Registration API #6246
base: dev/feature
Are you sure you want to change the base?
New Registration API #6246
Conversation
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
They are permitted if a creation supplier is provided
Added a default SkriptAddon origin and pivoted internally to avoid using an "unknown" origin
An implementation of Priority that enables positioning itself around specific SyntaxElement classes.
*/ | ||
public final class SkriptAddon { | ||
@Deprecated | ||
@ApiStatus.NonExtendable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to bother with this annotation if all of the constructors are package-private, nothing could extend it anyway.
* A class containing the interfaces representing Bukkit-specific SyntaxInfo implementations. | ||
*/ | ||
@ApiStatus.Experimental | ||
public final class BukkitInfos { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of this class name, do you think we could call it something a bit nicer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extremely good work, Pickle, just two more questions
private final Collection<String> patterns; | ||
private final Priority priority; | ||
|
||
protected SyntaxInfoImpl( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to allow syntaxes to be registered with no patterns (empty list)?
if (aPriority instanceof SyntaxPriority) { | ||
SyntaxPriority priority = (SyntaxPriority) aPriority; | ||
if (priority.beforeElements().contains(b.type())) { // a must be before b | ||
return -1; | ||
} | ||
// TODO improve: not ideal, but we just stick it at the end to make sure it comes after everything | ||
return 1; | ||
} | ||
|
||
if (bPriority instanceof SyntaxPriority) { | ||
SyntaxPriority priority = (SyntaxPriority) bPriority; | ||
if (priority.afterElements().contains(a.type())) { // b must be after a | ||
return -1; // returning that a must be before b | ||
} | ||
// a does not have a relationship with b, allow it to keep moving up | ||
return 1; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if a is before c, and b is after a? When a and b are compared, a has a syntax priority, but not related to b, so it hits the first return 1
and gets put after, when it really should be before b. Am I missing some symmetry requirement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah this needs reworked 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a graph maybe? 🥺
It is too unstable in its current state. It may return in a future PR.
For registration purposes, SkriptEvents are no longer tied to Structures (see StructEvent)
Description
This is built from the work done in #5331 (thanks @kiip1)
This PR is the initial effort to build a modern API for Skript. All new API has been marked as experimental. Old API has been deprecated (though if we merge this API as experimental, we may not want to deprecate old API yet). Backwards compatibility has been kept.
The main goal of this PR is to introduce new API for many of Skript's registration processes. Specifically, this PR tackles addon and syntax registration.
There is still work to be done here, but I think it is at a point where it is ready for review.
New Skript Core
The new Skript interface represents the core of the language. At this time, implementations require methods for registering an addon and obtaining a collection of all addons. The Skript interface is an extension of the SkriptAddon interface, which is further described below.
A default implementation is available, which is what the plugin uses. Below is an example of creating a Skript instance using the default implementation.
As mentioned, the Skript interface is an extension of the SkriptAddon interface. To prevent users from interacting directly with Skript's addon components (that is, to ensure they actually register themselves as an addon), the default implementation returns unmodifiable versions of all required addon components. This is not guaranteed for custom implementations.
New Addon API
The SkriptAddon interface is the center of the new Addon API. At this time, every SkriptAddon has a name, syntax registry, and localizer (the last two are described in further detail below)
An addon can be registered as follows:
Addon Modules
Also included in this new API are AddonModules, which enables Skript and its addons to be broken up into categories. For example, a Skript implementation not dependent on Minecraft might have a default module containing something like arithmetic.
Modules are functional interfaces with one method,
load
. The load method has aSkriptAddon
parameter that can be used for accessing the syntax registry, localizer, and other things when they are added!Loading a module is rather simple:
Note: loadModules() is simply a helper method for new MathModule().load(myAddon)
New Syntax Registration API
The syntax registration API is the biggest component of this PR. The registration process has been completely overhauled to provide greater control over the registration process.
Syntax Infos
The SyntaxInfo interface is a replacement for the SyntaxElementInfo class.
Every SyntaxInfo has the following properties:
Two default extensions exist:
Additionally, one Bukkit-specific implementation exists for Bukkit events:
Builders
Four builders exist for creating a SyntaxInfo, SyntaxInfo.Expression, SyntaxInfo.Structure, and BukkitInfos.Event.
The following is an example of building a SyntaxInfo for LitPi with the builder for SyntaxInfo.Expression:
SyntaxOrigin
A SyntaxOrigin describes where a syntax has come from. By default, it only has a name (String) which describes the origin. When specified, this would likely be the name of an addon (see AddonOrigin). When not specified, it is the fully qualified name of the SyntaxElement class.
Priorities
The priority system is an evolution of ExpressionType. Unlike ExpressionType, which is only for expressions, the priority system applies to all SyntaxInfos. By default, Skript has three priorities:
ExpressionType#EVENT
andExpressionType#PROPERTY
have been replaced with the constantsEventValueExpression#DEFAULT_PRIORITY
andPropertyExpression#DEFAULT_PRIORITY
respectively. The former lies between SIMPLE/COMBINED while the latter lies between COMBINED/PATTERN_MATCHES_EVERYTHINGAdditionally,
PropertyCondition#DEFAULT_PRIORITY
has been added for conditions. It also lies between COMBINED/PATTERN_MATCHES_EVERYTHING.Unlike Structure priorities, this new Priority system is purely relational. There are no "magic" numbers that determine position. The definition for the default three may be useful for visualizing this:
Additionally, a Priority may be created that comes before some other Priority. If one wanted to create a Priority for syntax that should be parsed really early, they might do something like:
Syntax Priorities
There is also a utility implementation, named SyntaxPriority, which gives very specific control over how a SyntaxInfo is ordered. It enables a Priority to position itself around specific SyntaxElements.
Let's say I create an expression ExprConflictsWithSomeSkriptExpression. Unfortunately, there is a big problem: it conflicts with some Skript expression named ExprSkriptExpression. My expression shares a similar pattern, and as a result, Skript keeps parsing part of my expression as ExprSkriptExpression. While I could change the general Priority so that my expression comes first, there is no guarantee that this will continue to work if the Priority of ExprSkriptExpression changes. This is where SyntaxPriority comes in, as I can just declare the following Priority:
SyntaxRegistry
The SyntaxRegistry is the home for an addon's syntax infos. A SyntaxRegistry has three important methods:
syntaxes(Key)
which returns all SyntaxInfos registered under a Key (e.g. all expressions, effects, etc.)register(Key, SyntaxInfo)
which registers a SyntaxInfo under a Keyunregister(Key, SyntaxInfo)
which unregisters a SyntaxInfo that is stored under a KeyA default implementation of a SyntaxRegistry may be obtained through:
Keys
Keys are used for storing SyntaxInfos of a certain type. Skript has six built in keys:
STRUCTURE
for structuresSECTION
for sectionsSTATEMENT
for effects and conditions (Statement classes)EFFECT
for effectsCONDITION
for conditionsEXPRESSION
for expressionsCreating a Key constant is simple:
Now, if we wanted to access the Statements in a registry:
Child Keys
Child Keys are the same as Keys, but they also have a parent Key. Thus, when a SyntaxInfo is registered under a Child Key, it is also registered under the parent Key. This is how the register for Statements works, as the Effect and Condition Keys are Child Keys of the Statement key. We can use this example to see how to build Child Keys:
Experimental Localization API
This PR includes an experimental (and likely subject to change) API for Localization. At this point in time, it exists for modern addons to register their language files. I avoided creating too much API as that will be for a separate PR reworking localization. The key idea here is loading language files, which can be done as follows:
setSourceDirectories
takes in three parameters:source
which is a class that is used for loading resources from a jar. The class should belong to the application registering an addon.languageFileDirectory
which is the path to the language file directory on the jar.dataFileDirectory
(optional) which is the path to the language file directory on the disk (e.g. user customizable lang files).Target Minecraft Versions: any
Requirements: none
Related Issues: