Identify, disconnect, and clean up badly connected RoadPoints

[TheDuckCow]

If we duplicate a roadpoint in the middle of a RoadNetwork, we get a lot of weird overlapping geometry. This is because one roadpoint points to another, but that other one doesn't point bad. See the video showing up this awkwardness of having to manually toggle refresh (there are a number of scenarios where this becomes necessary):

https://user-images.githubusercontent.com/2958461/226811922-847c676d-a39c-4dfd-952c-41bfd9afb367.mp4

Ideally, the RoadNetwork can be an authority to determine an "illegally connected" RoadPoint, and auto clear up any bad pointer references - and just as critically, clean up any of that invalid geometry left around so that we don't have to manually refresh the network auto update, which is a bit of a pain.

[TheDuckCow]

Some aspect of this was actually resolved in the recent release, though more could be done.

[bdog2112]

Questions:

• What is the issue? Is it the fact that the user was allowed to create an invalid setup?
• When do we do validation? a) When RoadPoint selected, b) When start/end point changed, c) When a new (to be created) "Validate RoadPoint" button is pressed, d) Other: (explain here)
• Do we arbitrarily correct a user's changes based on our assumptions or simply tell them what's wrong and let them make the corrections? (i.e. Which RoadPoint do we alter when making corrections? How do we know which is the right one? Does only the user know?)

In the given example, the user should have used the "+Next/Prev RoadPoint" buttons to add the RoadPoint rather than duplicating. In fact, I would go so far as to say users should never duplicate RoadPoints because it inherently creates an invalid junction.

Nevertheless, we could enforce certain rules. For instance, the given example showed two RoadPoints using the same third RoadPoint as their end point. We could say this is not allowed and that a RoadPoint can only be used as the end point for one RoadPoint. But, how do we enforce this and when do we check for the condition?

To enforce this, we could validate whenever the user changes the end point. We would iterate the entire network of RoadPoints and make sure that RoadPoint was only used once. If it was already used, then we would revert the user's change and set the end point to null.

In the case where the user duplicates a RoadPoint, there's no way of knowing when this has happened. We could always run validation when a RoadPoint is selected.

Alternatively, we could implement a tool button that says "Validate RoadPoint" and/or "Validate RoadNetwork". It follows that the validation routine would verify that a given RoadPoint is used no more than once as a start or end point.

If duplication was detected, then we would need to provide options for remediation. These might include aribitrarily keeping the first start/end point assignment and clearing all of the others or, perhaps, just printing a list in the console and letting the user do the cleanup.

Another approach would be to display a modal popup where the user selects the correct RoadPoint from a list of connected RoadPoints. Then, we would automatically disconnect all other RoadPoints from the start/end point. There would be a bit of a learning curve as we figure out how to do this. But, it would probably be the most convenient and intuitive solution for users.

[TheDuckCow]

Thanks for the detailed questioning and thinking through. I think the core original topic in question was brought up indeed around duplication of nodes, but then I generalized the title to be about really any situation with a bad situation. You outline the two main scenarios:

1) Duplication of a RoadPoint 2) Otherwise creating bad connections (where as you say, a roadpoint gets used more than once)

What is the issue? Is it the fact that the user was allowed to create an invalid setup?

Yes, we want to prevent invalid setups where two road segments are coming out of each other. It creates weird geometry that will never be desirable. We should have cleanup code the prevents this from persisting, so that other features like intersections can depend on the validity of the network (where roadpoints only connect to max 2 other roadpoints).

When do we do validation? a) When RoadPoint selected, b) When start/end point changed, c) When a new (to be created) "Validate RoadPoint" button is pressed, d) Other: (explain here)

Think more downstream: at the time mesh geometry updates are happening, when we're about to potentially create new RoadSegmetns. This should really be a pre-step which corrects any bad connections. This way, this code gets triggered whenever a transform / updated connection is done (whether on a single node, or if the whole network is updated), no different to how it's already running. So it shouldn't be a new lifecycle hook.

Do we arbitrarily correct a user's changes based on our assumptions or simply tell them what's wrong and let them make the corrections? (i.e. Which RoadPoint do we alter when making corrections? How do we know which is the right one? Does only the user know?)

The modal popup is an interesting idea. While indeed it gives the most control, most users would not expect a popup right after duplicating a node. Plus, whenever an invalid situation occurs, there will be a single action or moment that makes it invalid. So, it should be self-resolved at that point.

To give direction: How do we do this, "get the last action"?

Thinking about this more, we shouldn't even need to. We have bidirectional linked lists essentially right now. If we have an invalid setup, we could check whether the bidirectional point connections agree or not - and for the one which disagrees, disconnect what it is falsely claims being connected to.

Maybe I'm oversimplifying, but it seems to me like this could be a solid approach. Thinking it through:

Scenario A: The duplicate scenario

• The two roadpoints (original RPA0 and the duplicate RPA1) have exactly the same prior and next point init nodepaths as a result of a duplication. If this was at the end of a Road, then they might both list their EndPointInit as RPB
• When performing the checker, we should see that RPA0 claims RPB as it's end point; if we check RPB, we see that RPA0 is listed in at least one of the PriorPointinit or NextPointinit, so no issue - loop continues.
• When the checker gets to RPB, same deal: it claims to connect to RPA0; and RPA0 agrees it is connected to RPB, so we continue
• Once we get to RPA1 however, it claims to be connected to RPB. If we check RPB, we find neither the prior or next pt init claim to be connected to RPA1. Thus, we clear RPA1's connection to RPB. We would end up doing the same to the other connection slot of RPA1, if anything were filled out. Though this would be computed independently.

Scenario B: A manually created invalid setup by changing pt inits only.

• Say you have A-B-C all connected in a row, so B is fully hooked up
• From the Add Node menu, you create roadpoint D. So far we're good.
• With D selected, you set its NextPoint init to be B. This should trigger the rechecker followed by the mesh generation
• During this rechecking, you would see that roadpoint B does not think it is connected to D, so D is taken as the "liar" and this just-added

Some additional thoughts on this approach:

• To me, both scenarios above can follow the same checker logic and works out as I'd argue you would expect.
• You might think: why not enforce this at the time of setting the point init path? Well, I like to think of this auto resolving to be part of the generation step that gets deferred if auto-update is turned off, so someone programmatically changing the connection between a bunch of things doesn't start fighting our auto corrector.
• Yes, as road networks get longer and more complex, the number of checks increases - but it should be linear time O(n), and we can iterate in order of children (order doesn't matter, so long as each RP is looked at once). And we already do this while (re)generating segments.
• However if we are moving or transforming a node, we shouldn't need to check the whole network, we could just check the 2 roadpoints connecting to the middle roadpoint being transformed (similar to how we don't regenerate all road segments, just the 2 connected to the point being transformed).

Let me know if you think I'm missing any considerations @bdog2112. Of course once we implement this, any scenes saved to disk may be updated immediately on next regeneration, but I'd argue that's working as intended as it sanitizes the network (maybe that's even what we call this function.