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

Experimental features & the using structure. #6552

Merged
merged 36 commits into from
May 18, 2024

Conversation

Moderocky
Copy link
Member

@Moderocky Moderocky commented Apr 11, 2024

Summary

Toggleable feature-flags through a using <feature> structure for individual scripts to enable optional, experimental or alternative behaviour at parse time.

Goals

  • To provide a way to enable syntax and features only in certain scripts.
  • To be able to ship experimental changes without affecting users.
  • The means to offer new and alternative features without unwanted breaking changes.

Motivations

Skript has difficulties with breaking changes. Breaking changes make the userbase reluctant to update, even if it means missing out on speed or security improvements.
Currently, there is no concept of an 'opt-in' feature apart from specific config options.

Description

This proposal adds API for 'experimental features' both within the Skript developer API and the language itself.

These features exist as 'flags' on a script that are detectable as either present or not present, and so parse-time behaviour can be configured for this (such as choosing which version of a syntax to init, or whether to allow a particular matched pattern to be used).

The Purpose of Experimental Features

In essence, features allow Skript (or addons) to enable syntax (or syntax variations) only within particular files that opt in to using them.

An obvious use case of this is to ship testing (literally experimental) syntax in stable versions, that users who want to test (or need the functionality of) the unstable syntax can opt into, without affecting anybody who doesn't want to be at risk from it.

This may give a potential avenue for shipping newly-released Minecraft version features in a patch version of Skript, by gating them behind a temporary toggle (e.g. 1.21 syntax).

Another use of opt-in features is to prepare users for upcoming breaking changes (or, alternatively, giving users who cannot update access to the old functionality).

For example, a feature flag could be used to enable case [in]sensitivity for a particular script.

The third and final intended purpose is for offering experimental changes for the purpose of getting feedback on their use.

These changes can be shipped in a version, behind an opt-in flag (e.g. 2.9.2 big head syntax) that users are able to test and provide feedback on, with the understanding that the functionality is likely to change (or even be removed) in future versions.

Using Features in Scripts

Scripts can enable an experimental/optional feature with the new using <feature> simple structure, which should be placed at the top of the file.

using example feature

on script load:
    ...

This enables the feature (example feature) in -- and only in -- the current script.

A script may use multiple features.

using 1.21 syntax
using threadlocal variables
using strict lists
using example feature
using my cool addon feature

There is no penalty for using a feature, even if no feature by that name exists.

using foobar fake feature

In the above example, imagine nothing registers a foobar fake feature. No error is reported for 'using' a non-existent feature (although the user will be warned that no feature by that name was registered) and parsing will continue as normal.

Implementation note: this no-failure behaviour was specifically done for forwards compatibility; Skript might initially register an experimental feature for X, but when X eventually becomes standard syntax then the feature is no longer needed and can be removed without breaking anything using it.

A feature's presence can be detected from within a script itself, although only within the current script.

using example feature

on script load:
    if the script is using "example feature":
        broadcast "wow, a feature!"
    if the script is using {something}:
        broadcast "wow, another feature"

Using Features in the Skript API

All new API can be found in the org.skriptlang.skript.lang.experiment package.

Creating a Feature Flag

Opt-in features are registered as an Experiment.

An Experiment needs to provide:

  1. A 'code name' (the simple name of the feature for error triage, e.g. example feature)
  2. Its life cycle phase, which is used for providing warnings or recommendations to the user:
    • EXPERIMENTAL for opt-in features that are likely to undergo changes or become the default (e.g. new version syntax, testing changes, etc.)
    • STABLE for opt-in features that are planned to exist semi-permanently (e.g. case sensitivity, strict whitespace, backslash escapes in strings, etc.)
    • MAINSTREAM for legacy features that have now become default behaviour; users will be warned they no longer need to enable these
    • DEPRECATED for features that are dangerous or expected to be removed; users will be warned against using these
  3. Its SkriptPattern matcher, usually through the form of several String patterns.

Third-party addons or extensions will need to create implementations of this class and, ideally save them in a constant or some kind of enum.

A simple factory method Experiment.constant(codeName, phase, patterns...) is provided to make this easier to use.

Skript itself has a Feature enum for simplicity:

EXAMPLE_FEATURE("example feature", LifeCycle.STABLE)
FOO("foo", LifeCycle.STABLE, "foo[bar]", "food", "foot[ ]ball")

Registering the Experimental Feature

Features can be registered with Skript's new ExperimentManager at the same time as syntax registration.

The experiment manager is available during run-time as well (as we pre-emptively expect this may be used by addons that modify syntax, such as skript-reflect) although, obviously, features will not be available to a script after it has already been parsed.

The experiment manager instance is obtained from Skript.experiments() and the flag is registered using manager.register(addonInstance, experiment).

static final Experiment MY_FEATURE = Experiment.constant("my feature", 
                                                         LifeCycle.STABLE, 
                                                         "my [cool] feature");

void registerMyThing(SkriptAddon addon) {
    Skript.experiments().register(addon, MY_FEATURE);
}

This will then be available to scripts:

using my feature
using my cool feature

Detecting an Experimental Feature

Within syntax classes, during parse time, the presence of an experimental feature can be detected.

A simple method is provided for checking: this.hasExperiment(feature)

public boolean init(Expression<?>[] expressions, int pattern, 
                    Kleenean delayed, ParseResult result) {
    
    if (!this.hasExperiment(MY_FEATURE))
        return false;
    // do your syntax stuff here...
    return true;
}

Please note this is only (reliably) available during parse time.
The value can be stored for use during run-time.

The Unknown Feature

If a user enters a non-existent feature, e.g.

using foobar fake feature

Skript will create an Experiment with the code-name foobar fake feature and the UNKNOWN life cycle phase. This is registered to the script as a real feature would, and is detectable.

The reasoning behind this behaviour is 1) to attempt support for back-references in features registered after parsing time (since you can test for a feature flag by name) and 2) in case any future use (or third party extension) wants to use the feature flag for passing unknown, unregistered data in the using structure.


Target Minecraft Versions: any
Requirements: none
Related Issues: #6551

@Moderocky Moderocky added enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature Pull request adding a new feature. 2.9 Targeting a 2.9.X version release labels Apr 11, 2024
@ShaneBeee
Copy link
Contributor

I like all of this, seems really cool.
The only thing Im not a fan of is the use of enums and the registration steps.
In my opinion (and I really want to emphasize this is just my opinion here) it would be nicer looking and easier to do this all in one line.

Yoinked from @BaeFell

static final Experiment MY_FEATURE = Skript.experiments().register(addon, "my feature", LifeCycle.STABLE, "my [cool] feature");

Something like this seems a litter nicer looking.

@Moderocky
Copy link
Member Author

Moderocky commented Apr 11, 2024

static final Experiment MY_FEATURE = Skript.experiments().register(addon, "my feature", LifeCycle.STABLE, "my [cool] feature");

I can definitely add an extra method for that, I just want to make sure that people keep a reference to their feature so they can detect for it (cos you'll be pretty stuck if you don't haha)

The enums in Skript itself just make it simpler, since you can just add a new enum to the class (and they're all auto-registered when the class loads), there's an example in the testing class:
image

@ShaneBeee
Copy link
Contributor

static final Experiment MY_FEATURE = Skript.experiments().register(addon, "my feature", LifeCycle.STABLE, "my [cool] feature");

I can definitely add an extra method for that, I just want to make sure that people keep a reference to their feature so they can detect for it (cos you'll be pretty stuck if you don't haha)

The enums in Skript itself just make it simpler, since you can just add a new enum to the class (and they're all auto-registered when the class loads)

awesome thanks. Im excited to use this.... APPROVED (im assuming you wrote it all good)

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.

Great work Kenzie ⚡ ⚡ That was fast! we were just talking about it the other day :D

I am actually ashamed to comment on formatting stuff for such great PR 😬
Mostly formatting stuff and couple questions

Moderocky and others added 3 commits April 12, 2024 09:45
Version thingy.

Co-authored-by: Ayham Al Ali <20037329+AyhamAl-Ali@users.noreply.github.com>
Co-authored-by: Ayham Al Ali <20037329+AyhamAl-Ali@users.noreply.github.com>
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.

Can't wait to see this in action 🤩

Copy link
Member

@sovdeeth sovdeeth left a comment

Choose a reason for hiding this comment

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

no credit for knezie

@Moderocky Moderocky requested a review from sovdeeth April 13, 2024 17:48
Copy link
Member

@Pikachu920 Pikachu920 left a comment

Choose a reason for hiding this comment

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

could we make this extensible by addons? skript-reflect has a similar structure to opt in to beta features. it would be nice to keep them consistent!

* Experimental feature toggles as provided by Skript itself.
*/
public enum Feature implements Experiment {
;
Copy link
Member

Choose a reason for hiding this comment

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

whats this little guy up to

Copy link
Contributor

@ShaneBeee ShaneBeee Apr 15, 2024

Choose a reason for hiding this comment

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

Correct me if I'm wrong, but I believe he's needed if there's anything (fields/constructur/method) after enums.
He may seem lonely as he doesn't have any enum friends to keep him company.
If I were to venture a guess, Id assume Kenzie did this as Skript doesn't have any features yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

whats this little guy up to

He's keeping the seat warm!
(Enum classes need a ; before you can write any non-enum stuff!)

@Moderocky
Copy link
Member Author

could we make this extensible by addons? skript-reflect has a similar structure to opt in to beta features. it would be nice to keep them consistent!

It absolutely is, that's a core feature of the design, you've just gotta make something (anything) implementing Experiment (you can do it like Skript's enum or you can just use the constant method or do your own thing) and then register it with Skript.experiments()!
Eventually this will probably use the registrations system (when it's finished) so the current manager's a bit of a dummy for that so that we don't have to break everything when it changes.

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.

looking good

Moderocky and others added 2 commits May 16, 2024 09:09
Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>
@Moderocky Moderocky merged commit ef3d360 into SkriptLang:dev/feature May 18, 2024
5 checks passed
@Moderocky Moderocky deleted the experiments branch May 18, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.9 Targeting a 2.9.X version release enhancement Feature request, an issue about something that could be improved, or a PR improving something. 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.

None yet

6 participants