[1.15.2] Middle-click-copying an itemstack resets capabilities for all itemstacks of that item

[T-Dark_]

I have an item which consumes some energy from its capability and teleports the player on right click. It works fine both in creative and in survival.

Until I duplicate the item by middle-clicking in creative mode. After that, the capability data of every single existing itemstack, as well as the new one, is reset to its default value of "0 energy".

I am aware that, in creative mode, the client tells the server what to sync, instead of the other way around. The strange part is that ALL itemstacks are affected, instead of just the copied one, as I would expect.

Returning to survival mode doesn't fix the problem. While, restarting the game does.

What could be causing this, and how do I fix it? I tried to take a look at get and read ShareTag, but they had no visible effect. They didn't even alter the nbt tag.

Here is the problematic file, in the code's repo.

[TheGreyGhost]

20 minutes ago, diesieben07 said:

Your implementation of getShareTag and readShareTag is wrong.

They must be an extension of the normal ItemStack tag. Please refer to the Javadocs.

What are getShareTag and readShareTag supposed to be used for?

ItemStack NBT stores the Capability information in ForgeCaps, I thought?  The only issue I had getting it to work was marking the ItemStack as dirty on the server in order to force it to synch to the client.

[TheGreyGhost]

57 minutes ago, T-Dark_ said:

What could be causing this, and how do I fix it?

Try implementing ICapabilitySerializable<INBT>

 

eg like this

https://github.com/TheGreyGhost/MinecraftByExample/blob/master/src/main/java/minecraftbyexample/mbe32_inventory_item/CapabilityProviderFlowerBag.java

 

-TGG

 

[T-Dark_]

9 minutes ago, TheGreyGhost said:

Try implementing ICapabilitySerializable<INBT>

 

When I did that, I had a problem: I need to know the energy level of the item client-side to display it in a tooltip. That's why I ended up writing to the stack's standard nbt instead: that's synced to the client automatically.

 

Am I supposed to use packets to do that?

[T-Dark_]

38 minutes ago, diesieben07 said:

Your implementation of getShareTag and readShareTag is wrong.

They must be an extension of the normal ItemStack tag. Please refer to the Javadocs.

I did read the javadocs, and what I got is that if you get the share tag, set the stack's tag to what you just got, and get the share tag again, the two share tags must be equal.

 

Did I misunderstand the requirement? Is getShareTag supposed to return the entire tag, including the share tag?

[T-Dark_]

So there is a getTag method, which returns the tag without the share tag, and a getShareTag method, which still returns the whole tag, but this time with the share tag?

 

And the way you use them is by overriding getShareTag to "serialize" your data (and return the entire nbt tag, not just the data you serialized), and overriding readShareTag to "deserialize" your data? Is the share tag a sub-compound inside the main tag (like forgeCaps), or can it be flattened into the main tag?

[TheGreyGhost]

Howdy DieSieben

23 hours ago, diesieben07 said:

The capability NBT is separate from the normal stack NBT and not synced to the client by default. getShareTag and readShareTag can be used to change this.

Are you sure?  I don't think that's right.  I wrote a tutorial example which attaches a capability to an ItemStack, changing the rendering on the client based on the capability, and it worked fine.

 

When I look at the Forge code it appears to send the caps nbt as well as the vanilla nbt.

   /**
    * Write the stack fields to a NBT object. Return the new NBT object.
    */
   public CompoundNBT write(CompoundNBT nbt) {
      ResourceLocation resourcelocation = Registry.ITEM.getKey(this.getItem());
      nbt.putString("id", resourcelocation == null ? "minecraft:air" : resourcelocation.toString());
      nbt.putByte("Count", (byte)this.count);
      if (this.tag != null) {
         nbt.put("tag", this.tag.copy());
      }
      CompoundNBT cnbt = this.serializeCaps();
      if (cnbt != null && !cnbt.isEmpty()) {
         nbt.put("ForgeCaps", cnbt);
      }
      return nbt;
   }

 

The one thing I did have to do was mark the ItemStack as dirty (after updating the capability) in order to force it to be resynched to the client:

 

      CompoundNBT nbt = itemStackBeingHeld.getOrCreateTag();
      int dirtyCounter = nbt.getInt("dirtyCounter");
      nbt.putInt("dirtyCounter", dirtyCounter + 1);
      itemStackBeingHeld.setTag(nbt);

(dirtyCounter is an arbitrary nbt tag)

 

-TGG

[TheGreyGhost]

5 hours ago, diesieben07 said:

Probably because you tested in single player. getShareTag literally only comes in to play when connecting to a real server (or with custom packets). Because in singleplayer vanilla's networking does a "clever optimization" and does not serialize and deserialize it's packets to a bytestream, it just passes them through a concurrent queue. That causes the ItemStacks to just be copied, which copies the capability tag. On an actual network connection, getShareTag is used (see PacketBuffer#writeItemStack).

hmm ok that would explain it.  I'll do a bit more testing to figure out the best way to work around that (i.e. other than custom packets) for ItemStacks as well as Entity and the others.

 

Do you think that the Forge overlords would support the addition of a "transmitToClient" flag for capabilities?  Do you know who designed it / is maintaining it?  It appears to be Lex.

I'm more than happy to code it up and submit a pull request but I'm not so keen to waste a few days doing it only to have it rejected out of hand.

 

-TGG

[TheGreyGhost]

On 7/8/2020 at 5:07 PM, diesieben07 said:

Well, 90% of the time you don't want everything to be synced. So then the capabilities need a separate thing that will sync to the client... I seem to vaguely remember there being a PR for this in a the past but I don't remember how it went.

Well my basic thought is something like

public interface ICapabilitySerializable<T extends INBT> extends ICapabilityProvider, INBTSerializable<T> {
  default T serializeNBTforPacketToClient() {return null;}
}
  
CapabilityProvider::
protected final @Nullable CompoundNBT serializeCapsForPacketToClient()
    {
        final CapabilityDispatcher disp = getCapabilities();
        if (disp != null)
        {
            return disp.serializeNBTforPacketToClient();
        }
        return null;
    }
  
IForgeItem::
    default CompoundNBT getShareTag(ItemStack stack)
    {
        CompoundNBT cNBTbase = stack.getTag();
        CompoundNBT cNBTcaps = this.serializeCapsForPacketToClient();
      if (cNBTcaps != null && !cNBTcaps.isEmpty()) {
         cNBTbase.put("ForgeCaps", cNBTcaps);
      }
    }  
  

Seems simple enough.

 

A lambda that the capability can use to inform the ItemStack (or Entity, World, etc) that it is dirty would also be useful.