Allow Persistent Data Container API to accept Adventure Key

[Astralchroma]

Allows the PDC API to accept Adventure Keys making it more consistent with the rest of the API by making new versions of the functions which accept Key directly; making it a non-breaking change. The return types of getKeys() were not changed as that can already be handled as Key anyway.

[masmc05]

It is a breaking change as the bytecode signature of the method changes completely. An already compiled plugin will definitely break

[Astralchroma]

Huh, didn't realize. Would it be acceptable to just have two different versions of the functions then, one for NamespacedKey and one for Key?

[masmc05]

I can't say how acceptable it is, but it does make more sense to add a new set of methods that accepts keys, then make the current ones default with redirecting to key ones (by casting namespaced key to simply key)

[Astralchroma]

I can't say how acceptable it is, but it does make more sense to add a new set of methods that accepts keys, then make the current ones default with redirecting to key ones (by casting namespaced key to simply key)

Done!

[Strokkur424]

It is a breaking change as the bytecode signature of the method changes completely. An already compiled plugin will definitely break

Wouldn't it be possible to add a bytecode rewriter, which redirects the method calls to the old methods with the new key ones? This doesn't break source code after all, it's just a bytecode incompatible change.

Disregarding that, wouldn't it be generally better to replace all methods taking in a NamespacedKey with Key variants instead, as a sort of general cleanup PR? I am not a Paper developer, so I am not talking for the team, but from my understanding, that seems like a pretty typical "team-reserved" pull request, similar to The Material<->Item/BlockType switch, which might also be best done in-between update cycles.

[Astralchroma]

Wouldn't it be possible to add a bytecode rewriter, which redirects the method calls to the old methods with the new key ones? This doesn't break source code after all, it's just a bytecode incompatible change.

I'm not familiar with Paper's bytecode rewriting capabilities so I hadn't really considered that as an option.

Disregarding that, wouldn't it be generally better to replace all methods taking in a NamespacedKey with Key variants instead, as a sort of general cleanup PR? I am not Paper developer, so I am not talking for the team, but from my understanding, that seems like a pretty typical "team-reserved" pull request, similar to The Material<->Item/BlockType switch, which might also be best done in-between update cycles.

Perhaps, but I feel that if the decision is made to go down that route, it will never happen as Paper's team tends to take an exceptionally long amount of time to do anything other than Minecraft updates. I was hoping to get this done quickly because I keep running into it and it's annoying.

[yannicklamprecht]

I can't say how acceptable it is, but it does make more sense to add a new set of methods that accepts keys, then make the current ones default with redirecting to key ones (by casting namespaced key to simply key)

@lynxplay meant that's probably not worth to contribute. So I didn't. Otherwise I've a ABI compatible version of that pr.