Adding PersistentDataObjects

[derverdox]

Hey!
Whenever I code plugins, I must choose between saving my game data via PersistentDataContainers or a separate solution.
Normally, I prefer the PersistentDataContainer since you don't have to worry about the data being saved by the Minecraft server.
However, if you want to store larger objects with PersistentDataContainers it gets quite handy to store all the data.

That is why I have implemented PersistentDataObjects. They are added to PersistentDataContainers and implement serialization functions. It looks similar to the nbt read and write functions of particular types in nms.

The benefit of such a system:

You can work with objects that can be manipulated with state-changing methods. The serialization of those objects happens when the PersistentDataContainer is serialized.

Current flaws of this system:
Some PersistentDataContainers are serialized after plugins have been unloaded (e.g. Chunks) which results in ClassLoader Problems. Thus, the data containers that contain PersistentDataObjects need to save all objects to the pdc and clear their cache. Currently, a weak datastructure is used to cache all data containers that hold potential objects so they can be saved before plugins are unloaded.

Quick example:

public class TestData implements PersistentDataObject {
    public static TestData getTestData(PersistentDataHolder persistentDataHolder){
        return persistentDataHolder.getPersistentDataContainer().getPersistentDataObjectCache().loadOrCreatePersistentDataObject(new NamespacedKey("test","test_persistent_data_object"), new TestData());
    }
    public int testData = 1;
    
    @Override
    public PersistentDataContainer serialize(@NotNull PersistentDataAdapterContext context) {
        PersistentDataContainer persistentDataContainer = context.newPersistentDataContainer();
        persistentDataContainer.set(new NamespacedKey("test","test_data"), PersistentDataType.INTEGER, testData);
        return persistentDataContainer;
    }

    @Override
    public void deSerialize(PersistentDataContainer persistentDataContainer) {
        if(persistentDataContainer.has(new NamespacedKey("test","test_data"), PersistentDataType.INTEGER))
            this.testData = persistentDataContainer.get(new NamespacedKey("test","test_data"), PersistentDataType.INTEGER).intValue();
    }

    public TestData setTestData(int testData) {
        this.testData = testData;
        return this;
    }

    public int getTestData() {
        return testData;
    }
}

[lynxplay]

Welcome to paper 🎉

Generally, i can see the need to delay serialisation. Iirc we actually chatted about this a while back.

A major major problem I have with this, is the fact that we are now leaking instances and their respective plugin class loaders into the server state. This means that, if plugin A stores it's persistent data object on player 1, plugin A dies or gets disabled, we now have the server hold onto an object created from a class that is loaded from plugin As class loader even tho that class loader should have long been gone.

If we don't care about this issue and are fine with leaking this stuff, a way nicer implementation is probably to expand the set method on the PDC to allow users to define if their passed object is serialised directly or not.

The PDC can just store the passed data object and the persistent data type passed by user to e.g.

set(NamespacedKey key, PersistentDataType<P,C> type, P instance, bool serialise)

Alternatively we can shift this logic into potentially custom persistent data type implementations.
Either way I'd much preferred an implementation (if we do, which I am not a fan of in the first place for the previously mentioned issue), to not completely invert an existing API but now having serialisation in two places (PDT and your persistence objects).

What also needs to be considered is that, at least rn, the PDC does a proper full copy of the passed data as it serialises it.
A change like this screams for plugin Devs to be lazy and mutate their passed data objects after passing it to the pdc, which would work but kinda breaks the API contract of the PDC to some degree as people might accidentally mutate data when they previously would not.

[Owen1212055]

We would like to thank you for your contribution here, however, such a feature is a bit too expansive for us to have in the API.
Mainly, we prefer that plugins defer the logic for saving themselves here.

Some PersistentDataContainers are serialized after plugins have been unloaded (e.g. Chunks) which results in ClassLoader Problems
Instead, plugins should try to store primitive data whenever possible, to prevent issues like this.

But as mostly explained by Lynx, this API has some issues in itself that would need to be resolved, but is also something I do not think we are interested in upkeeping.

Thank you very much, we hope to see you again.