Bed bug fix: Remove "reverse" toggle in favor of swap_node/set_node combo

[jeremyshannon]

This is a code change intended to fix the elusive "half-bed" bug. I've never been able to reproduce this in a controlled manner, but I've seen it on servers and heard about it for years. Unable to recreate this bug to study it, I instead scrutinized the logic of beds/api.lua and found a likely culprit in the "reverse" toggle. It's intended to ensure that when a bed top is destroyed, it removes the bottom, but that the bottom doesn't subsequently try to take out another node when its own destruct_bed is called. A measure to prevent chain reactions, it would appear, and a sensible one at first glance.

The trouble seems to be that this is not multiplayer aware and requires a strict order to the destruction of beds: top->bottom->top->bottom. (or vice versa if it the bottom is destroyed somehow) If two people were to destroy beds on a server on the same server tick (and I believe lag will make this more likely), that order could end up as top->top->bottom->bottom. Each bed top could trip the singular "reverse" toggle on and then off, before either of the bed bottoms are destroyed, at which point you get two destroyed tops and only one destroyed bottom, and the toggle left in the on state to cause another bed to split if the server isn't restarted before that.

This fix eliminates the suspect reverse toggle entirely. Now bed halves destroy their mates via swap_node (which doesn't call on_destruct) and set_node to then clear any metadata (such as ownership) that would be destroyed by the old method. This patch has been running on Land of Catastrophe for two months and I have had no reports of half beds appearing in that time.

[sfan5]

The toggle logic definitely looks bad but I don't think the situation you are describing can actually happen. Lua does not execute concurrently and afaik remove_node directly calls the relevant callbacks (no such thing as queuing). So the order should always be bed 1 node_dig -> on_destruct -> other half remove_node -> other half on_destruct, bed 2 node_dig -> [...].

As an alternative solution if the variable was just set to true before the remove_node call and toggled off afterwards I think it would work just as well (bug-free).

[jeremyshannon]

Talking with the other devs tonight reminded me about this. Since this fix was added to Exile's codebase, roughly one year and seven months have now passed without any broken half-beds appearing on Land of Catastrophe, Land of Bugs, or any of our other Exile development servers that we've run. I'm just sayin'. :)