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

Adds experimental annotation #7109

Open
wants to merge 2 commits into
base: dev/feature
Choose a base branch
from

Conversation

cheeezburga
Copy link
Member

@cheeezburga cheeezburga commented Sep 22, 2024

Description

This PR adds a new experimental annotation, which can be used to provide information about an experiment on which a syntax element depends. The provided information is the experiment name, the associated life cycle, and the associated feedback line (as a string).

Also updates the existing doc annotations, and adds experimental stuff to SkriptEventInfo and ClassInfo,


Target Minecraft Versions: any
Requirements: none
Related Issues:

- Updates the existing annotations
- Updates SkriptEventInfo to include an experimental method
@@ -248,7 +215,7 @@ public <R> ClassInfo<T> math(final Class<R> relativeType, final Arithmetic<? sup
/**
* Only used for Skript's documentation.
*
* @param name
* @param name The name of this ClassInfo
* @return This ClassInfo object
*/
public ClassInfo<T> name(final String name) {
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
public ClassInfo<T> name(final String name) {
public ClassInfo<T> name(String name) {

@@ -260,7 +227,7 @@ public ClassInfo<T> name(final String name) {
/**
* Only used for Skript's documentation.
*
* @param description
* @param description The description(s) of this ClassInfo
* @return This ClassInfo object
*/
public ClassInfo<T> description(final String... description) {
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
public ClassInfo<T> description(final String... description) {
public ClassInfo<T> description(String... description) {

@@ -272,7 +239,7 @@ public ClassInfo<T> description(final String... description) {
/**
* Only used for Skript's documentation.
*
* @param usage
* @param usage The usage(s) of this ClassInfo
* @return This ClassInfo object
*/
public ClassInfo<T> usage(final String... usage) {
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
public ClassInfo<T> usage(final String... usage) {
public ClassInfo<T> usage(String... usage) {

@@ -284,7 +251,7 @@ public ClassInfo<T> usage(final String... usage) {
/**
* Only used for Skript's documentation.
*
* @param examples
* @param examples The example(s) for this ClassInfo
* @return This ClassInfo object
*/
public ClassInfo<T> examples(final String... examples) {
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
public ClassInfo<T> examples(final String... examples) {
public ClassInfo<T> examples(String... examples) {

@@ -296,7 +263,7 @@ public ClassInfo<T> examples(final String... examples) {
/**
* Only used for Skript's documentation.
*
* @param since
* @param since The version of the plugin in which this ClassInfo was added
* @return This ClassInfo object
*/
public ClassInfo<T> since(final String since) {
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
public ClassInfo<T> since(final String since) {
public ClassInfo<T> since(String since) {

@@ -310,14 +277,34 @@ public ClassInfo<T> since(final String since) {
*
* Only used for Skript's documentation.
*
* @param pluginNames
* @param pluginNames The plugins, other than Skript, that this ClassInfo requires
* @return This ClassInfo object
*/
public ClassInfo<T> requiredPlugins(final String... pluginNames) {
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
public ClassInfo<T> requiredPlugins(final String... pluginNames) {
public ClassInfo<T> requiredPlugins(String... pluginNames) {

* @return This ClassInfo object
*/
public ClassInfo<T> requiredPlugins(final String... pluginNames) {
assert this.requiredPlugins == null;
this.requiredPlugins = pluginNames;
return this;
}

/**
* Provides information regarding the experiment which this ClassInfo requires.
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
* Provides information regarding the experiment which this ClassInfo requires.
* Provides information regarding the experimental feature in this ClassInfo.

@@ -526,8 +504,7 @@ public String toString(final int flags) {
}

@Override
@NotNull
public String toString(final @Nullable Event event, final boolean debug) {
public @NotNull String toString(final @Nullable Event event, final boolean debug) {
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
public @NotNull String toString(final @Nullable Event event, final boolean debug) {
public @NotNull String toString(@Nullable Event event, boolean debug) {

@@ -136,6 +120,21 @@ public SkriptEventInfo<E> since(String since) {
return this;
}

/**
* Only used for Skript's documentation.
Copy link
Member

Choose a reason for hiding this comment

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

missing explanation of what this is

@sovdeeth sovdeeth added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Sep 22, 2024
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.

looks good with efy's changes

Comment on lines +19 to +20
String featureName();
LifeCycle phase();
Copy link
Member

@APickledWalrus APickledWalrus Sep 22, 2024

Choose a reason for hiding this comment

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

Do you think it might be better to require the actual experiment here? It seems tedious to have to update the phase whenever an experiment updates. The experiment itself can just be grabbed from the enum: https://github.com/SkriptLang/Skript/blob/ef3d3603c4aaedf519f2c1322c3a0f09f4b53a67/src/main/java/ch/njol/skript/registrations/Feature.java

Copy link
Member Author

@cheeezburga cheeezburga Sep 23, 2024

Choose a reason for hiding this comment

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

I was thinking this as well. Maybe it would be better to have the annotation be like, @Feature(String featureName, String feedbackLink), and then use smth like ExperimentRegistry#find(String name) later to get the experiment and its phase and stuff?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants