ParadiseSS13 / Paradise

Paradise Station's GitHub main repository.
https://www.paradisestation.org/forum
GNU Affero General Public License v3.0
397 stars 1.2k forks source link

Emptying Laden Satchels of Holding & Ore-boxes can Hang/Crash #8943

Closed KasparoVy closed 6 years ago

KasparoVy commented 6 years ago

Problem Description: You are able to hang/crash the server by sufficiently filling ore boxes/satchels of holding. Although ore-boxes are ever-so-slightly less crashy, but that may be random chance.

What did you expect to happen: No-one should have this ability

What happened instead: I expected the satchel/ore-box to spew my loot without crashing the server

Why is this bad/What are the consequences: You can hang/crash the server with 100% reliability, but it takes time to mine the ore to do so.

Steps to reproduce the problem: Mine the entire mining z-level with an ore box or satchel of holding, then empty it

Possibly related stuff (which gamemode was it? What were you doing at the time? Was anything else out of the ordinary happening?): There were a ton of pugs/corgis/foxes on station along with about 60 active players, 80 clients, but this hangs/crashes the server regardless of other load

KasparoVy commented 6 years ago

Might be a job for CHECK_TICK, stoplag() or some other voodoo like... This is definitely may be relevant due to possible optimizations. Ties into components.

This issue would probably be resolved by implementing /datum/component/storage

Allfd commented 6 years ago

@KasparoVy Thank you! This is super helpful.

@Fox-McCloud @tigercat2000

Fox-McCloud commented 6 years ago

This issue would probably be resolved by implementing /datum/component/storage

Probably not, to be honest. Having dumping things call CHECK_TICK will help some, but the problem still exists, unfortunately.

Making ores a stack will help an ok amount; maybe together they can prevent this, but, to be honest, if someone makes a deliberate point of collecting every single ore on the asteroid, then dumping it all at once---notttt much to prevent that.

KasparoVy commented 6 years ago

Thanks for the insight, /datum/component/storage looked promising is all-- just why I thought I'd mention it

Fox-McCloud commented 6 years ago

Oh, it is promising. It'll be a pain in the butt to implement, but it's infinitely better than storage being either some copy-pasted nightmare snowflake or relying on something to be a child of /obj/item/storage. Wanna add storage to a hat? Just add the component. Wanna add storage to your mob's butt? ...yeah. Or maybe a flower, box, locker, etc.

It's one of those "needed badly, extremely good ideas, but burdensome to implement and no one really wants to do it".

Fox-McCloud commented 6 years ago

Should be resolved for now.