Movement rework

[lewc]

I've been tossing this around as something I'd like to do, but I guess this is the best place to formalize it. The resulting change would probably hit everywhere in the codebase, so I also wanted to make sure it could be run by maints as needed.

 

In my work on refactoring the orbiter, I reworked orbiting to follow COMSIG_MOVABLE_MOVED signals, where if objects are moved, the signals indicate that. There's a bit more detail with separate MOVED signals of moving things between containers, but I won't get into the nitty-gritty there.

After the refactor went live, I've seen at least three or four different cases where orbiting has broken where it previously used to work: disposals, vent crawling, table climbing, etc. The root cause is pretty much the same across each issue, where an atom is moved by directly setting its .loc. This is easy and convenient to write, but circumvents any general movement handling from being applied on it. In particular, we never get Moved(), which does a few things like sending the MOVED signal and updating parallax/client views. This isn't good for orbiters, proximity monitors, or anything else that relies on signals to know when something's moved.

As for the fix, I've been doing some research into how /tg/ handles their movement. It seems to me that they've settled on two basic move methods:

 

- abstract_move() for anything that should have no side effects from movement and shouldn't be affected by anything when moving. This includes things like the AI eye and observers, and the /tg/ function docs note that it could be good for moving something onto a tile with a bear trap or something (possibly skipping crossed() signals).

 - forceMove() for literally everything else. Seriously, I'm pretty sure there was an effort to replace all .loc calls with forceMove.

 

ForceMove already exists in our code, but abstractmove would be a new addition. It shouldn't need to go into too many places, so it should probably be an easier first addition. After that, though, it might come down to switching every manual setting of .loc to a forceMove (save for the rare cases where abstractmove would be better). I think it'd be best to do in pieces if possible, since I already ran into some bugs while trying to get vent crawling working with it (it resets the client's perspective and can fire off some other behavior). If we could document and understand exactly what is going on deep below this proc (particularly in the depths of Moved(), including its overrides), we could probably work out most of the issues before plugging it into everything.

I know I'm still a fairly new contributor to the codebase, so it's possible I could be off the mark on something significant, but I feel pretty confident in the fact that this would not only make my life easier w/r/t managing each little orbiting bug as they appear, but also pave the way for more things to be able to consistently rely on movement signals, like steel's proximity monitor component. I'd be happy to try and take the lead on this, as a result.

[Sirryan2002]

Given the strain movement/key-input puts on our master controller/CPU, I wonder how changing this up would affect that for better or worse. Is setting loc manually more CPU intensive then using forcemove() ?

[lewc]

Right, so there's Move() as well in our codebase (I think doMove() on /tg/) which handles key-input type movement where diagonals need to be accounted for as well. I would have to look into the usage of that a bit more.

But to answer your question, no; forceMove sets .loc as one of the first things it does. There may be a little more overhead on using forceMove() over .loc setting, since it calls Moved() and fires other enter/exit signals.