Add an AvailableEvent(s) annotation by Nuutrai · Pull Request #7668 · SkriptLang/Skript

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

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.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

 		"\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

 })
-@Since("2.3")
-@RequiredPlugins("Paper 1.12.2+")
+@AvailableEvents(ServerListPingEvent.class)


Should be PaperServerListPingEvent

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

Efnilite · 2025-08-10T17:23:58Z

Do you still plan on working on this @Nuutrai?

sovdeeth · 2025-09-18T01:22:42Z

Closing due to no response. Let me know if you'd like to keep working on it, I'll re-open it.

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

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 abandonedaccount6235 March 22, 2025 13:44

Merge branch 'dev/feature' into docs

7b3216c

Efnilite requested review from a team and AyhamAl-Ali 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 Releases Jun 3, 2025

sovdeeth moved this to Needs Reviews in 2.12 Releases 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>

sovdeeth removed the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Jun 15, 2025

erenkarakal added this to 2.13 Releases Jul 1, 2025

erenkarakal removed this from 2.12 Releases Jul 1, 2025

erenkarakal moved this to In Review in 2.13 Releases Jul 1, 2025

Efnilite mentioned this pull request Aug 6, 2025

JSONGenerator for addons and docs.json improvements #8116

Merged

sovdeeth closed this Sep 18, 2025

sovdeeth removed this from 2.13 Releases Sep 19, 2025

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

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

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!

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!

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!

Efnilite commented Aug 10, 2025

Uh oh!

sovdeeth commented Sep 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

Nuutrai commented Mar 3, 2025 •

edited

Loading

sovdeeth left a comment •

edited

Loading