[1.16.5] Question about Capabilities (actually saving/editing the data to ServerEntityPlayer)

[Burchard36]

Hello, I recently opened a different thread here I decided to update to 1.16.5 which was actually not that bad, however I have a question about Capabilities:

How do you actually save the Capability? I have a ServerEntityPlayer Capability to store data about the player (health, energy etc, currently only have health for right now) and im able to access it from the ServerEntityPlayer#getCapability method

How im doing it:

 

Spoiler

    @SubscribeEvent(priority = EventPriority.HIGH)
    public void onHit(final LivingHurtEvent e) {
        if (e.getEntityLiving() instanceof ServerPlayerEntity) {
            final ServerPlayerEntity mpPlayer = (ServerPlayerEntity) e.getEntityLiving();
			/* i run other code here, hence why this is in LivingHurtEvent */
          
            /* This is just for testing pulling the values and setting the values */
            mpPlayer.getCapability(ModCapabilities.PLAYER_STATS).ifPresent(iCurrentStats -> {
                DragonBlock.LOGGER.info("ACCEPTED STATS_CAPABILITY");
                DragonBlock.LOGGER.info("Capability Health: " + iCurrentStats.getCurrentHealth());
                DragonBlock.LOGGER.info("Capability max health " + iCurrentStats.getMaxHealth());

                iCurrentStats.setHealth(iCurrentStats.getCurrentHealth() + 2);
            });
        }
    }

 

So how do you actually go about saving the data you set? Whenever the Health logs it is always -1 (The default value thats set), is there a way your supposed to overwrite a players Capability or am I going about this the wrong way?

Here is the Capability classes im using:

ModCapabilites (This also handles attaching to player, i should probably check if player already has the capability, but for the save of this question ignore it for now since im changing Capability values onHit up above):

 

Spoiler

public class ModCapabilities implements ICapabilityProvider{

    @CapabilityInject(ICurrentStats.class)
    public static Capability<ICurrentStats> PLAYER_STATS = null;

    public ModCapabilities() {
        this.registerCapabilities();
    }

    @SubscribeEvent
    public void onAttachCapabilities(final AttachCapabilitiesEvent<Entity> e) {
        if (e.getObject() instanceof ServerPlayerEntity) {
            e.addCapability(new ResourceLocation("dragon_block", "player_stats"), this);
        }
    }

    public void registerCapabilities() {
        CapabilityManager.INSTANCE.register(
                ICurrentStats.class,
                new CapabilityWorker<>(PLAYER_STATS),
                CurrentStats::new
        );
    }

    public static Capability<ICurrentStats> get() {
        return PLAYER_STATS;
    }

    @Nonnull
    @Override
    public <T> LazyOptional<T> getCapability(@Nonnull Capability<T> cap, @Nullable Direction side) {
        if (cap == PLAYER_STATS) {
            return LazyOptional.of(CurrentStats::new).cast();
        } else return LazyOptional.empty();
    }


    private static class CapabilityWorker<S extends CompoundNBT, C extends INBTSerializable<S>> implements Capability.IStorage<C> {

        private final Capability<C> cap;

        public CapabilityWorker(final Capability<C> capability) {
            this.cap = capability;
        }

        @Nullable
        @Override
        public final INBT writeNBT(final Capability<C> capability,
                                 final C instance,
                                 final Direction side) {
            if (this.cap != capability) return null; // Make sure were writing to the right Capability
            return instance.serializeNBT();
        }

        @Override
        @SuppressWarnings("unchecked") // No need to check S
        public final void readNBT(final Capability<C> capability,
                            final C instance,
                            final Direction side,
                            final INBT nbtBase) {
            if (this.cap != capability) return; // Make sure were writing to the right Capability
            instance.deserializeNBT((S) nbtBase); // No need to check, always using NBT Compounds
        }
    }
}

 

Current Stats (NBTTags is just an Enum class with a String constructor):

 

Spoiler

public class CurrentStats implements ICurrentStats {

    private long currentHealth;
    private long maxHealth;

    public CurrentStats() {
        this.currentHealth = -1;
        this.maxHealth = -1;
    }

    @Override
    public long getCurrentHealth() {
        return this.currentHealth;
    }

    @Override
    public long getMaxHealth() {
        return this.maxHealth;
    }

    @Override
    public void setHealth(long amt) {
        this.currentHealth = amt;
    }

    public final void setCurrentHealth(final long amount) {
        this.currentHealth = amount;
    }

    public final void setMaxHealth(final long amount) {
        this.maxHealth = amount;
    }

    @Override
    public final CompoundNBT serializeNBT() {
        final CompoundNBT comp = new CompoundNBT();
        comp.putLong(NBTTags.CURRENT_HEALTH.toString(), this.getCurrentHealth());
        comp.putLong(NBTTags.MAX_HEALTH.toString(), this.getMaxHealth());
        return comp;
    }

    @Override
    public final void deserializeNBT(final CompoundNBT nbt) {
        this.currentHealth = nbt.getLong(NBTTags.CURRENT_HEALTH.toString());
    }
}

 

ICurrentStats:

 

Spoiler

public interface ICurrentStats extends INBTSerializable<CompoundNBT> {

    long getCurrentHealth();
    long getMaxHealth();
    void setHealth(final long amt);
}

 

The area where I'm changing/wanting to change the capability values is stated at the top of the page

So: What am I exactly doing wrong that I'm not able to edit Capability values? Am I missing something that causes the data to not be changed when I call setHealth? The event 100% fires as the log messages I have are logged, and the values returned are of the default values in CurrentStats. I know i need to handle the on death event to rebind the Capability (Was planning on adding that after I get this working), is this something similar to that? It does feel a little wrong to edit the values like that in ifPresent so maybe that has something to do with it?

[Burchard36]

20 minutes ago, diesieben07 said:

For attached capabilities your ICapabilityProvider needs to implement INBTSerializable if you want to save data. You can use ICapabilitySerializable, which combines both interfaces.

And when I override the Method(s) serializeNBT and deserializeNBT inside of ICapabilityProvider, what exactly goes into it (I assume its the same how I did the methods in CurrentStats class)? If so, there no real data to serialize or deserialize since the data is per player and there is no player object to get at the time of overriding these methods? Or am is something flying by my head right now?

By the way I made ICapabilityProvider implement INBTSerializable<CompoundNBT>, this is the ICapabilityProvider class now:

ModCapabilities:

 

Spoiler

public class ModCapabilities implements ICapabilityProvider, INBTSerializable<CompoundNBT>{

    @CapabilityInject(ICurrentStats.class)
    public static Capability<ICurrentStats> PLAYER_STATS = null;

    public ModCapabilities() {
        this.registerCapabilities();
    }

    @SubscribeEvent
    public void onAttachCapabilities(final AttachCapabilitiesEvent<Entity> e) {
        if (e.getObject() instanceof ServerPlayerEntity) {
            final ServerPlayerEntity p = (ServerPlayerEntity) e.getObject();
            if (!p.getCapability(ModCapabilities.PLAYER_STATS).isPresent()) {
                DragonBlock.LOGGER.info("Added player_stats to player instance");
                e.addCapability(new ResourceLocation("dragon_block", "player_stats"), this);

            }
        }
    }

    public void registerCapabilities() {
        CapabilityManager.INSTANCE.register(
                ICurrentStats.class,
                new CapabilityWorker<>(PLAYER_STATS),
                CurrentStats::new
        );
    }

    public static Capability<ICurrentStats> get() {
        return PLAYER_STATS;
    }

    @Nonnull
    @Override
    public <T> LazyOptional<T> getCapability(@Nonnull Capability<T> cap, @Nullable Direction side) {
        if (cap == PLAYER_STATS) {
            return LazyOptional.of(CurrentStats::new).cast();
        } else return LazyOptional.empty();
    }

    @Override
    public CompoundNBT serializeNBT() {
        /* What are you supposed to do here? */
        return null;
    }

    @Override
    public void deserializeNBT(CompoundNBT nbt) {
        /* What are you supposed to do here? */
    }


    private static class CapabilityWorker<S extends CompoundNBT, C extends INBTSerializable<S>> implements Capability.IStorage<C> {

        private final Capability<C> cap;

        public CapabilityWorker(final Capability<C> capability) {
            this.cap = capability;
        }

        @Nullable
        @Override
        public final INBT writeNBT(final Capability<C> capability,
                                 final C instance,
                                 final Direction side) {
            if (this.cap != capability) return null; // Make sure were writing to the right Capability
            return instance.serializeNBT();
        }

        @Override
        @SuppressWarnings("unchecked") // No need to check S
        public final void readNBT(final Capability<C> capability,
                            final C instance,
                            final Direction side,
                            final INBT nbtBase) {
            if (this.cap != capability) return; // Make sure were writing to the right Capability
            instance.deserializeNBT((S) nbtBase); // No need to check, always using NBT Compounds
        }
    }
}

 

 

Edited August 2, 2021 by Burchard36
spelling

[Burchard36]

15 minutes ago, diesieben07 said:

You need to serialize and deserialize any data your provider needs to be persistent. 

Wouldnt this make data for every single player the same by that point then? There is no Per Player data to serialize inside of this class, they get attached to the player

 

15 minutes ago, diesieben07 said:

What? You need to serialize/deserialize any capabilities that your provider exposes here (provided these capabilities need to be persistent). I am not sure why you would need the player object for that.

I would need the player object to get the NBT data from that Compatability dont it?

I mean simply looking at this code right here inside of ModCapabilities just straight up confuses me to hell:

 

    @Override
    public CompoundNBT serializeNBT() {
        /* What are you supposed to do here? */
        return null;
    }

    @Override
    public void deserializeNBT(CompoundNBT nbt) {
        /* What are you supposed to do here? */
    }

1) I know for serializeNBT i need to create a new CompoundNBT(), but the actual data values that I set in there where do they come from? Am I just supposed to put any value and return the compoundnbt I created? 

2) I know for deserializeNBT I need to set the nbt object's key/value into variables on a instance of ICurrentStats

I do something similar to this in CurrentStats class (It implement ICurrentStats which extend INBTSerializable<Compound>):
 

    @Override
    public final CompoundNBT serializeNBT() {
        final CompoundNBT comp = new CompoundNBT();
        comp.putLong(NBTTags.CURRENT_HEALTH.toString(), this.getCurrentHealth());
        comp.putLong(NBTTags.MAX_HEALTH.toString(), this.getMaxHealth());
        return comp;
    }

    @Override
    public final void deserializeNBT(final CompoundNBT nbt) {
        this.currentHealth = nbt.getLong(NBTTags.CURRENT_HEALTH.toString());
    }

which is why the ICapabilityProvider's deserializeNBT and serializeNBT are confusing me, theres no data to get or set the values to?

Edited August 2, 2021 by Burchard36
rewording, again sorry its late here

[Burchard36]

2 minutes ago, diesieben07 said:

You (hopefully, if not you need to change this) attach a new instance of your capability provider to every player.

Ohh I see, I have the AttachCapabilitiesEvent<Entity> INSIDE of ICapabilityProvider and setting the Event#addCapability(resoureLocation, this) inside of that class, which isn't creating a new one ill definably fix that up
 

8 minutes ago, diesieben07 said:

From the capability instances in your capability provider. Currently you don't have this, your getCapability just always creates a new instance. Which... yes, will not allow you to save any data. Because you just create a new instance, modify its data and then drop it on the ground. You need to have your capability instances as fields in your capability provider.

This was likely due to me treating it like it managed every single player from the one class, Ill definably rework this as well

That actually makes much more sense now, I was treating the ICapabilityProvider as one class that managed EVERY players Capability object,  I think that was my entire issue here actually, I'll rework this class tomorrow as its 6am here, I'll post what I did and whether it worked or not thanks a bunch!

 

[Burchard36]

9 hours ago, diesieben07 said:

You (hopefully, if not you need to change this) attach a new instance of your capability provider to every player.

No, your capability is a field in your capability provider.

From the capability instances in your capability provider. Currently you don't have this, your getCapability just always creates a new instance. Which... yes, will not allow you to save any data. Because you just create a new instance, modify its data and then drop it on the ground. You need to have your capability instances as fields in your capability provider.

Got it working! Thank you! Now I'm able to edit the values inside of it at runtime even added the function to save the data across deaths, however I have one small issue now:

I was actually testing this on a single-player world at first and realized that the server starting events get fired on a single-player world, I assume that's something new with 1.16 forge starting a internal server when in single-player? If that's the case which it seems like it is, event the packets im sending from the server side are getting sent to the client, so here's my question:


When I leave and rejoin a single-player world, I get this Stack-Trace: Server thread/ERROR] [net.minecraftforge.eventbus.EventBus/EVENTBUS]: Exception caught during firing event: Duplicate Capability Key: dragon_block:current_stats com.dragonblock.lib.player.capabilities.ModCapabilities@634cf181

This happens when I attach my Capability to a player during AttachCapabilitiesEvent<Entity>, I am checking if the Capability exists with ServerEntityPlayer#getCapability#isPresent but its still being added

Event:

 

    @SubscribeEvent
    public void onAttach(final AttachCapabilitiesEvent<Entity> e) {
        if (e.getObject() instanceof ServerPlayerEntity) {
            final ServerPlayerEntity player = (ServerPlayerEntity) e.getObject();
            if (!player.getCapability(ModCapabilities.PLAYER_STATS).isPresent()) {
                e.addCapability(new ResourceLocation("dragon_block", "current_stats"), new ModCapabilities());
            }
        }
    }

This error doesn't happen when this runs on the server though (Leaving and rejoining), so I assume I will have to handle Capabilities differently on single-player? I do have my server sided classes inside of FMLServerStartingEvent and they're getting registered while on a single-player world, so I lead to believe this is setting up a internal server on single-player.

Edited August 2, 2021 by Burchard36
grammar fix

[Burchard36]

17 minutes ago, diesieben07 said:

 

Don't register event handlers in FMLServerStartingEvent. You'll end up registering them multiple times in single player (which will then cause them to fire multiple times, in this case each trying to register the same capability).

Ahh that was it, fixed everything as well as moved my other events now
 

1 hour ago, diesieben07 said:

Don't do that.

So I don't need to check if a player doesn't already have the Capability attached?