CustomKeyParsers is broken since the Key optimization commit

[ArDiDEx]

Minecraft's Version

Plugin's Version

(close to latest build)

Describe the bug

Since the optimization of Keys (commit 0bf6e516) CustomKeyParsers changed behavior and even broked

So, lets start with the behaviour change: CustomKeyParsers are now required to register themselves with a key that has durability of 0, so parsers that registered themselves with Key.of(Material.BARRIER) now require Key.of(Material.BARRIER, 0) because if they dont they get the global key with doesnt have the "0" as subkey and doesnt get called for any block (atleast for island calculation) caused by the following code: https://github.com/BG-Software-LLC/SuperiorSkyblock2/blob/b53dc6b6393815428d48129ee44e007dc6d71de4/src/main/java/com/bgsoftware/superiorskyblock/core/key/types/MaterialKey.java#L33-L44

Now the part that broke: I adapted my CustomKeyParser to also include Key.of(Material.BARRIER, 0)` After testing it on my dev server it just makes is recalculation not work anymore because of this code: https://github.com/BG-Software-LLC/SuperiorSkyblock2/blob/b53dc6b6393815428d48129ee44e007dc6d71de4/src/main/java/com/bgsoftware/superiorskyblock/core/key/collections/MaterialKeyMap.java#L136-L151 So, without custom parser the code returns at L148 (or L144 if both sets are null) But with a custom parser the code does a Sets.union with returns an unmodifiable set that causes the calculation to crash afterwards due to https://github.com/BG-Software-LLC/SuperiorSkyblock2/blob/b53dc6b6393815428d48129ee44e007dc6d71de4/src/main/java/com/bgsoftware/superiorskyblock/island/algorithm/DefaultIslandCalculationAlgorithm.java#L85

To Reproduce

Create a Custom Key Parser class,

for the behaviour change just try with and without the , 0 in Key.of(Material.XXX) and see that without it doesnt work for the code that broke just try with any material and return a valid custom key in getCustomKey, place the material on your island a do /is recalc

Additional Information

[OmerBenGera]

Regarding the key changes: You are right, there were some changes that might affect the way the CustomKeyParsers behave. I recommend using Key#ofMaterialAndData if you wish to use a string-based key, or Key#of(<Material>, <data>) for material based ones. This change was not really intended, it is a side effect. However, it's better to use these methods and avoid in all costs the usage of Key#of(<string>) to get the best optimizations of the new keys system.

In short, unlike the old system, the new system is not based on strings for keys, but on a more complex system where there are multiple key types (material based, entity-type based or string based) which in most cases will be much more optimized, especially when comparing them and making lookups in KeyMaps.

Regarding the UnsupportedOperationException, it was fixed with latest dev build. Let me know if it still causes issues.

[OmerBenGera]

I take my words back. The unintended change-behavior, is as said, unintended. Not only that, until the full stable release, you have no other options but to use Key#of(<string>), as Key#ofMaterialAndData does not exist. Therefore, I added a check in the Key#of(<string>) method to try and convert the provided key to MaterialKey and EntityTypeKey first, and only if it fails to create a CustomKey object. This should fix the unintended changes. Let me know if it works now (both the error and the key change)

[OmerBenGera]

@ArDiDEx Hey, does it work?

[ArDiDEx]

Hey! Sorry for the late response

Just tried it out and it seems to be working fine, though at first i got an error which isnt really clear without looking at the code, so i'm just gonna post it here if you want to fix it (im not gonna create a new issue for it as its not really "important", it happens when adding HOPPER_MINECRAFT into entity limits, and it didnt produce an error before)

[OmerBenGera]

Hey! Sorry for the late response

Just tried it out and it seems to be working fine, though at first i got an error which isnt really clear without looking at the code, so i'm just gonna post it here if you want to fix it (im not gonna create a new issue for it as its not really "important", it happens when adding HOPPER_MINECRAFT into entity limits, and it didnt produce an error before)

I will fix the error. Anyways, the entity type for minecart hoppers is MINECART_HOPPER https://bg-software.com/entities/