Add Foiled/Glow NBT + API

[TreemanKing]

Adds Foiled/Glow API using NBT.

Possibly can add the API to the Overrides for the default items that are foiled. Follows most of the logic of the unbreakable tag.

Relevant:
#5274

Also not sure if this is going to be relevant due to #8711.

P.S. For some reason Patch 1018 was edited and it says it removed something? I also can't rebuild or apply new patches after rebuilding the first time which is confusing to me.

[lynxplay]

I honestly think this might be best to keep for property API.

There is certainly a need for "glowing" but given that mojang does not support this natively without the empty enchant array, exposing this to the API seems a tad bleh, we are kinda just hacking for the plugin user abusing the "quirk" that the client still displays enchantment glint on an empty but present enchantment array.

imo it is a mistake to promote this to a stable API interface like ItemMeta. The property API would make a lot more sense as the user is responsible for the data they input and can hack around in there, making it a lot clearer that what they are doing might not be 100% valid state.

Given we also closed MM's PR for this (#5725), imo this is probably not worth continuing your effort here and just letting property API handle it once out. Obviously just my opinion, I'd love to hear your and other people's thought on this.

[Owen1212055]

Thank you for your contribution, however yes I am more inclined to side with lynx's comment... especially with the sensitive nature of itemstacks currently.

[TreemanKing]

There is certainly a need for "glowing" but given that mojang does not support this natively without the empty enchant array, exposing this to the API seems a tad bleh, we are kinda just hacking for the plugin user abusing the "quirk" that the client still displays enchantment glint on an empty but present enchantment array.

So from my understanding of this is that client not detecting the following server code. Am I correct with this understanding? Or is it not picking up the last statement correctly?

public boolean isEnchanted() {
        return this.tag != null && this.tag.contains("Enchantments", 9) ? !this.tag.getList("Enchantments", 10).isEmpty() : false; 
}

Given we also closed MM's PR for this (#5725), imo this is probably not worth continuing your effort here and just letting property API handle it once out. Obviously just my opinion, I'd love to hear your and other people's thought on this.

I believe MM made a fake enchantment, whereas this focuses on having either an enchantment or NBT associated with the item.

// This what all the Override items reference, I've just added an additional check to the is Foiled method, similar to that of the isEnchanted
public boolean isFoil(ItemStack stack) {
        return stack.isEnchanted() || stack.isFoiled();
}

public boolean isFoiled() {
        return this.tag != null && this.tag.contains("Foiled") && this.tag.getBoolean("Foiled");
}

I believe with a touchup, it would be more than stable as it is essentially the same as the "Unbreakable" tag. Also left it as draft as I think a discussion would be better than just going straight into review.

[TreemanKing]

From what it seems from testing, the glowing is handled by the client and the conditions as well so any server side changes won't impact if an item glows.