Add an AvailableEvent(s) annotation

[Nuutrai]

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

[Nuutrai]

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

[Fusezion]

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]

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

[Nuutrai]

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]

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]

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]

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]

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

[Absolutionism]

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

[Nuutrai]

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

[Efnilite]

Do you still plan on working on this @Nuutrai?

[sovdeeth]

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