Pulling needs cleanup

[metalgearsloth]

Ideally:

• Probably shouldn't use the distancejoint (even though it's useful for me for debugging).
• Combine its 4 systems into 1.
• Remove the statemanagementsystem.
• Remove the GetComponents everywhere because these have a tendency to cause runtime exceptions very easily.
• MovingTo is disgusting and legacy from pre-box2d.

[mirrorcult]

move all of the logic out of giant fucking setters please

[20kdc]

the state management system was created in the wake of tons of pulling state bugs because people kept mucking around with the pulling state when they really shouldn't, failing to trigger the necessary events. It might be an idea to move things like pull joint creation into event handlers, but I would not recommend removing the state management system.

[20kdc]

Just to be totally clear, the separation of the state management system was done on purpose as a defensive measure against accidental state-corrupting changes.

There appeared to be confusion about which functions were actually supposed to mutate state, i.e. https://github.com/space-wizards/space-station-14/blob/e801ffac45bc70edccd4b5233f1de7c227399741/Content.Shared/Pulling/Components/SharedPullableComponent.cs#L260 wasn't supposed to be messing with _pullJoint.

The State Management System should make this clearer.

[metalgearsloth]

I'm still not convinced the system isn't a bandaid tbh

[20kdc]

if you have a better way to guarantee that only the specific setup functions access the state fields, please implement it

[metalgearsloth]

hence the issue...

[gradientvera]

Literally just make a single system that handles this, and EXPLICITLY say using comments which methods should be used to modify the component state. With Friend, you can prevent anything but the system that handles pulling write the component's members, which in my opinion is way more than enough. You don't need a snowflake system to handle the state, just let a single system handle all this and leave comments so people don't fuck it up.