hypermine icon indicating copy to clipboard operation
hypermine copied to clipboard

Server can transmit the same graph nodes twice

Open Ralith opened this issue 5 years ago • 3 comments

When a client first connects, the graph is synchronized to them in full. Each timestep, newly introduced graph nodes from that step are broadcast to all clients. These sets can overlap, leading to the same nodes being added to the graph twice, which produces invalid topology and leads to panics when traversing the graph.

One solution could be to replace the first incremental update with a full sync directly, rather than having an independent initial sync.

Ralith avatar May 17 '20 21:05 Ralith

A more robust solution would be to ensure the initial sync reflects exactly the previous step, which by definition cannot overlap with new nodes introduced in the next step.

Ralith avatar Jan 12 '24 07:01 Ralith

My most recent encounter of the bug was caused by ensure_nearby being called in the server's Sim::new without populating the nodes afterwards, followed by a client connecting before the server ran the first step. Because the nodes were not populated, they were still in the graph's fresh list, but they were also in the graph, causing the initial sync to include nodes that were not fully initialized.

I believe removing that call to ensure_nearby in PR #342 might have fixed this issue entirely, although I have not 100% confirmed that.

In case the above was confusing (since I don't think the vocabulary has been standardized), I should clarify that when I say "populated", I mean that a Node instance was added to the respective node of the graph, and it does not mean that terrain has been generated. To avoid bugs, "populating" a set of nodes should ideally occur immediately after these nodes are added to the graph, as otherwise, code elsewhere (such as in the character controller) has to handle the case of the character having incomplete information about the node it's in. Or, in this case, the server can get confused about which nodes it has transmitted to the client already.

I believe that if this bug is encountered, the client will immediately crash due to a debug_assert!.

patowen avatar Jan 12 '24 22:01 patowen

The most recent occurrence of this bug was fixed in https://github.com/Ralith/hypermine/pull/427.

patowen avatar Aug 12 '24 00:08 patowen