[1.20.1-0.7.41r] Memory Card changes

[GStefanowich]

Describe

I updated recently from 1.20.1-0.7.40r to 1.20.1-0.7.41r to fix #642

It seems like the change to Memory Cards is a bit broken

• Save the owner of the memory card to the inventory manager after a player places the card into the manager and clear the card after. Resolves a security issue where players could eventually steal memory cards from other players

Steps to reproduce

When inserting a bound Memory Card into the Inventory Manager it clears the Memory Card as the feature change says, and saves to the block:

{
  ownerId: [I; -1172986723, 321014915, -1283924489, -1583910946],
  z: 4089,
  x: 9818,
  ForgeCaps: {},
  id: "advancedperipherals:inventory_manager",
  y: -10,
  Items: [
    {
      Slot: 0b,
      id: "advancedperipherals:memory_card",
      tag: {},
      Count: 1b
    }
  ]
}

Removing the blank card afterwards deletes the player link:

{
  z: 4089,
  x: 9818,
  ForgeCaps: {},
  id: "advancedperipherals:inventory_manager",
  y: -10,
  Items: []
}

Without removing the blank card, it appears that after some time, such as leaving and reloading the chunks, or restarting the server, that the Inventory Manager then reads the blank card still present, and also deletes the player link since the card is now "unbound".

Multiplayer?

Yes

Version

1.20.1-0.7.41r (Latest 1.20.1)

Minecraft, Forge and maybe other related mods versions

NeoForge 47.1.106 Minecraft 1.20.1

Screenshots or Videos

No response

Crashlog/log

No response

[zyxkad]

Thanks for reporting the bug, didn't realize about chunk load/unload before. Takeout inventory card to clean owner is expected btw

[GStefanowich]

From a quick glance it appears to be this,

Loading NBT reads the "ownerId" UUID from the block, but it also afterwards calls to setItem

AdvancedPeripherals/src/main/java/de/srendi/advancedperipherals/common/blocks/blockentities/InventoryManagerEntity.java

Lines 73 to 80 in c09b8c0

	public void load(CompoundTag data) {
	if (data.contains("ownerId")) {
	this.owner = data.getUUID("ownerId");
	}
	super.load(data);
	// Fresh the memory card for backward compatibility
	this.setItem(0, this.getItem(0));
	}

The setItem method then checks the presence of the card, to which "ownerId" is not set on the card anymore so owner is set to null

AdvancedPeripherals/src/main/java/de/srendi/advancedperipherals/common/blocks/blockentities/InventoryManagerEntity.java

Lines 54 to 64 in c09b8c0

	public void setItem(int index, @NotNull ItemStack stack) {
	if (stack.getItem() instanceof MemoryCardItem && stack.hasTag() && stack.getTag().contains("ownerId")) {
	UUID owner = stack.getTag().getUUID("ownerId");
	this.owner = owner;
	stack.getTag().remove("ownerId");
	stack.getTag().remove("owner");
	} else {
	this.owner = null;
	}
	super.setItem(index, stack);
	}

[zyxkad]

yep exactly