Add PersistentDataKey API

[Strokkur424]

This PR introduces a new class to the PDC group: PersistentDataKey<C>. This class act as an intersection type of both a PersistentDataType<?, C> and a NamespacedKey.

Example code for testing and usage:

Click to expand

package io.papermc.testplugin;

import io.papermc.paper.datacomponent.DataComponentTypes;
import io.papermc.paper.datacomponent.item.ItemLore;
import io.papermc.paper.datacomponent.item.Tool;
import io.papermc.paper.persistence.PersistentDataKey;
import io.papermc.paper.registry.RegistryKey;
import io.papermc.paper.registry.set.RegistrySet;
import java.util.List;
import net.kyori.adventure.key.Key;
import net.kyori.adventure.text.Component;
import net.kyori.adventure.text.format.NamedTextColor;
import net.kyori.adventure.text.format.TextDecoration;
import net.kyori.adventure.util.TriState;
import org.bukkit.Registry;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;
import org.bukkit.event.block.BlockBreakEvent;
import org.bukkit.inventory.ItemStack;
import org.bukkit.inventory.ItemType;
import org.bukkit.persistence.PersistentDataType;
import org.bukkit.plugin.java.JavaPlugin;

public final class TestPlugin extends JavaPlugin implements Listener {
    static final PersistentDataKey<Integer> BLOCKS_MINED = PersistentDataKey.of(
        Key.key("testplugin", "blocks_mined"),
        PersistentDataType.INTEGER
    );

    @Override
    public void onEnable() {
        this.getServer().getPluginManager().registerEvents(this, this);
        this.registerCommand("super-pick", (source, _) -> {
            if (!(source.getExecutor() instanceof Player player)) {
                source.getSender().sendRichMessage("<red>Only players can execute this command!");
                return;
            }

            final ItemStack is = ItemType.DIAMOND_PICKAXE.createItemStack();
            is.setData(DataComponentTypes.ENCHANTMENT_GLINT_OVERRIDE, true);
            is.setData(DataComponentTypes.UNBREAKABLE);
            is.setData(DataComponentTypes.TOOL, Tool.tool()
                .addRule(Tool.rule(
                    RegistrySet.keySetFromValues(
                        RegistryKey.BLOCK,
                        Registry.BLOCK.stream().toList()
                    ),
                    Float.MAX_VALUE,
                    TriState.TRUE
                ))
                .build());
            is.editPersistentDataContainer(pdc -> pdc.set(BLOCKS_MINED, 0));
            updatePickLore(is);
            player.give(is);
        });
    }

    @EventHandler
    void onMine(BlockBreakEvent event) {
        final ItemStack held = event.getPlayer().getInventory().getItemInMainHand();
        if (held.getPersistentDataContainer().has(BLOCKS_MINED)) {
            held.editPersistentDataContainer(pdc -> pdc.set(BLOCKS_MINED, pdc.getOrDefault(BLOCKS_MINED, 0) + 1));
            updatePickLore(held);
        }
    }

    private void updatePickLore(ItemStack is) {
        final int mined = is.getPersistentDataContainer().getOrDefault(BLOCKS_MINED, 0);
        is.setData(DataComponentTypes.LORE, ItemLore.lore(List.of(
            Component.text("Mined: ", NamedTextColor.DARK_GRAY)
                .append(Component.text(mined, NamedTextColor.RED))
                .decorationIfAbsent(TextDecoration.ITALIC, TextDecoration.State.FALSE)
        )));
    }
}

Why?

The purpose of this addition is to have an API-native way to store the access key and data type of a PDC value in one place. Traditionally, a plugin developer will have to keep track of the key and the data type separately, which may cause accidental errors not caught at compile time due to incorrect data type usage, or may generally not be nice to work with.

Why Key over NamespacedKey?

The Adventure Key class is nicer to use than NamespacedKey. New API typically always uses Key. In a PR that aimed to refactor PDC to use Key instead, the change was denied due to bytecode incompatibility. This, however, it not a problem for newly added classes or methods.

The implementation, PaperPersistentDataKey actually still uses NamespacedKey, as it can be cast to Key without any complications, which for as long as NamespacedKey stays the primary key class for PDC, needs to be exposed.

Final notes

As with all my PRs, the implementation I have provided here is purely after what I thought was most fitting at the moment. I am open to improvement suggestions and critique on this addition.

[Strokkur424]

Also, I am open to renaming suggestions. Feel free to suggest better naming for the two classes and static methods on the current PersistentDataKey interface (#of and #ofSimple)

[masmc05]

Why would the key's generic need to track primitive type? I thought the primitive generic was more for enforcing implementation consistency of the primitive type in all the methods than for outside consumers

[Strokkur424]

Why would the key's generic need to track primitive type? I thought the primitive generic was more for enforcing implementation consistency of the primitive type in all the methods than for outside consumers

Actually good catch. I just copied the semantics of the PersistentDataType object blindly, since even the PDC #set method keeps track of the primitive type. Turns out, having it as a wildcard is perfectly fine too. That makes things easier, as I can get rid of the "simple" interface now.

[Strokkur424]

I am thinking about merging the distinct #getKey(): Key and #getNamespacedKey(): NamespacedKey into just a single #getKey(): NamespacedKey.

The NK class already extends Key. My idea behind this was to encourage developers to use Adventure Key (AK) over NK, but if NK is exposed over the API anyways, there's really no advantage, even if we wanted to switch to only AK (which I am not aware of.)

I will tend to this in the evening.

[WouterGritter]

Could we look into actually registering these PersistentDataKeys, enforcing the type on every get/set? A bare key would then also get type checking. That'll widen the scope a lot though and it would open up lots of other questions, e.g. reading a BOOLEAN as a BYTE, or a UUID as a raw BYTE_ARRAY (to avoid encoding it just to decode it again), both fine today, would then be disallowed.

[Strokkur424]

I've tended to the review comments and my previous comment about removing the distinct getKey/getNamespacedKey methods. I don't think it makes sense to poke into the PDC internals to register these PDK classes... the way it works right now is more than enough, IMO

[SirYwell]

The asymmetry of passing Key and getting NamespacedKey is a bit weird but might be fine. If you want to keep that as an internal detail, you could just return Key but cast in the relevant places, although I'm not sure if that's a great solution.

[Strokkur424]

The asymmetry is a by-product of wanting to promote Key usage in API, but everything in PDC wanting a namespaced key. If I'd wanted to completely hide it to the outside, I could do that by making the implementation on PDC/PDCV an implementation detail as well, instead of just using default, but I think it's not worth it in this case.