Pryaxis / TShock

☕️⚡️TShock provides Terraria servers with server-side characters, anti-cheat, and community management tools.
GNU General Public License v3.0
2.41k stars 377 forks source link

platinum coins duping themselves #2004

Open Nephthis opened 4 years ago

Nephthis commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 20200613171001_1 20200613171012_1
Quinci135 commented 4 years ago

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 https://github.com/Pryaxis/TShock/blob/bcdd0dc8be9b3985bdc37824d7832a51671cf05a/TShockAPI/GetDataHandlers.cs#L3446 in that file's version Currently https://github.com/Pryaxis/TShock/blob/general-devel/TShockAPI/GetDataHandlers.cs#L3538

Kojirremer commented 3 years ago

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

TheBambino commented 3 years ago

Anyone find any counter measures until a fix is implemented?

Kojirremer commented 2 years ago

imagen Expert pirates, latest stable.

PotatoCider commented 2 years ago

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 commented 2 years ago

image 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)

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

PotatoCider commented 2 years ago

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 commented 11 months ago

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

PotatoCider commented 11 months ago

@Bionear Could you try disabling RangeChecks in config.json? This would hopefully match vanilla item drop behaviour. If my above guess is correct, you should stop duplicating coins

D3RKDUDE commented 11 months ago

And finally someone at least suggested sth.. Thank you bro i will surely try it and will keep you posted...

On Sat, Sep 30, 2023, 07:37 PotatoCider @.***> wrote:

@Bionear https://github.com/Bionear Could you try disabling RangeChecks in config.json? This would hopefully match vanilla item drop behaviour. If my above guess is correct, you should stop duplicating coins

— Reply to this email directly, view it on GitHub https://github.com/Pryaxis/TShock/issues/2004#issuecomment-1741670084, or unsubscribe https://github.com/notifications/unsubscribe-auth/BCVLNJPKTBEZNBGAWCEPWUDX46LJRANCNFSM4NV36BRA . You are receiving this because you were mentioned.Message ID: @.***>

D3RKDUDE commented 11 months ago

@PotatoCider well actually i turned off rangecheck and still theres same problem...

idunnololz commented 3 months ago

I found a way to consistently reproduce the problem. I cleared out a section of my world for a rod of discord farm, afk'd in it and came back to ~1000 platinum coins within an hour. To reproduce this bug:

  1. Mine out a section larger than the monster spawn range.
  2. Build a platform in the middle out of a block. I used pearlstone.
  3. Use a summoner build and summon something. For this I used a Sanguine staff with 5 summons.
  4. Build a 1 block box around your character so most monsters cannot get to you.
  5. Equip a greedy ring. I think the accessory that increases coin pickup range is all you need though but I did this test with a greedy ring.
  6. Afk in the middle of platform.

Essentially what will happen is monsters will start dying both outside your coin pickup range and inside your coin pickup range. Within ~15 minutes the amount of coins outside your pickup range will become extremely substantial and it creates a positive feedback loop where monsters will pickup the coins, die and then duplicate those coin. Eventually this leads to monsters dropping multiple platinums. When these die in your pickup range you starts to amass an insane amount of coins while the coins outside the pickup range continues to feed this cycle.

Video showing this bug: https://www.youtube.com/watch?v=6KAf1HUzpDE

More information: It doesn't appear that you need multiple people to be signed on. I was the only one online on the server when I reproduced this issue.

PotatoCider commented 3 months ago

@idunnololz Is it possible for you to provide a world download?

idunnololz commented 3 months ago

Yep. Here's the world files & config: worlds.zip

idunnololz commented 3 months ago

Oh yeah here is a map of the world. I also highlighted in red where the video was taken/where you can reproduce the issue: Screenshot 2024-06-19 140112