shanemadden / factorio-deadlock-beltboxes-loaders

Other
8 stars 16 forks source link

Fix global scope of variables which could lead to desyncs #20

Closed Deadlock989 closed 4 years ago

Deadlock989 commented 4 years ago

boskid was hassling me about this a few months back

Deadlock989 commented 4 years ago

Consider removing all this auto-unstacking stuff by ownlyme, given the exploits?

Deadlock989 commented 4 years ago

Just to clarify - I don't think this is actually leading to any desyncs - we would have heard about it by now. You don't get a desync from just a variable, it has to be a variable that decides some action that affects the game state. But global variables whose values are changed after initialisation are bad practice in any Factorio control script and people's validation tools are flagging it as a risk.

shanemadden commented 4 years ago

Ughhhh the auto unstacking. It defaults to disabled so I really don't feel too bad about it being buggy - I haven't had a chance to dig into the dupe exploit so maybe it's worth trying to shore up with one more fix, but.. I'm clearly not willing to put a ton of effort into it. The immediate follow on of 'well without that, if we use batch unstacking we need 1 stack unstacking recipes' takes us back down the road of having a separate set of hand-unstacking recipes which I just don't want to deal with. This is what I get for adding features.