Adds Enchantment Glint Syntaxes

[NotSoDelayed]

Description

This PR adds the enchantment glint syntaxes into Skript!

---

Target Minecraft Versions: Minecraft 1.20.5
Requirements: Spigot 1.20.5
Related Issues: none

[sovdeeth]

needs tests

[Fusezion]

Might be worth adding an expression to give with/without enchantment glint

%itemtypes% with[:out] enchantment glint [override]

[NotSoDelayed]

The following will be added in the coming hours:

• Expression to get preset enchantment glint items
• Condition of whether an item has enchantment glint override
• TESTS!!

[Fusezion]

Aside from other requested changes looks fine to me

[APickledWalrus]

It seems unnecessary to me to have an Expression, Condition, and Effect for the same thing. I think we should pick between Condition and Effect versus Expression (I think the former may be preferrable as I am not really a fan of boolean expressions). I'd like to gather some thoughts on this :)

[sovdeeth]

It seems unnecessary to me to have an Expression, Condition, and Effect for the same thing. I think we should pick between Condition and Effect versus Expression (I think the former may be preferrable as I am not really a fan of boolean expressions). I'd like to gather some thoughts on this :)

I'm in agreement, in most cases a condition and effect flow much better. I'm not as opposed to having multiple ways to do the same thing, though, so I'd be ok with leaving the expression in too.

[NotSoDelayed]

It seems unnecessary to me to have an Expression, Condition, and Effect for the same thing. I think we should pick between Condition and Effect versus Expression (I think the former may be preferrable as I am not really a fan of boolean expressions). I'd like to gather some thoughts on this :)

For this case, how about I scrap the condition syntax? As the user can use the expression to check whether the override is set or not.

[APickledWalrus]

I honestly might rather see the expression scrapped, but I'd like opinions from others.

[NotSoDelayed]

…. though what is the purpose of explicitly setting it to false?

If the meta is set to true, the ItemStack will glint if there’s no enchantment, where vice versa the ItemStack will not glint if there’s an enchantment. Setting it to null removes the enforcement.

So if this expression is retained, it will return true, false, and null.

[NotSoDelayed]

The condition alone will return true if there’s an override — it could be set to either true or false. So this only tells the user that there’s an enforcement, but the value is unknown to them without the expression.

With the expression retained + the new whether %condition% expression, the user can do the following:

set {_} to whether enchantment glint override is set
set {_} to whether enchantment glint override is not set
set {_} to whether enchantment glint override is true
set {_} to whether enchantment glint override is false

[APickledWalrus]

Sorry for the confusion. We could maybe have another condition like %itemtypes% are forced [not] to glint? Might be worth seeing what others think.

[sovdeeth]

The condition alone will return true if there’s an override — it could be set to either true or false. So this only tells the user that there’s an enforcement, but the value is unknown to them without the expression.

With the expression retained + the new whether %condition% expression, the user can do the following:

set {_} to whether enchantment glint override is set
set {_} to whether enchantment glint override is not set
set {_} to whether enchantment glint override is true
set {_} to whether enchantment glint override is false

re: using set/not set as intended states
I don't like this, because it means that the expression returning nothing is good and intended. Which conflicts with how skript returns nothing when it fails to get the right type. If you get none from {_item}'s enchant glint override, then you don't know if it's working as intended or if {_item} is actually a number, or just not set. A second condition/pattern to handle that behavior would be better, imo.

[sovdeeth]

tests failing

[NotSoDelayed]

Apologies for the commit spam due to test suite configuration in Gradle tasks in skriptTestJava17 only has up to 1.20.4 somehow.

[sovdeeth]

Apologies for the commit spam due to test suite configuration in Gradle tasks in skriptTestJava17 only has up to 1.20.4 somehow.

1.20.6 requires java 21, so it's under skriptTestJava21

[Fusezion]

Just a quick look over and some suggestions

[Fusezion]

Thoughts about adding some test using the parse section testing most if not all patterns?

[Moderocky]

I approved this before but for some reason it's back in my queue, it still looks fine.

[APickledWalrus]

I still think we should look into how the conditions can be modified so that we do not need to include a boolean expression