Expose more native map methods to PersistentDataContainer

[ngel8844]

Is your feature request related to a problem?

Will reduce boilerplate code & reduce multiple map calls where unnecessary.

Describe the solution you'd like.

The methods are self-explanatory, since they would be modifying the underlying map, just like the existing PersistentDataContainer set & get methods.

PersistentDataContainer::computeIfAbsent
PersistentDataContainer::compute
PersistentDataContainer::computeIfPresent

Describe alternatives you've considered.

Having to access the underlying map numerous times, instead of being able to use the methods provided by the underlying map.

Other

No response

[MiniDigger]

mmmh, is there maybe some interface we use? is implementing Map too much?

generally like more utils like this.

[Machine-Maker]

Well Map wouldn't really work here. get needs two parameters, a key and a type. The Map interface only has 1 parameter.

[GOjayson]

Hello, I would like to help but I've never have contributed before. could someone give me a bit more info? Thanks.

[lynxplay]

You can find our contribution guide at https://github.com/PaperMC/Paper/blob/master/CONTRIBUTING.md.

If you have more questions feel free to hop by our discord's paper-dev channel.

[ham1255]

Well Map wouldn't really work here. get needs two parameters, a key and a type. The Map interface only has 1 parameter.

since minecraft server includes guava maybe look into this? https://guava.dev/releases/30.1-jre/api/docs/com/google/common/collect/Table.html

[pop4959]

Minecraft includes it currently but is not guaranteed in the future so it's probably not a good idea to design API around that.

[ham1255]

Minecraft includes it currently but is not guaranteed in the future so it's probably not a good idea to design API around that.

never thought of that tbh, but since guava is mostly used in datafixerupper i would assume they are not removing guava anytime soon

[maestro-denery]

aren't both Paper and CraftBukkit use Guava in many places, i.e. Preconditions? I think it's alright to use it if there is something useful in it

[electronicboy]

Using inside of the API and exposing it as ABI are two entirely different concepts, but, given that guava has been part of the internals for years, and they're still using it, it's probably a reasonably safe bet; not to mention, it's already actually exposed in the API

Saying that, Table comes with some implementation expectations that the PDC does not meet;

[TheRealRyGuy]

bad idea to just manually impl? takes 30 seconds, and don't think there's any good interface that can be used for this afaik

[Machine-Maker]

No, I think a manual implementation is fine. PDC should have get getOrThrow, getOrDefault, 3 computes, maybe replace, stuff like that.

[TheRealRyGuy]

For mapping functions on compute, is it worth it to pass persistent type to it? For something like computeIfPresent, passing NamespacedKey, PDT, as well as a generic val into a function feels too bulky
Also if you do want PDT's passed in, what patch should I be putting a TriFunction impl into?
Here's the current list rn, feel free to add more

• getOrThrow w/ an optional exception supplier
• 3 computes
• replaceAll
• Implementing Iterable<NamespacedKey> - not sure about this one, just QoL util

Also can #getOrDefault be moved to be implemented in API instead of CraftPDC? Feels unnecessary

(also editing on phone because I'm realizing I just didn't commit to pr)

[lynxplay]

I don't think there is a point in passing PDT ?
The mapper for both computeIfPresent and computeIfAbsent know the output type by the method parameter ?

pdc.computeIfPresent(customDataKey, PersistentDataType.STRING, (key, s) -> s.trim());

Concerning that moving getOrDefault to the API serves literally 0 advantage for functionality, I'd argue against it as its just, more diff for no reason.

[TheRealRyGuy]

what i was thinking, i'm just incompetent at patch modifying even following instructions so just wanted to make sure, ty