Add an AvailableEvent(s) annotation #7668

Nuutrai · 2025-03-02T03:53:15Z

Description

This annotation will hopefully bring some sort of (sk) unity to the Events annotation catastrophe.
Let me know if I should (if this is good) include a deprecation of the @Events annotation.
(Here you go efy!)

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

* Adds ParsingStack data type * Add ParseStackOverflowException * Store ParsingStack in ParserInstance * Implement the parsing stack in SkriptParser Document SkriptParser#parse_i a bit better * Improve robustness of ParsingStack Add more operations to ParsingStack * Add usage note to ParsingStack * Add modification notice to ParserInstance#getParsingStack * Switch from Stack class to LinkedList class Add notice on iteration order * Change brackets of ignored catch block Co-authored-by: Patrick Miller <apickledwalrus@gmail.com> * Review response, slight documentation changes, variable rename, formatting changes * Update src/main/java/ch/njol/skript/lang/parser/ParsingStack.java Co-authored-by: Kiip <25848425+kiip1@users.noreply.github.com> * Add `@param` tags to method Javadocs for completeness sake Co-authored-by: LimeGlass <16087552+TheLimeGlass@users.noreply.github.com> * Forgot import because of GH suggested change commit * Fix newer merge changes. --------- Co-authored-by: TPGamesNL <pet.teun.03@gmail.com> Co-authored-by: TPGamesNL <29547183+TPGamesNL@users.noreply.github.com> Co-authored-by: Patrick Miller <apickledwalrus@gmail.com> Co-authored-by: Kiip <25848425+kiip1@users.noreply.github.com> Co-authored-by: LimeGlass <16087552+TheLimeGlass@users.noreply.github.com>

* init commit * optimize imports * replace try/catches with isValidUUID method, final changes * Final changes * tiny change * Tests and changes * fix test * Add a uuid function and final changes * Remove the AnyValued for UUIDs and tests for parsing uuids * Change throwing an exception to asserting false when deserializing * Clean up --------- Co-authored-by: Efnilite <35348263+Efnilite@users.noreply.github.com>

# Conflicts: # src/main/java/ch/njol/skript/doc/AvailableEvents.java # src/main/java/ch/njol/skript/expressions/ExprAbsorbedBlocks.java # src/main/java/ch/njol/skript/expressions/ExprAppliedEffect.java # src/main/java/ch/njol/skript/expressions/ExprAppliedEnchantments.java # src/main/java/ch/njol/skript/expressions/ExprAttacked.java # src/main/java/ch/njol/skript/expressions/ExprAttacker.java # src/main/java/ch/njol/skript/expressions/ExprBeaconValues.java # src/main/java/ch/njol/skript/expressions/ExprClicked.java # src/main/java/ch/njol/skript/expressions/ExprCommand.java # src/main/java/ch/njol/skript/expressions/ExprCommandSender.java # src/main/java/ch/njol/skript/expressions/ExprDamage.java # src/main/java/ch/njol/skript/expressions/ExprEgg.java # src/main/java/ch/njol/skript/expressions/ExprEnchantItem.java # src/main/java/ch/njol/skript/expressions/ExprEnchantingExpCost.java # src/main/java/ch/njol/skript/expressions/ExprEnchantmentBonus.java # src/main/java/ch/njol/skript/expressions/ExprEnchantmentOffer.java # src/main/java/ch/njol/skript/expressions/ExprEvtInitiator.java # src/main/java/ch/njol/skript/expressions/ExprExperience.java # src/main/java/ch/njol/skript/expressions/ExprExplodedBlocks.java # src/main/java/ch/njol/skript/expressions/ExprExplosionBlockYield.java # src/main/java/ch/njol/skript/expressions/ExprExplosionYield.java # src/main/java/ch/njol/skript/expressions/ExprFertilizedBlocks.java # src/main/java/ch/njol/skript/expressions/ExprFinalDamage.java # src/main/java/ch/njol/skript/expressions/ExprHatchingNumber.java # src/main/java/ch/njol/skript/expressions/ExprHatchingType.java # src/main/java/ch/njol/skript/expressions/ExprHealAmount.java # src/main/java/ch/njol/skript/expressions/ExprHealReason.java # src/main/java/ch/njol/skript/expressions/ExprHealth.java # src/main/java/ch/njol/skript/expressions/ExprHoverList.java # src/main/java/ch/njol/skript/expressions/ExprInventoryCloseReason.java # src/main/java/ch/njol/skript/expressions/ExprLevel.java # src/main/java/ch/njol/skript/expressions/ExprLevelProgress.java # src/main/java/ch/njol/skript/expressions/ExprMe.java # src/main/java/ch/njol/skript/expressions/ExprMessage.java # src/main/java/ch/njol/skript/expressions/ExprProtocolVersion.java # src/main/java/ch/njol/skript/expressions/ExprReadiedArrow.java # src/main/java/ch/njol/skript/expressions/ExprSentCommands.java # src/main/java/ch/njol/skript/expressions/ExprVersionString.java

This reverts commit 2c9571e.

This reverts commit 56d6871.

Nuutrai · 2025-03-02T03:54:43Z

Oops, sorry Kenzie & Burb, there was a mishap with merging. Apologies for the (maybe?) notification

Efnilite

nice! the AvailableEvent annotation is not necessary as that can be handled by AvailableEvents, and you can remove the old Events from the classes

Fusezion · 2025-03-02T17:39:09Z

nice! the AvailableEvent annotation is not necessary as that can be handled by AvailableEvents, and you can remove the old Events from the classes

It was something I suggested, as a quality of life thing, most events only ever need 1, the same could have been said about @Example, I would also disagree on removing the old @Events from the classes where they changed, as this would be breaking changes to documentation sites outside of Skript, deprecation should be done first

Efnilite · 2025-03-03T11:36:37Z

nice! the AvailableEvent annotation is not necessary as that can be handled by AvailableEvents, and you can remove the old Events from the classes

It was something I suggested, as a quality of life thing, most events only ever need 1, the same could have been said about @Example, I would also disagree on removing the old @Events from the classes where they changed, as this would be breaking changes to documentation sites outside of Skript, deprecation should be done first

for @Example, it is intended to replace @Examples, not for it to be used so that you use @Example when there's only a single example, and @Examples when there are multiple. it's easier to have one annotation used for the same thing.

there's also no reason to allow @AvailableEvent to be repeated since you can just do this

@AvailableEvents({X, Y})

for @Example, this was done to easily be able to separate where an example starts/ends, as with the current @Examples this is harder to see. however, in this case, there's no need to separate them as the class names are not multiple lines long

you also dont even need the curly brackets when passing a single value to an annotation, e.g.
https://github.com/SkriptLang/Skript/pull/7668/files#diff-4cc59563f8735f651b5bdc718e0ec9eafdb047cbf2649d2574efa625e731264dR23
and this is also the case for the @Since annotation

therefore, the only difference between using @AvailableEvent for single values and @AvailableEvents is the letter s, which, in my opinion, is not worth it.

@AvailableEvent(X)
@AvailableEvents(X)

fair enough for keeping @Events

Revert "More changes" This reverts commit d4f3564 Revert "Correct list annotations" This reverts commit 79e4db5. Revert "Fixing a fix" This reverts commit cb92666.

Nuutrai · 2025-03-03T18:04:37Z

nice! the AvailableEvent annotation is not necessary as that can be handled by AvailableEvents, and you can remove the old Events from the classes

It was something I suggested, as a quality of life thing, most events only ever need 1, the same could have been said about @Example, I would also disagree on removing the old @Events from the classes where they changed, as this would be breaking changes to documentation sites outside of Skript, deprecation should be done first

for @Example, it is intended to replace @Examples, not for it to be used so that you use @Example when there's only a single example, and @Examples when there are multiple. it's easier to have one annotation used for the same thing.

there's also no reason to allow @AvailableEvent to be repeated since you can just do this
@AvailableEvents({X, Y})
for @Example, this was done to easily be able to separate where an example starts/ends, as with the current @Examples this is harder to see. however, in this case, there's no need to separate them as the class names are not multiple lines long

you also dont even need the curly brackets when passing a single value to an annotation, e.g. https://github.com/SkriptLang/Skript/pull/7668/files#diff-4cc59563f8735f651b5bdc718e0ec9eafdb047cbf2649d2574efa625e731264dR23 and this is also the case for the @Since annotation

therefore, the only difference between using @AvailableEvent for single values and @AvailableEvents is the letter s, which, in my opinion, is not worth it.
@AvailableEvent(X)
@AvailableEvents(X)
fair enough for keeping @Events

The AvailableEvent has been removed and replaced

sovdeeth · 2025-03-03T18:08:10Z

Is there any reason this can't be added on to the existing Events annotation? eg have 2 inputs, 1 string array and 1 class array, both with defaults to empty arrays?
I think it could be nicer for backwards compatibility.

Nuutrai · 2025-03-03T18:17:19Z

The events annotation doesn't do too good with representing what it means. Yes, AvailableEvents isn't too too much better, but it is more intuitive that these are the events that the element is available for. As long as Events is deprecated but still used, it should be okay to have a new annotation. Plus if there were to be two default options for Events, it would end up forcing Event(classes={}) from my knowledge

sovdeeth · 2025-03-03T18:17:50Z

The events annotation doesn't do too good with representing what it means. Yes, AvailableEvents isn't too too much better, but it is more intuitive that these are the events that the element can be used with. As long as Events is deprecated but still used, it should be okay to have a new annotation. Plus if there were to be two default options for Events, it would end up forcing Event(classes={}) from my knowledge

Makes sense

Nuutrai · 2025-03-04T03:01:24Z

(I also can't see what changes you've requested, or at least I can't find them)

Absolutionism · 2025-03-04T03:10:18Z

(I also can't see what changes you've requested, or at least I can't find them)

He didn't do any suggestions, he just marked his review as requested changes with his comment.

Efnilite

Looks good, almost there

src/main/java/ch/njol/skript/doc/JSONGenerator.java

Co-authored-by: Efnilite <35348263+Efnilite@users.noreply.github.com>

Efnilite

you can also increment the JSON_VERSION to 2, 0.

from my testing your changes work, so feel free to re-request me and it'll get approved 👍 thanks!

src/main/java/ch/njol/skript/doc/JSONGenerator.java

Co-authored-by: Efnilite <35348263+Efnilite@users.noreply.github.com>

src/main/java/ch/njol/skript/expressions/ExprCommand.java

src/main/java/ch/njol/skript/expressions/ExprCommandSender.java

Efnilite

after looking at it again, a few minor things still

src/main/java/ch/njol/skript/doc/JSONGenerator.java

src/main/java/ch/njol/skript/doc/AvailableEvents.java

Co-authored-by: Efnilite <35348263+Efnilite@users.noreply.github.com>

sovdeeth

I have some concerns since this is changing the meaning of the events annotation. Previously, it was meant to list SkriptEvents that the syntax can be used in, not Bukkit Events. These are not 1:1 and I wonder if there's a case where a syntax can be used in skript event A, but not skript event B, where both A and B are derivative of Bukkit event X.

Let me know your thoughts.

Nuutrai · 2025-03-22T19:12:58Z

I have some concerns since this is changing the meaning of the events annotation. Previously, it was meant to list SkriptEvents that the syntax can be used in, not Bukkit Events. These are not 1:1 and I wonder if there's a case where a syntax can be used in skript event A, but not skript event B, where both A and B are derivative of Bukkit event X.

Let me know your thoughts.

I'm not entirely sure how we would deal with this. I had originally thought that we could just check if the AvailableEvents contains all events in said Skript event, but then I thought that it would become a little bit annoying for users to have to look into the Skript event that they'd like to have in the AvailableEvents, and that it would take up more time during development. I do see your concern, but without needing to take up development time, I'm not currently sure of a way to deal with this

sovdeeth

Sorry about the delayed review. I agree in that I don't really see a way to resolve it and I think the failure case is small enough that it's worth adding. Perhaps a manual override option just in case, where you can still provide strings?

APickledWalrus

Looks pretty good. A few minor mistakes I noticed

src/main/java/ch/njol/skript/doc/JSONGenerator.java

APickledWalrus · 2025-06-05T17:20:12Z

src/main/java/ch/njol/skript/expressions/ExprSentCommands.java

 import ch.njol.skript.events.EvtPlayerCommandSend;
 import ch.njol.skript.lang.Expression;
 import ch.njol.skript.lang.ExpressionType;
 import ch.njol.skript.lang.SkriptParser.ParseResult;
 import ch.njol.skript.lang.util.SimpleExpression;
 import ch.njol.util.Kleenean;
+import com.destroystokyo.paper.event.brigadier.AsyncPlayerSendCommandsEvent;


APickledWalrus · 2025-06-05T17:20:48Z

src/main/java/ch/njol/skript/expressions/ExprProtocolVersion.java

@@ -35,9 +31,10 @@
 @Examples({"on server list ping:",
 		"\tset the version string to \"&lt;light green&gt;Version: &lt;orange&gt;%minecraft version%\"",
 		"\tset the protocol version to 0 # 13w41a (1.7) - so the player will see the custom version string almost always"})
-@Since("2.3")
-@RequiredPlugins("Paper 1.12.2 or newer")
+@AvailableEvents(ServerListPingEvent.class)


It should be PaperServerListPingEvent

APickledWalrus · 2025-06-05T17:22:13Z

src/main/java/ch/njol/skript/expressions/ExprLevelProgress.java

+@AvailableEvents(PlayerLevelChangeEvent.class)
 @Events("level change")


I think you can actually remove the Events annotation here. It doesn't seem to be required/used for the syntax?

I believe this was mentioned before, as I replied to it prior the removal of it there causes breaking changes to other documentation sites

APickledWalrus · 2025-06-05T17:25:05Z

src/main/java/ch/njol/skript/doc/JSONGenerator.java

+		for (Class<? extends Event> event : availableEvents.value()) {
+			if (events.get(event) == null) 
+				continue;
+			for (SkriptEventInfo<?> skriptEvent : events.get(event)) {


It's possible that two Bukkit events will resolve to the same SkriptEventInfo. Is that something we need to be checking?

src/main/java/ch/njol/skript/expressions/ExprHealReason.java

APickledWalrus · 2025-06-05T17:30:25Z

src/main/java/ch/njol/skript/expressions/ExprHealth.java

+@AvailableEvents(EntityDamageEvent.class)
 @Events("damage")


Based on the commented-out code, I think you can instead remove the Events annotation here. It doesn't appear to have event-specific behavior anymore

APickledWalrus · 2025-06-05T17:32:24Z

src/main/java/ch/njol/skript/expressions/ExprVersionString.java

@@ -29,9 +25,10 @@
 		"\tset the protocol version to 0 # 13w41a (1.7), so it will show the version string always",
 		"\tset the version string to \"&lt;light green&gt;Version: &lt;orange&gt;%minecraft version%\""
 })
-@Since("2.3")
-@RequiredPlugins("Paper 1.12.2+")
+@AvailableEvents(ServerListPingEvent.class)


Should be PaperServerListPingEvent

Co-authored-by: Patrick Miller <apickledwalrus@icloud.com>

Nuutrai and others added 10 commits March 1, 2025 21:17

Add annotation

ed0a7c1

Add annotation to needed expressions

2857590

Add annotation

5e909b3

Add annotation to needed expressions

29f90ec

More changes

d4f3564

Revert "String -> UUID (SkriptLang#7497)"

09ca234

This reverts commit 2c9571e.

Revert "Parsing stack (SkriptLang#5277) (SkriptLang#7534)"

2608e78

This reverts commit 56d6871.

Efnilite requested changes Mar 2, 2025

View reviewed changes

Nuutrai added 4 commits March 2, 2025 21:29

Adds a little documentation to the annotations

4d61948

Correct list annotations

79e4db5

Fixing a fix

cb92666

Deprecate Events

7ae9fe1

Revert stackable AvailableEvent change

81d2c47

Revert "More changes" This reverts commit d4f3564 Revert "Correct list annotations" This reverts commit 79e4db5. Revert "Fixing a fix" This reverts commit cb92666.

Oops

d650ca3

Efnilite added enhancement Feature request, an issue about something that could be improved, or a PR improving something. documentation Related to Skript's official documentation. labels Mar 3, 2025

Nuutrai marked this pull request as ready for review March 4, 2025 02:24

Nuutrai requested a review from Efnilite March 4, 2025 03:00

Efnilite requested changes Mar 8, 2025

View reviewed changes

src/main/java/ch/njol/skript/doc/JSONGenerator.java Outdated Show resolved Hide resolved

src/main/java/ch/njol/skript/doc/JSONGenerator.java Outdated Show resolved Hide resolved

Nuutrai and others added 3 commits March 8, 2025 10:15

Update src/main/java/ch/njol/skript/doc/JSONGenerator.java

e9a41b2

Co-authored-by: Efnilite <35348263+Efnilite@users.noreply.github.com>

Moved JsonArray generator to a helper

06f166f

Merge remote-tracking branch 'origin/docs' into docs

e27c96f

Efnilite requested changes Mar 10, 2025

View reviewed changes

Nuutrai and others added 2 commits March 10, 2025 22:32

Apply suggestions from code review

8b8dc44

Co-authored-by: Efnilite <35348263+Efnilite@users.noreply.github.com>

Update JSON_VERSION to 2.0

c81f8cf

Efnilite requested changes Mar 11, 2025

View reviewed changes

src/main/java/ch/njol/skript/expressions/ExprCommand.java Outdated Show resolved Hide resolved

src/main/java/ch/njol/skript/expressions/ExprCommandSender.java Outdated Show resolved Hide resolved

Nuutrai and others added 2 commits March 11, 2025 10:46

Correct AvailableEvents for ExprCommand and ExprCommandSender

1465585

Merge branch 'dev/feature' into docs

f877e60

Efnilite approved these changes Mar 13, 2025

View reviewed changes

Efnilite requested changes Mar 13, 2025

View reviewed changes

Nuutrai and others added 2 commits March 13, 2025 18:06

Switch #cacheEvents() to static loading

dd02e4c

Apply suggestions from code review

f4a7e42

Co-authored-by: Efnilite <35348263+Efnilite@users.noreply.github.com>

Efnilite approved these changes Mar 13, 2025

View reviewed changes

Efnilite requested a review from Pikachu920 March 22, 2025 13:44

Merge branch 'dev/feature' into docs

7b3216c

Efnilite requested review from AyhamAl-Ali and a team as code owners March 22, 2025 13:44

sovdeeth requested changes Mar 22, 2025

View reviewed changes

Merge branch 'dev/feature' into docs

7cc725f

skriptlang-automation bot added needs reviews A PR that needs additional reviews and removed needs reviews A PR that needs additional reviews labels May 15, 2025

sovdeeth approved these changes Jun 3, 2025

View reviewed changes

skriptlang-automation bot added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Jun 3, 2025

sovdeeth added this to 2.12 Release Jun 3, 2025

sovdeeth moved this to Needs Reviews in 2.12 Release Jun 3, 2025

APickledWalrus requested changes Jun 5, 2025

View reviewed changes

Apply half suggestions from code review

a985dda

Co-authored-by: Patrick Miller <apickledwalrus@icloud.com>

		@AvailableEvents(PlayerLevelChangeEvent.class)
		@Events("level change")

		@AvailableEvents(EntityDamageEvent.class)
		@Events("damage")

Uh oh!

Add an AvailableEvent(s) annotation #7668

Are you sure you want to change the base?

Add an AvailableEvent(s) annotation #7668

Uh oh!

Conversation

Nuutrai commented Mar 2, 2025

Description

Uh oh!

Nuutrai commented Mar 2, 2025

Uh oh!

Efnilite left a comment

Choose a reason for hiding this comment

Uh oh!

Fusezion commented Mar 2, 2025

Uh oh!

Efnilite commented Mar 3, 2025

Uh oh!

Nuutrai commented Mar 3, 2025

Uh oh!

sovdeeth commented Mar 3, 2025

Uh oh!

Nuutrai commented Mar 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sovdeeth commented Mar 3, 2025

Uh oh!

Nuutrai commented Mar 4, 2025

Uh oh!

Absolutionism commented Mar 4, 2025

Uh oh!

Efnilite left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Efnilite left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Efnilite left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sovdeeth left a comment

Choose a reason for hiding this comment

Uh oh!

Nuutrai commented Mar 22, 2025

Uh oh!

sovdeeth left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

APickledWalrus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

APickledWalrus Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

APickledWalrus Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

APickledWalrus Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

Fusezion Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

APickledWalrus Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

Nuutrai commented Mar 3, 2025 •

edited

Loading

sovdeeth left a comment •

edited

Loading