platinum coins duping themselves

[Nephthis]

• TShock version: 4.4.0 pre 11

I noticed some strange behaviour with platinum coins (and possibly other coins).
Unfortunately I didn't have debug logs active at the time and the problem seems to be difficult to reproduce but I will try to add some logs as soon as I can reproduce the bug.
For now I can only describe it:

when I was killed in an artificial underground crimson biome I dropped about 70 platinum, I came back to it, killed a few enemies to get there and collected about 170 platinum.
Later I had about 280 platinum coins with me, got killed and collected 816 platinum coins when I came back.
It appears that if an enemy collects the coins and drops them when you kill it the amount is significantly higher than the amount it collected (though I can't verify as of yet that the coins were indeed picked up by an enemy).
If it helps, at the time of my deaths there were the following enemies present:
Giant Bat,
Skeleton Archer,
Armored Skeleton,
Rock Golem,
Ichor Sticker,
Crimslime,
Herpling

I will try to reproduce the event but it's rather difficult to get the right enemy to pick up the coins.

[Nephthis]

followup:
this is definitely an issue with enemies picking up coins and duping them, I have attached my server-logs, hopefully they provide a clue to what is happening (and if it's my fault somehow)
2020-06-06_14-25-46.log

[Olink]

Someone probably needs to check this code:
https://github.com/Pryaxis/TShock/blob/general-devel/TShockAPI/GetDataHandlers.cs#L3446

Probably need to pull code out to Bouncer, and also probably if we handle it we need to send the packet back to the user with a value of 0.

[marda7]

We have the same issue on our server. All coins are affected I think.
It looks like monsters picking up the coins (animation happens) but the coins are not removed from floor in that moment.

Steps to reproduce:

1. Build a farm similar to the screenshots.
2. Don't loot all the money beside in the mid (maybe even drop money outside the mid? Didn't test)
   As more money drops/time passes, you get exponentialy more money over time (starting with afew gold, leading to 100+ plat after 30 min per monster, depending on how much it looted. Mummys seem to pickup the most/drop the most in my setup)
3. Kill monsters, some should make the way too the mid where you kill them. (I used a raven and went afk). After 30-60 min I had 8 stacks platinum, sometimes one mob is dropping 300+ platinum coins
   
   

[Quinci135]

Think it might be a vanilla multiplayer bug actually and might go something like this
Money is on ground
On item update which happens on game update
Check if money item in contact with a valid npc that can pickup coins (in Terraria.ID.NPCID name is CantTakeLunchMoney)
Roll number to see how much money is taken, and remove amount from coin stack. If less than 0, deactivate item.
If on multiplayer, send packet to notify server/client
If valid npc (0-200) add the extra value to the npc
But clients are all sending the extra value to the server?
Server tells clients there's some coins on the ground
All clients notice that an npc has picked up the coins
All clients try and send the extra value packet
Server receives extra value, so Main.Npc[npcIndex].extravalue += client's value
Which stacks with all players
Unless I've got this wrong, shouldn't the server just ignore this packet from clients?

The linked section in GetDataHandlers would be

TShock/TShockAPI/GetDataHandlers.cs

Line 3446 in bcdd0dc

private static bool HandleSyncExtraValue(GetDataHandlerArgs args)

in that file's version
Currently https://github.com/Pryaxis/TShock/blob/general-devel/TShockAPI/GetDataHandlers.cs#L3538

[Kojirremer]

Guess it's obvious, but I'll just add that it happens in both expert and master difficulties.

[TheBambino]

Anyone find any counter measures until a fix is implemented?

[Kojirremer]

Expert pirates, latest stable.

[pcider]

To keep everyone on this issue up to speed, this issue most likely resurfaced after my PR #2601 which was merged in the latest version (v4.5.17).

[Quinci135]

In theory what's happening from my previous description, I think it's a vanilla issue itself though haven't tested

Maybe server-side check if any one player has added
Any value to that npc in the last X seconds (since amount taken by npc is rng + amount of coins on ground)

Another untested thought that comes to mind is maybe clients attempt to subtract the money added to the npc from the coins on the ground but are unable to by some anti-cheat such as distance or item stack that isn't yours modification checks thus the coins remain there and clients will continue adding money to the npc repeatedly alongside the aforementioned mirroring effects of above. If tshock caused maybe check for recent npc sync extra value and allow coin stack modification|deletion if nearby npc or matching coin values, though not sure what is networked first.

Coin taken from ground stack rng (see Terraria.Item.GetPickedUpByMonsters)

• Default 50-76% money taken rng roll (so for gold and platinum)
• If only copper coins, +0-51% taken so 50-127% but effectively 50-100%
• If only silver coins, +0-26% taken so 50-102% but effectively 50-100%

Codewise extra value packet is sent before item stack editing (local variable names from '.pdb' which for some reason comes with terraria mobile server, N being npc id)
'NetMessage.SendData(92, -1, -1, null, N, extraMoney, position.X, position.Y);'
'NetMessage.SendData(21, -1, -1, null, i);'

Neither replicated in a vanilla nor tshock server yet for me so can't say exactly what it is

[pcider]

Koji stated on discord that only one player was online. My guess is that your 2nd thought is most likely the case.

Bouncer / OnItemDrop rejected from range check from Dodge appears 868 times in the logs above, anddupe range check appears 64 times. However, there is no GetDataHandlers / HandleSyncExtraValue log sent, meaning that all SyncExtraValue packets probably went through.

Reference:
https://github.com/Pryaxis/TShock/blob/v4.4.0-pre11/TShockAPI/Bouncer.cs#L608
https://github.com/Pryaxis/TShock/blob/v4.4.0-pre11/TShockAPI/GetDataHandlers.cs#L3446
https://discord.com/channels/479657350043664384/479657350043664386/985960999230263316

[D3RKDUDE]

I have this problem in my server too. I really need it to be fixed... just tell me how!