Replace NodeId implementation with global hash

[patowen]

Having NodeId be the global hash that is guaranteed to be consistent across the server and its clients allows simpler reasoning about code. The main downsides are

• It's 4 times larger, being a u128 instead of a u32 internally, which could potentially impact performance.
• The graph has to use a map instead of an array for its internal data representation, also potentially impacting performance.

The advantages are

• There is only one NodeId, removing ambiguity in discussions and in variable names etc.
• There is no risk of introducing bugs later on by accidentally assuming that a local NodeId means the same on the server and client.

I don't expect NodeId's size to be the source of any bottlenecks, as each node is a large enough part of the data structure, that the handling of individual voxels and/or the matrix math for each node is likely to take much more processing time. Because of this, I think it's likely that this change is worthwhile, but I'm not 100% convinced.

The most complicated change is the rewriting of the tree() traversal function in Graph, since the original design was not compatible with the hashmap-based graph implementation, so it was replaced with a breadth-first search. It is only used for synchronizing the graph between the server and client. Ideally, we wouldn't need to do that anymore, but trying to naively remove this functionality would introduce a subtle bug because the server could move the player or another entity to a node the client doesn't know about, and the client does not and cannot properly handle that.

[Ralith]

There is only one NodeId, removing ambiguity in discussions and in variable names etc.

I think this is compelling. We can always reintroduce local IDs if this shows up in profiling in the future, and otherwise we'll benefit from a simpler model.

There is no risk of introducing bugs later on by accidentally assuming that a local NodeId means the same on the server and client.

I don't think this is a significant risk; we should just never transmit NodeIds over the wire, which shouldn't be too hard to keep track of.

Ideally, we wouldn't need to do that anymore, but trying to naively remove this functionality would introduce a subtle bug because the server could move the player or another entity to a node the client doesn't know about, and the client does not and cannot properly handle that.

We should treat such behavior from the server as a critical bug. That said, it's good to make changes like this incremental, so it makes sense to get rid of graph sync in a second pass.

[patowen]

There is only one NodeId, removing ambiguity in discussions and in variable names etc.

I think this is compelling. We can always reintroduce local IDs if this shows up in profiling in the future, and otherwise we'll benefit from a simpler model.

There is no risk of introducing bugs later on by accidentally assuming that a local NodeId means the same on the server and client.

I don't think this is a significant risk; we should just never transmit NodeIds over the wire, which shouldn't be too hard to keep track of.

Ideally, we wouldn't need to do that anymore, but trying to naively remove this functionality would introduce a subtle bug because the server could move the player or another entity to a node the client doesn't know about, and the client does not and cannot properly handle that.

We should treat such behavior from the server as a critical bug. That said, it's good to make changes like this incremental, so it makes sense to get rid of graph sync in a second pass.

All this makes sense to me. I believe the main feature we would need to get rid of this TreeIter requirement would be for the server to tell the player where they are with the full node path and to decide when to provide that info (so that clients know which graph nodes they must generate).

Thanks for merging!