Set up an initial framework to allow megastructures to spawn in the world #454
Conversation
patowen
force-pushed
the
initial-megastructure-framework
branch
from
feb3991 to
3624275
Compare
|
Please rebase for clarity. |
patowen
force-pushed
the
initial-megastructure-framework
branch
from
3624275 to
345639b
Compare
Done! Thanks for reviewing the other PRs so quickly! |
|
|
||
| // This implementation is rather complicated and can be difficult to follow. It is recommended to see the | ||
| // `alternative_implementation` test for an equivalent algorithm. Most of the logic here is just maintaining | ||
| // state to allow `PeerTraverser` to work much like an iterator instead of storing everything into an array or vec. |
There was a problem hiding this comment.
Question for @Ralith: I would like to point out this comment. Based on the structure of the code and where the bottlenecks might be, it seems pretty viable to have a function that returns a Vec<PeerNode> instead of using this iterator-like approach. Although I went through the effort of writing this, and I believe I was careful to make it work correctly, I think this optimization may have ended up being premature, as I don't think the creation of a temporary Vec for each node being added to the graph would be a bottleneck, especially since it only needs to be called when a node (which as 20 chunks) is added to the graph, not every frame.
Basically, I'm considering restructuring this entire implementation to look a bit more like the alternative_implementation unit test, but I would like to check with you before updating this PR.
| pub struct NodeState { | ||
| kind: NodeStateKind, | ||
| surface: Plane, | ||
| road_state: NodeStateRoad, | ||
| enviro: EnviroFactors, | ||
| horosphere: Option<HorosphereNode>, |
There was a problem hiding this comment.
I haven't exactly decided how we would want to support multiple different kinds of megastructures. We essentially need NodeState to be aware of every megastructure there is, but it seems silly to have a field for every type of megastructure when only one would be populated (barring future work of supporting multiple megastructures in the same node, like something inside a horosphere).
My current idea is to punt this design question for a future PR when we would actually have multiple megastructure types to choose from, as trying to figure this out now seems like a premature abstraction.
There was a problem hiding this comment.
Punting seems like a good call. I've always figured we'd have something like Option<Box<dyn Megastructure>> in the end, but getting the interface for that right will be difficult before we actually have multiple types. A good follow-up might be to implement a handful of hyperrogue classics to fill things out.
| /// Whether the horosphere will still be relevant after crossing the given side of the current node. | ||
| fn should_propagate(&self, side: Side) -> bool { | ||
| // TODO: Consider adding epsilon to ensure floating point precision | ||
| // doesn't cause `average_with` to fail |
There was a problem hiding this comment.
The scenario I'm afraid of here is that two megastructures are evaluated to not interfere, but when they're actually generated, due to slight floating point precision differences, they end up interfering. With the current implementation of average_with, that would cause a panic (a panic which allowed me to catch my earlier miscalculation of what could count as a peer node, where I thought all peer nodes were edge-adjacent to this node). Worse, it would be a rare panic that would be hard to reproduce.
It might be good to have some epsilon value to add some buffer here so that should_propagate is biased towards returning true when we're checking for possible interference. That would likely be paired with a unit test to see if this epsilon is doing its job.
| for x in 0..chunk_size { | ||
| for y in 0..chunk_size { | ||
| for z in 0..chunk_size { | ||
| let pos = MVector::new( |
There was a problem hiding this comment.
This calculation could probably be a function in ChunkLayout. I hope to try to do that in a PR that also tries to rename some terms to be more intuitive and switches the matrix transformations x_to_y to y_from_x.
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod test { |
There was a problem hiding this comment.
I probably need more tests here. I'm having trouble thinking of good test ideas. While the code has a fair amount of logic, a lot of it feels like specifications, where the tests would either repeat the code verbatim without testing anything useful, or the tests would give concrete examples that don't have much clarity and don't add much confidence (apart from possibly testing for regressions). I definitely welcome your suggestions here. If/when I have time to take another look at this, I'll also brainstorm test ideas.
There was a problem hiding this comment.
Drive-by thought before I dive too deep: property-based testing?
There was a problem hiding this comment.
Possibly? I don't think this is necessarily complicated enough that we would need automated test cases. (I'm not that familiar with property-based testing, but initial googling suggests it's writing formal specifications and having an automated framework write tests around these specifications).
Thinking about this further, it might make sense to write tests similar to check_chunk_incident_max_elevations, where we set up NodeState for various nodes, manipulate properties we care about, and check that they propagate to a new NodeState as expected. That way, I would be able to deliberately and clearly write a test that, for instance, fails if I use the naive peer node implementation.
common/src/graph.rs
Outdated
| @@ -16,6 +16,7 @@ use crate::{ | |||
| pub struct Graph { | |||
| nodes: FxHashMap<NodeId, NodeContainer>, | |||
| layout: ChunkLayout, | |||
| horospheres_enabled: bool, | |||
There was a problem hiding this comment.
I realized while setting up this option that we don't actually have a framework for allowing worldgen to be configured in any way. The world is forced to be the same world every time. Since enabling horospheres is a worldgen option, I just had to put it somewhere that works. However, putting it in Graph feels wrong. Perhaps fixing this can be punted to another PR, although if we do that, I should probably write a TODO comment here.
Of course, this is meant to be temporary anyway, as scattering random horospheres about is really just meant for debugging megastructure code.
There was a problem hiding this comment.
Making this configurable at all seems premature, yeah. Might be nice once there's more to tinker with.
There was a problem hiding this comment.
The main reason I'm making it configurable now is so that people playing on the master branch don't have to deal with horospheres being randomly placed everywhere.
EDIT: Although, an alternative option is to just use a constant so that people can turn it on or off by editing one part of the code, since I don't expect debug horospheres to be part of any compiled release.
There was a problem hiding this comment.
I went ahead and made this a constant defined in the horosphere module to simplify things.
To provide something to test with, an option has been added to scatter a random assortment of horospheres throughout the world.
patowen
force-pushed
the
initial-megastructure-framework
branch
from
345639b to
1a7c2bc
Compare
This PR adds all the fundamental additions required to support the random spawning of large structures. To demonstrate this framework, a temporary option is available to spawn a random assortment of horospheres throughout the world.
While I tried to document the code well to explain the reasoning behind the algorithm, a summary would be worthwhile, so I'll repeat the summary I gave in Discord a while back (with a few changes):
One nice thing about the dodecahedral tiling, which isn't true of every tiling but works in our case, is that every time you get to a neighbor of a node, you are crossing a plane that divides the space into two halves. You can never cross that plane again without crossing a descender. Here is a 2D representation:
A horosphere in any given position can only be generated by one unique node. The criteria for a node to be able to generate a horosphere are as follows:
Following these two rules and carefully choosing a probability distribution within a given node allows you to generate a random distribution of horospheres in a completely isotropic way.
Implementation-wise, each node containing part of a horosphere needs to have a reference to said horosphere. To avoid unbounded memory usage, we can "forget" about a horosphere the moment we cross a plane that leaves that horosphere behind.
The problem is that some of these horospheres may intersect with each other. To prevent this, one naive approach would be to just skip generating a horosphere if it would overlap with one of the horospheres it (or a parent) already generated. The issue is that it doesn't capture all possible overlaps because two nodes that aren't ancestors of each other can generate intersecting horospheres. The picture below shows a 2D example, where dots with arrows show which node owns which horocycle.
The problem arises because although these two nodes initially appear unrelated, they are related after all because it is possible to reach the same node from both of them. Nodes of the same depth in the graph with this property need to be somewhat aware of each other to avoid causing conflicts when they generate any kind of megastructure. In this PR, I call them "peer nodes", and a fair amount of logic is needed to determine which nodes are peer nodes. See the PR's diff for details.
To get the full list of potential horospheres a node needs to know about, there are three steps:
For more detail on step 3, a candidate can only become a horosphere if it doesn't intersect with any other inherited horospheres or higher-priority candidate horospheres known by the node we're focused on or any of its peers. Candidate horosphere priority can be chosen pretty much arbitrarily. This PR just uses their node-local w-coordinate.
The interaction of steps 1, 2, and 3 means that the order in which various properties are initialized in various nodes interact in intricate ways, so it is no longer feasible to populate
NodeStatefor each node in the order it's added to the graph. Instead, we split it intoPartialNodeState(for nodes that completed step 2), andNodeState(for nodes that completed step 3).One consequence of the importance of peer nodes is that it effectively widens node paths in the graph. In practice, this should be fine, as we already tend to want to form a large sphere of nodes around each player, but it may increase the cost of some things. I don't expect it to be an important bottleneck, though.
For a visual of this, see the below image. For the circled node to have a fully set up
NodeState, all red nodes need aNodeState, and all blue nodes need aPartialNodeState.Fortunately, this approach should be reusable for any kind of megastructure, as the only property this relies on is the fact that it can be bounded by a plane.
One concern I have had in the past is that this intersection test is somewhat asymmetric, so isotropy may no longer be preserved. I don't think this is a major concern, as things look correct. Perhaps there is a way that someone could use this to find the origin if they're lost using some tricky analysis on the distribution of megastructures, but I don't think that's possible, and if it is, it probably isn't something we need to address.
For simplicity, this PR only allows a node to reference at most one horosphere, which effectively means that all horospheres are separated so that the convex hulls of the nodes containing them will never intersect.