Jump to content

[1.17.1] Player capabilities are invalidated on death before they can be copied


Danny and Son

Recommended Posts

When a player dies, and I attempt to copy my custom capabilities data from the original player object to the new player object in the PlayerEvent.Clone event, the getOriginal().getCapability method is not giving me the capability. Upon further inspection in the Debugger, the capability is marked as invalid. Specifically, event.original.capabilities.caps[0].mycapabilityLazyOptional.isValid = false. This is unique to 1.17.1. In 1.16.5, with the same code isValid = true, and the data is copied as expected.

All other functionality seems to be working fine in 1.17.1. Data is being saved when a player leaves and rejoins, and can be found in their player data file NBT. It can be queried by other methods and classes while the player is alive. It is only on death that the capability data of the original player object can not be retrieved.

Do we need to take a different approach in 1.17.1 or is something not working as expected?

Link to comment
Share on other sites

Yes this is intentional.
The system we had before is no longer valid for the way Mojang designed the code in 1.17.
Modders now have to explicitly revive and then invalidate their entities as designed in my commit.

I do Forge for free, however the servers to run it arn't free, so anything is appreciated.
Consider supporting the team on Patreon

Link to comment
Share on other sites

That doesn't seem to work unless I'm doing it incorrectly:

    @SubscribeEvent
    public void onPlayerDeath(PlayerEvent.Clone event)
    {
        if (event.getEntity() instanceof Player playerEntity) {

            event.getOriginal().reviveCaps();
            event.getOriginal().getCapability(CapabilityNutritionalBalancePlayer.HEALTHY_DIET_PLAYER_CAPABILITY).ifPresent(originalInutritionalbalancePlayer -> {

                playerEntity.getCapability(CapabilityNutritionalBalancePlayer.HEALTHY_DIET_PLAYER_CAPABILITY).ifPresent(newInutritionalbalancePlayer -> {


                  . . . .

                });

            });
            event.getOriginal().invalidateCaps();
        }

    }

After running event.getOriginal().reviveCaps(), the caps are still invalid, so the lambda code is never executed.

Similarly for dimension changes:

    @SubscribeEvent
    public void onDimensionChange (PlayerEvent.PlayerChangedDimensionEvent event){
        event.getEntity().reviveCaps();
    }

The caps are still invalid after executing this.

Am I doing something wrong?

Link to comment
Share on other sites

My provider is returning a valid cap in every other circumstance though. Just not during cloning or dimension changes. Data is saved on leave and rejoin. GUI elements that query the caps are working as expected. Other events that are taking actions based on values within the caps are working.

In the debugger, I can see all the data in my cap in the 2 above scenarios. It is exactly what it's supposed to be, but
event.entity.capabilities.caps[0].nutritionalBalanceLazyOptional.isValid = false
and it remains so after calling reviveCaps().

Other than that, the debugger is showing that all the other data is exactly what it's supposed to be. What would cause isValid to be false?

Link to comment
Share on other sites

It might help to add that when I check the player object at any other point, such as when opening the mod's GUI, nutritionalBalanceLazyOptional.isValid = true. So, I'm guessing it's being invalidated by the death and dimension change, and reviveCaps() is not changing that. Based on my understanding of the commit you referenced above, reviveCaps() is supposed to change that to true. Right?

Link to comment
Share on other sites

  • 2 weeks later...

Can someone confirm that Player.reviveCaps() does, indeed, restore capabilities attached to the player? This does not appear to be happening in my case. I have thoroughly debugged my code (otherwise I would not have opened this thread), and the only issue appears to be that LazyOptional.isValid becomes false on death and dimension change, and this is not changed to true when calling reviveCaps().

In short, reviveCaps is not doing what I think it's supposed to do. So, either it's not working correctly, or I am misunderstanding how it's supposed to work.

Link to comment
Share on other sites

  • 4 weeks later...

As far as I know, no solution has been found. I might need to open an issue on the MinecraftForge GitHub.

I have a couple questions for you just to make sure we're both dealing with the same issue:

Are your capabilities working when the player leaves and rejoins?
Have you also tried using reviveCaps() with no success?
Is your code posted publicly or can you share an excerpt of your PlayerEvent.Clone event?

Link to comment
Share on other sites

  • 3 weeks later...

The reviveCaps() worked for me. I'm using forge 37.0.67.

My Clone Event:

public static void onDeath(PlayerEvent.Clone event) {
    if (event.isWasDeath()) {
        event.getOriginal().reviveCaps();
        event.getOriginal().getCapability(ModCapabilityImpl.MOD_CAPABILITY).ifPresent(oldStore -> {
            event.getEntity().getCapability(ModCapabilityImpl.MOD_CAPABILITY).ifPresent(newStore -> {
               newStore.copyForRespawn((ModCapabilityImpl) oldStore);
            });
        });
        event.getOriginal().invalidateCaps();
    }
}

In my IModCapability:

void copyForRespawn(ModCapabilityImpl oldStore);

In my ModCapabilityImpl:

@Override
public void copyForRespawn(ModCapabilityImpl oldStore) {
	this.value = oldStore.value;
}

Hope this helps!

  • Thanks 2
Link to comment
Share on other sites

On 10/6/2021 at 4:20 PM, DaqEm said:

The reviveCaps() worked for me. I'm using forge 37.0.67.

Interesting. Thanks for sharing that. I haven't tested it since 37.0.30. I ended up changing over to SaveData. It's much simpler to work with, gives me a lot more control, and I don't have to worry as much about it breaking every time Minecraft updates. The only drawback is that the data will persist even if player data files are deleted. They would have to delete the custom data file instead. I don't think that's a big issue, especially given that the tradeoff is that the data also persist through death, dimension changes or whatever without having to play any dirty tricks.

Link to comment
Share on other sites

  • 1 month later...
On 9/21/2021 at 4:17 PM, Danny and Son said:

As far as I know, no solution has been found. I might need to open an issue on the MinecraftForge GitHub.

I have a couple questions for you just to make sure we're both dealing with the same issue:

Are your capabilities working when the player leaves and rejoins?
Have you also tried using reviveCaps() with no success?
Is your code posted publicly or can you share an excerpt of your PlayerEvent.Clone event?

The information is saved if I add listeners in AttachCapabilitiesEvent:

event.addCapability(key, provider);
event.addListener(provider.capOptional::invalidate);

However, I get NullPointerException when going from dimension to dimension at this line in EntityJoinWorldEvent.

player.getCapability(MistCaps.CAPABILITY_MIST).orElseThrow(() -> new NullPointerException("Player has no mist capability"));

If I do not add listeners, then everything works fine when changing the dimension, but the information is not saved when I restart the game.

Of course I have tried using reviveCaps(). No result.

Sorry, I don't speak English very well...

Link to comment
Share on other sites

But how can I save information when restarting the game?

I use this example:

https://github.com/VazkiiMods/Botania/blob/0c1138252901ea646f6f97f9427f62ccd258e9d3/src/main/java/vazkii/botania/common/capability/SimpleCapProvider.java#L42

public class SimpleCapProvider<C extends INBTSerializable<CompoundTag>> implements ICapabilityProvider, INBTSerializable<CompoundTag> {

	private final C instance;
	private final LazyOptional<C> capOptional;
	private final Capability<C> capa;
	
	public SimpleCapProvider(Capability<C> capa, NonNullSupplier<C> instance) {
		this.capa = capa;
		this.instance = instance.get();
		this.capOptional = LazyOptional.of(instance);
	}

	@Nonnull
	@Override
	public <T> LazyOptional<T> getCapability(@Nonnull Capability<T> cap, @Nullable Direction side) {
		return capa.orEmpty(cap, capOptional);
	}

	@Override
	public CompoundTag serializeNBT() {
		return this.instance.serializeNBT();
	}

	@Override
	public void deserializeNBT(CompoundTag nbt) {
		this.instance.deserializeNBT(nbt);
	}

	public static <C extends INBTSerializable<CompoundTag>> void attach(AttachCapabilitiesEvent<?> event, ResourceLocation key, Capability<C> cap, NonNullSupplier<C> capInstance) {
		SimpleCapProvider<C> provider = new SimpleCapProvider<>(cap, capInstance);
		event.addCapability(key, provider);
		//event.addListener(provider.capOptional::invalidate); //TODO Capa invalidate
	}
}

And this is my capa registration

public class MistCaps {

	@CapabilityInject(IMistCapaHandler.class)
	public static Capability<IMistCapaHandler> CAPABILITY_MIST;

	@CapabilityInject(ISkillCapaHandler.class)
	public static Capability<ISkillCapaHandler> CAPABILITY_SKILL;

	@CapabilityInject(IFoodHandler.class)
	public static Capability<IFoodHandler> CAPABILITY_FOOD;

	private static final List<CapaEntry<? extends INBTSerializable<CompoundTag>>> capaList = Lists.newArrayList();

	public static void init(RegisterCapabilitiesEvent event) {
		register(event, "player_capa", IMistCapaHandler.class, () -> CAPABILITY_MIST, MistCapaHandler::new);
		register(event, "skill_capa", ISkillCapaHandler.class, () -> CAPABILITY_SKILL, SkillCapaHandler::new);
		register(event, "food_capa", IFoodHandler.class, () -> CAPABILITY_FOOD, FoodCapaHandler::new);
		Mist.LOGGER.info("Misty caps has been registered!");
	}

	//////////////////////////////////////////////////////// MAGIC ////////////////////////////////////////////////////////

	private static <C extends INBTSerializable<CompoundTag>> void register(RegisterCapabilitiesEvent event, String name, Class<C> clazz, NonNullSupplier<Capability<C>> capa, NonNullSupplier<C> instance) {
		event.register(clazz);
		capaList.add(new CapaEntry<C>(Mist.resLoc(name), capa, instance));
	}

	@SubscribeEvent
	public void attachCapabilitiesPlayer(AttachCapabilitiesEvent<Entity> event) {
		if (event.getObject() instanceof Player) capaList.forEach(entry -> attach(event, entry));
	}

	@SubscribeEvent
	public void cloneCapabilitiesEvent(PlayerEvent.Clone event) {
		capaList.forEach(entry -> clone(event, entry));
	}

	private static <C extends INBTSerializable<CompoundTag>> void attach(AttachCapabilitiesEvent<?> event, CapaEntry<C> entry) {
		SimpleCapProvider.attach(event, entry.res, entry.capa.get(), entry.instance);
		if (event.getCapabilities().get(entry.res) == null) {
			Mist.LOGGER.error("Player didn't attach [" + entry.capa.get().getName() + "] capa");
		} else Mist.LOGGER.info("Player has attached [" + entry.capa.get().getName() + "] capa");
	}

	private static <C extends INBTSerializable<CompoundTag>> void clone(PlayerEvent.Clone event, CapaEntry<C> entry) {
		try {
			//event.getOriginal().reviveCaps();
			C original = event.getOriginal().getCapability(entry.capa.get()).orElseThrow(NullPointerException::new);
			CompoundTag nbt = original.serializeNBT();
			C clone = event.getPlayer().getCapability(entry.capa.get()).orElseThrow(NullPointerException::new);
			clone.deserializeNBT(nbt);
			//event.getOriginal().invalidateCaps();
		} catch (Exception e) {
			Mist.LOGGER.error("Could not clone capability [" + entry.capa.get().getName() + "] when player [" + event.getOriginal().getName() + "] changing dimensions");
		}
	}

	private static class CapaEntry<C extends INBTSerializable<CompoundTag>> {

		private final ResourceLocation res;
		private final NonNullSupplier<C> instance;
		private final NonNullSupplier<Capability<C>> capa;
		
		public CapaEntry(ResourceLocation res, NonNullSupplier<Capability<C>> capa, NonNullSupplier<C> instance) {
			this.res = res;
			this.instance = instance;
			this.capa = capa;
		}
	}
}

 

Edited by Liahim

Sorry, I don't speak English very well...

Link to comment
Share on other sites

8 hours ago, Luis_ST said:

you still save infos, in your SimpleCapProvider via the implementation of INBTSerializable (#serializeNBT and #deserializeNBT)

But capa didn't save during restarting the game.

7 hours ago, Danny and Son said:

Is your init function being called? It doesn't look like your subscribing to RegisterCapabilitiesEvent.

All events are subscribed in my main class. As I wrote in this message 

All methods work fine and information is saved when I use event.addListener(provider.capOptional::invalidate); 

But I am getting NullPointerException when going from dimension to dimension when trying to get a cap on the EntityJoinWorldEvent.

Edited by Liahim

Sorry, I don't speak English very well...

Link to comment
Share on other sites

23 hours ago, Liahim said:
		register(event, "player_capa", IMistCapaHandler.class, () -> CAPABILITY_MIST, MistCapaHandler::new);
		register(event, "skill_capa", ISkillCapaHandler.class, () -> CAPABILITY_SKILL, SkillCapaHandler::new);
		register(event, "food_capa", IFoodHandler.class, () -> CAPABILITY_FOOD, FoodCapaHandler::new);

show one of these CapaHandlers (the implementation not the interface)

Link to comment
Share on other sites

public class MistCapaHandler extends ItemStackHandler implements IMistCapaHandler {

	private Player player;
	private int pollution;
	private int toxic;

	public MistCapaHandler() {}

	// ...

	@Override
	public CompoundTag serializeNBT() {
		CompoundTag nbt = super.serializeNBT();
		nbt.putInt("Pollution", this.pollution);
		nbt.putInt("Toxic", this.toxic);
		return nbt;
	}

	@Override
	public void deserializeNBT(CompoundTag nbt) {
		super.deserializeNBT(nbt);
		this.pollution = nbt.getInt("Pollution");
		this.toxic = nbt.getInt("Toxic");
	}
 
    // ...
}

 

Sorry, I don't speak English very well...

Link to comment
Share on other sites

I fixed it!
The point was that I worked with different instances of the cap in this place:

public SimpleCapProvider(Capability<C> capa, NonNullSupplier<C> instance) {
	this.capa = capa;
	this.instance = instance.get();
	this.capOptional = LazyOptional.of(instance); // <-----------
}

But this is correct:

public SimpleCapProvider(Capability<C> capa, NonNullSupplier<C> instance) {
	this.capa = capa;
	this.instance = instance.get();
	this.capOptional = LazyOptional.of(() -> this.instance); // <-----------
}

Sorry, I don't speak English very well...

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Announcements



×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.