Minecraftforge: [1.12] Client ItemStack NBT overriding server ItemStack NBT

[MrCrayfish]

So I currently dealing with potentially large amounts of NBT on an ItemStack. Obviously I've looked to Item#getShareTag() and Item#getNBTShareTag() to only send the necessary data for the client and it's worked except for this particular case. If a player is in creative mode and is playing on a server, certain actions cause the NBT of an ItemStack on the server side to be updated by the client side. However the client side only has the necessary data, resulting in the data being lost on server side.

So far I've discovered that these particular actions cause the problem (Must be creative mode and on a server)

• Changing the item's slot
• Dropping the item by clicking it outside the GUI
• Opening the GUI just after receiving an item (within the lag frame)

These have allowed me to discover the problem lies with CPacketCreativeInventoryAction and how it is handle on the server with NetHandlerPlayServer#processCreativeInventoryAction. A similar fix (https://github.com/MinecraftForge/MinecraftForge/pull/3776) was added not long ago but did not cover all situations, such as the one described.

Here is a video demonstrating the problem:
https://youtu.be/tS8E0HGzaNs

Pull request for the fix: https://github.com/MinecraftForge/MinecraftForge/pull/4594

[raoulvdberge]

Seems like this is still a problem @mezz

[TheAwesomeGem]

I have a similar issue. Sometimes when I place boats, it places invisible boats. Upon dropping my boat item and picking it up, I can finally place boats again. This issue sometimes happen randomly.

[donington]

I have a setup where the server synchronizes packets unto the client containing capability data. It remains proper under all survival inventory conditions but as soon as I open inventory in creative mode it is like all of the capabilities are just wiped away on the server side. This is even though the client actually has the data available.

On debugging, CPacketCreativeInventoryAction once the server has it seems to have all the information needed available, but it is still lost somehow. The server suiciding after 60sec of debugging has me at my wits end tonight.

When I have more time this weekend I will try to dig deeper, but the problem to me seems to sit further down the rabbit hole than CPacketCreativeInventoryAction.

[MrCrayfish]

@donington Already made a PR for this issue. https://github.com/MinecraftForge/MinecraftForge/pull/4594

[donington]

Here is what I think is happening. In NetHandlerPlayServer.processCreativeInventoryAction I found this:

...
if (flag1 && flag2)
{
    if (itemstack.isEmpty())
    {
        this.player.inventoryContainer.putStackInSlot(packetIn.getSlotId(), ItemStack.EMPTY);
    }
...

Since GuiContainerCreative doesn't use some kind of packet equivalent to processClickWindow, this gem wipes out the capability data on the server side any time CPacketCreativeInventoryAction is received with an empty ItemStack. There is no tracking of the client mouse in this scenario available either.

The only way I can think to preserve this right now would be to make a cache that fixes incoming ItemStacks since otherwise all the needed information is gone by that point thanks to this.

[HenryLoenwind]

Do I understand this correctly---picking up a stack wipes it on the server so it only exists on the mouse pointer client-side?

Sent from my iPhone

On 16. Jan 2018, at 18:17, donington [e-mail] wrote:

Warning: This message has had one or more attachments removed
Warning: (msg-19299-81.html).
Warning: Please read the "j-e-b-Attachment-Warning.txt" attachment(s) for more information.

Here is what I think is happening. In NetHandlerPlayServer.processCreativeInventoryAction I found this:

...
if (flag1 && flag2)
{
   if (itemstack.isEmpty())
   {
       this.player.inventoryContainer.putStackInSlot(packetIn.getSlotId(), ItemStack.EMPTY);
   }
...

Since GuiContainerCreative doesn't use some kind of packet equivalent to processClickWindow, this gem wipes out the capability data on the server side any time CPacketCreativeInventoryAction is received with an empty ItemStack. There is no tracking of the client mouse in this scenario available either.

The only way I can think to preserve this right now would be to make a cache that fixes incoming ItemStacks since otherwise all the needed information is gone by that point thanks to this.

--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/MinecraftForge/MinecraftForge/issues/4580#issuecomment-358035467

This is a message from the MailScanner E-Mail Virus Protection Service

The original e-mail message contained potentially dangerous content,
which has been removed for your safety.

The content is dangerous as it is often used to spread viruses or to gain
personal or confidential information from you, such as passwords or credit
card numbers.

Due to limitations placed on us by the Regulation of Investigatory Powers
Act 2000, we were unable to keep a copy of the original attachment.

The content filters found this:
MailScanner: Found a script in HTML message

--
Postmaster

MailScanner thanks transtec Computers for their support

[josephcsible]

The item is still there; it's just its NBT that gets wiped.

[HenryLoenwind]

what donington posted looks like:

1. user picks up stack from slot
2. client moves stack from inventory slot to mouse slot
3. client sends "inventory slot = EMPTY" to server
4. server deletes stack in inventory slot
5. user places stack into slot
6. client moves stack from mouse slot to inventory slot
7. client send stack to server
8. server takes stack from network packet ind puts it into inventory slot

Which would mean that my earlier idea of "7.5 server tries to match network packet stack to some inventory slot stack" won't work :(

[donington]

Yes @HenryLoenwind that is what I am observing. I can't think of a solution to this problem that doesn't involve patching GuiContainerCreative.handleMouseClick to send more mouse context to the server, similar to CPacketClickWindow. As it is now there is no way to tell the difference between a mouse clicking a slot and just a general update packet in NetHandlerPlayServer.processCreativeInventoryAction.

Plus the handleMouseClick in question is just a hot mess

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation (e.g. "I have checked the code and this issue is still relevant because ___." or "Here's a screenshot of this issue on the latest version"). Thank you for your contributions!

[josephcsible]

This is still an issue in Forge 2738, just as described in the OP.

[LatvianModder]

Is it still after https://github.com/MinecraftForge/MinecraftForge/pull/4932? You don't even need to store data as NBT anymore, just use capabilities and use new method to sync them with client

[josephcsible]

Ah, I wasn't aware of that, That looks like it gives mods the means to fix it themselves. Given that I still see the problem, I assume the mods just haven't done so.

[malte0811]

I might be missing something here, but how would readShareTag help with this? The main case in which getShareTag is used (AFAIK) is when the server has too much data to send it all to the client without crashing (bags etc). At least that's where we came across this issue in IE, it resets the content of the Engineer's toolbox, which isn't synced unless the GUI is opened. When the sharetag (and therefore the client) doesn't have all data, reconstructing the full data from it should be impossible.

[DrakoAlcarus]

Anyone know if this was patched, and what version that might be on? I can't go too high up, or thaumcraft crashes, but I lost all my progression tools from IE toolboxes, as did my friends. Looking for a fix, besides not using the toolbox.

[loucass003]

This is still an issue in Forge 2759, just as described in the OP.

[MaryWeeb]

Still an issue in latest Forge, any ETA on this?

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation (e.g. "I have checked the code and this issue is still relevant because ___." or "Here's a screenshot of this issue on the latest version"). Thank you for your contributions!

[UK-IN-US]

This issue is still present as of the 14th of July 2019.

[LexManos]

Feel free to propose a solution instead of just bumping the issue.

[ShaneCourtrille]

@LexManos Actually the bot that marked it stale specifically says "This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation" so UK-IN-US was just following the guidance.

[LexManos]

No, it wasn't. "please reply with a short explanation" is the key part here. "still present" doesn't qualify as a short explanation. It's just a bump. Just like yours was. If you want to contribute something USEFUL to the conversation, then that qualifies. So, going to readd the stale tag that should of been kept 2 months ago.

Long Story short this is a fundamental issue with how Vanilla does creative mode. It trusts the client. The client tells the server what items it has, no questions asked. Changing that is on Mojang's side, as it requires a refactor of the entire creative inventory management.

[UK-IN-US]

So I am going to start by stating that my Java knowledge is very poor and my knowledge of Minecraft’s code structure is essentially nil.

That out of the way, how does Thermal handle this issue? The Satchels contain lots of items at higher levels, and I have personally tested with a variety of items with lots of NBT data. They do not have this issue. What is it about their approach that makes it work, whereas this issue still presents itself?

[diesieben07]

There is a mechanism that allows items to customize how NBT data is synchronized to the client. This is to account for the fact that sending around large item stacks like that is inefficient, especially if the data is never needed on the client.
As far as I understand it the bug happens if the player is in creative mode and using an item that makes use of this functionality. The client will use it's "reduced" view of the NBT data and the server will blindly follow (usually it is the other way around).

[stale[bot]]

This issue has been automatically closed because it has not had activity in a long time. Please feel free to reopen it or create a new issue.

[Dakedres]

Encountering a similar issue using build 2854, but my players are reporting it's happening when logging in and out of the server too. Wanted to know if anyone else has encountered this type of behavior or if it's just my pack.