118 auto placement and snapping

[bdog2112]

Say hello to RoadContainer snapping! Please review the requirements and the new capabilities and let me know if anything is missing or needed.

MAIN HILIGHTS:

• Select RoadPoint and click "Roads>RoadPoint" to add a new RoadPoint and connect it to selected. (The remaining requirements were unclear. Please let me know if you want to add more to the "Roads>RoadPoint" feature.)
• Select a RoadContainer or Nested Scene RoadContainer and LMB-drag it near some unconnected Edge RoadPoints. A snapping guide will appear between an Edge RoadPoint in the dragged container and the closest external Edge RoadPoint. Upon LMB-release, the container will snap to the target RoadPoint. Adjust plugin.ROADPOINT_SNAP_THRESHOLD to increase/decrease the snapping distance.

ADDITIONAL NOTES: Implementation uses "_connect_rp_on_click" to create Inter-RoadContainer linkage when the selected container is snapped. But, that comes with some pros and cons.

On the pros side, if the snapped RoadContainer is moved, the user can click Update Roads and maintain the linkage to the snapped RoadPoint.

On the cons side, moving the snapped RoadContainer sometimes seems to leave open edge RoadPoints that don't remain linked. But, they can no longer be used in connections.

Sometimes, there is also a drawing issue that requires a delicate dance of Refreshing Roads and moving the RoadContainer around until everything redraws as expected.

There is also a separate intermittent drawing issue for the snapping guides. Sometimes, if the user drags really fast and then stops, the snapping guide may appear in what might be called the previous frame's position rather than the current frame's position. Additional dragging causes the guide to update as expected.

When snapping RoadContainers, there is a long pause upon LMB-release. Checked the timing of different phases of the commit proces and it only takes 6ms to perform the necessary transforms. So, it's unclear what is causing the actual pause. Let me know if you want me to dig deeper.

[bdog2112]

If you wish to simulate an Intersection and test:

• Create a new "nested" scene with a RoadContainer as the root node
• Create Edge RoadPoints for external connections (here's how)
  • Add a 2x2 Road
  • This adds two RoadPoints
  • A RoadPoint must be connected to another RoadPoint to be considered a valid "Edge"
  • In the future, the RoadIntersection object should take the place of the 2nd RoadPoint
  • Select one of the RoadPoints, and set its "terminated" property to "true"
  • Only un-terminated Edge RoadPoints will be considered when snapping
  • Place as many of these 2x2 Roads as needed for external connections
• Save and close the scene
• Create a new "main" scene for testing
  • Add a RoadManager
  • Add a RoadContainer to the RoadManager
  • Add a 2x2 Road to the RoadContainer
  • Add the nested scene to the RoadManager
  • Save the main scene's default state so it can be restored quickly
• Test snapping functionality
  • Drag nested scene towards RoadContainer in main scene
  • Snapping previews are displayed when RoadPoints are in close proximity
  • Release LMB when snapping preview hilights desired RoadPoints
  • Nested scene snaps into position
  • For kicks, try snapping two RoadContainers together or two Nested Scenes.
  • Also, try moving containers after linkage has been created and observe what works and what doesn't work.
  • Try using "Roads>Refresh roads" to update the scene afterwards

[TheDuckCow]

Great to have this hands on! I'm testing it out now, and already having a great time snapping road pieces together. Pretty solid, just a few comments so far from me:

A: Connection tool for snapping not always displayed

Maybe you are referring to this above, sometimes the snapping connection preview line doesn't display, even when the snapping itself still works in the end. Haven't debugged deeply, but I think my pressing undo in the video below is the culprit, though maybe there are other scenarios. The snapping indicator seems to forever not draw however, even after manually changing the export vars of the container to simulate clearing. Maybe we need to properly clear the properties set on undo/redo for the snapping?

https://github.com/TheDuckCow/godot-road-generator/assets/2958461/8df0ecb7-e8aa-4de0-90ae-acd524b45ddb

B: Adding "next" roadpoint via selection

Select RoadPoint and click "Roads>RoadPoint" to add a new RoadPoint and connect it to selected. (The remaining requirements were unclear. Please let me know if you want to add more to the "Roads>RoadPoint" feature.)

As for the next RP on add, yes I think the change is useful, working nicely! My next question would be...

C: Snapping tool for RPs moved to overlap containers

How much of a challenge would it be to extend the snapping tool we have for RoadPoints (snapping to container edges) too? See an example here, where I'm looking to connect this existing RP to another RP on another (non-saved scene) container. So it's kind of like a shortcut to connect a dynamic (open scene) container's RP to another container, without having to use the connection tool (which would also create another RP in the process).

Demo of what I'm hoping could be able to do the appropriate snapping (and auto adjustment of the lane counts/widths/shoulders to match what we're snapping onto):

https://github.com/TheDuckCow/godot-road-generator/assets/2958461/9ccd2516-e359-4158-8472-994314927e25

D: Eliminating extra RP added when connected via snapping

Finally, snapping a road container to an existing same-scene ("dynamic") container I think should result in modifying the point being snapped to, instead of creating a new one. I think that'll be more intuitive for users in the end. So it means the number of lanes on RP_002 in the video below would be modified (and width, shoulder) to match the edge of the SavedRoad I'm snapping. I'd say the reverse should be true based on my paragraph above, where you move a RoadPoint to overlap with an open edge of another container.

https://github.com/TheDuckCow/godot-road-generator/assets/2958461/62980d92-79ca-4381-b579-d448665570fd

E: The "terminating" feature working

See here my little demo, works nicely! As you say, we'll change this to an intersection point in the future, but works for now. Only comment is that our "connection tool" (not to be confused with our snapping feature here) still picks up these points. But, since we're going to change it, don't worry about changing the connection tool, it's more a note to self in the future.

https://github.com/TheDuckCow/godot-road-generator/assets/2958461/5da0b53f-111e-4368-92b1-d4a0f69e0168

(also posted to Twitter here)

Regarding the delay, I didn't notice it for my smaller 2-roadpoint snapping, but did notice it for my larger scene snapping. My gut tells me it's due to the transform on the container somehow triggering a rebuilding of all the geo for the container (which ideally, it should know it doesn't need to do). So I'm not worried about that in this PR, something for us to address separately.

Summary

Curious to hear what you have to say on each of the above, and then based on your response we can discuss priority (maybe we don't need to do all of this right now).

[bdog2112]

We need to clearly define how RoadPoint relationships work in the following connections: 1 RoadContainer to Roadcontainer 2 Nested Scene to RoadContainer 3 Nested Scene to Nested Scene 4 Direct RoadPoint to RoadPoint within same RoadContainer (self explanatory)

Important questions on this topic: Do we ever connect RoadPoints directly via their pt_inits? When do we add extra support RoadPoints? How do we clear support RoadPoints? How do we clear support data not stored in RoadPoints? Do we auto update lanes when snapping? Which lanes take precedence (selected or target RoadPoint)?

A: Connection tool for snapping not always displayed What properties are set by "_connect_rp_on_click"? How do we clear them? The inner workings didn't fully make sense to me. But, I could see that there were 4 arrays roughly called "rp_locals". They were used for some connections and omitted on others. We could just refrain from using _connect_rp_on_click in certain situations and do more direct and traditional pt_init connections.

B: Adding "next" roadpoint via selection - Excellent!

C: Snapping tool for RPs moved to overlap containers This sounds do-able. In theory, this blends in with the RoadContainer snapping. We simply ask: Is dragged object a RoadContainer... Is dragged object a RoadPoint. Snap this way if it's a RoadContainer. Snap this other way if it's a RoadPoint. We need clear requirements. Hence, it's important to answer all of the questions at the beginning of this list.

D: Eliminating extra RP added when connected via snapping RoadContainer snapping employs the same behavior as the Connection Tool. Does the Connection Tool behave in the desired manner? Maybe we have a mix of scenarios and we need to define different behavior for each one.

E: The "terminating" feature working- Great!

[TheDuckCow]

Important questions on this topic: Do we ever connect RoadPoints directly via their pt_inits? When do we add extra support RoadPoints? How do we clear support RoadPoints? How do we clear support data not stored in RoadPoints? Do we auto update lanes when snapping? Which lanes take precedence (selected or target RoadPoint)?

In terms of the two modes/operations:

• Snapping: Should never create new roadpoints, but instead the operation is the movement of "something" (for now, a container; in the future, could be RoadPoints as well)
• Connection tool: Should never move roadpoitns
  • in the case of connection two RPs, nothing new should be created, it's just a connection
  • Between an RP of the active container, and an RP on another container, when creating a connection: this should always create a new RP, becayse we assume the two source RPs are not in the same location, and by definition of the conneciton too, we aren't moving any nodes, therefore to create a exactly aligned matching conneciton we have to create a new RoadPoint. This new roadpoint becomes the new "edge" roadpoint, and this is the one which is visualized as connected via the export vars on the parent container.

Supporting point (and something we should well document):

• The edges of containers are by definition roadpoints with open connections; therefore a connection across a road container is really managed by the export vars on the road container itself, not by setting the paths of the RoadPoint within the respective containers.

Focus for this week:

1. Let's add the "unsnapping" tool, where when moving a RoadContainer which is connected to another road container, it will automatically disconnect on release, as well as visualize that it would be performing the disconnect using the red line drawing overlay (similar to the disconnect mode we see in the connect tool).
   • It might make sense to still account for the existing radius; if we don't exceed this radius, it should snap back into place (ie no change in the end). So underneath this radius, we can show the blue-gray indicator; outside this radius, it will show as red.
2. Look into what is allow the snapping tool to still snap even when the connection isn't actually open
   • To recreate: drag in a saved scene, snap connect one side. Then drag and connect the other side (without manually "disconnect", so in theory both sides are now somehow connected). Finally, drag one back to overlap the first initial connect RoadPoint, to find that it snaps without the visual
3. Let's see if we can fix some bugs by converting the initialized "" nodepath values of the container export vars (initially set in the cotnainer.update_edges, and anywhere else that we are "clearing" the, clear to a null instead of a "" (assuming it lets us). Failing that, we'll need to sprinkle some extra checks for val is null or val == ""

Let's do this in this branch for now. Out of scope for now:

• Fixing the redo last
• The RoadPoint snapping
• Elimination of the extra roadpoint + live updating the lane counts (let's get things stable first).
• Snapping disconnection with a same-scene container and another saved scene container.

[bdog2112]

Updates have been made to insure that connected Edges won't accept new connections. Unsnapping is also in an introductory state.

Keep in mind that these changes target primarily scene to scene connections versus RoadContainer to RoadContainer connections. Although, those do function. But, they might require some additional work to become fully fleshed out.

Made an executive decision regarding unsnapping scenes with multiple connections. Read on to learn more and feel free to make additional suggestions.

When unsnapping a scene with a single connection, a single blue or red line is shown to indicate whether the action will result in a snap or an unsnap.

When unsnapping a scene with multiple connections, a single blue or red line is shown as before. However, it only references the 1st connection in the list of connections. Other connections DO NOT display a line. However, they DO get unsnapped so that they're available for future connections.

This iteration uses the "_disconnect_rp_on_click" function. The main takeaway is that unsnapping a scene with multiple connections results in multiple undo points; one for each connection plus a translate. So, additional work will be needed if we want a singular undo point. At any rate, unsnapping feels pretty good so far!

[TheDuckCow]

Leaving an initial comment as I found one edge case; similar to RP's, we'll probably have to do a "cleanup routine" which requires doing checks for two container edges being mutually connected (and if not, resetting the one making the "false claim"), replicated here by duplicating a node that had one connection already.

Likely want to resolve this in the similar way as we have it for roadpoints, via their "_autofix_noncyclic_references". Does mean creating and then sprinkling around an equivalent _is_internal_updating var (not export var) on the container class. If we end up not needing, then great! That would actually be the best case scenario. The reason this is a problem for roadpoitns is because we have to do this cyclic check in the routine that runs immediately after any RoadPoint prior/next connection change, and thus could (without this internal flag) interrupted "internal" changes we were trying to do.

https://github.com/TheDuckCow/godot-road-generator/assets/2958461/828a9ea7-37d9-498c-8965-6fa7064ea241

[bdog2112]

This week's primary focus was to Move Connected RoadPoints when the user dragged a RoadContainer. Secondary focus was fixing references. This inquiry is about Moving Connected RoadPoints.

The request conflicts with the "Unsnapping" feature. Hence, we have to make some decisions about whether to treat a Scene Move as an Unsnap or a Move.

One approach is to always Unsnap Scene to Scene connections and always Move Connected RoadPoints. Users would simply need to employ the "Connection Tool" to Disconnect connected RoadPoints. For now, I'm working on that scenario.

In the scenario of moving a Scene connected to a "RoadContainer": The choice is arbitary. We could "Unsnap" or we could "Move Connected". But, we can't do both. Maybe we could Unsnap by default and Move Connected when user pressed "shift".

Assuming we used shift to Move Connected, then we would need to consider how that worked when two "Scenes" were connected. We could Move the Connected Scene along with the Selected Scene. But, that could potentially lead to a cascade of Scenes moving all at once. The safest bet would be to simply Unsnap the Connected Scene even though we're technically performing a "Move Connected" action.

Putting it all together: We could be Moving a Scene connected to both a RoadContainer and another Scene simultaneously. So, the default behavior would be to Unsnap everything. If user pressed shift, then we would Disconnect the two Scenes. But, we would Move the RoadContainer's connected point because shift was pressed.

Again, I am currently setting the behavior so that Scene to Scene connections are always Unsnapped and Scene to RoadContainer connections are always Moved. Please let me know if you want it to work differently.

[bdog2112]

When dragging a "Connected" Container, current behavior is to always Unsnap. Snapping is only available on "Unconnected" Containers. This provides behavior that is clean and concise. Containers are either Snapping or Unsnapping. Never both.

We cannot currently Snap a Container if it has a "Moving" RoadPoint because that makes it "Connected". So, the question is: Do we want users to be able to Snap in this scenario?

[TheDuckCow]

Hey @bdog2112 here's how I would think of it:

The request conflicts with the "Unsnapping" feature. Hence, we have to make some decisions about whether to treat a Scene Move as an Unsnap or a Move.

I'd prioritize like this:

1. Prefer "dynamic" movements where possible
2. Fall back to snapping/unsnapping if that's not possible.

So with that philosophy in mind, a few scenarios and how I'd consider treating them:

• Moving a saved scene away from a saved scene: only unsnapping applies, as we assume we can't modify saved scenes
• Moving a same-scene container (A) which is connected to another same-scene container (B):
  • These are BOTH dynamic elements, and so we have full flexibility.
  • If the user is moving container A, which means the entire container is visually selected, they probably expect all portions of meshes contained inside container A to be moved together. Therefore the result of moving container A is that the connecting edge roadpoint of container B gets moved (while container B as a parent remains unmoved, and the other RoadPoints of container B are left in place)
• Moving a saved scene container A which is connected to the open-scene container B
  • Treated identically to the above scenario actually, container A can't be modified while container B can be, so the connecting roadpoint of container B is moved to keep up with the new location of Container A
• Moving an open-scene container B which is connected to the saved-scene container B
  • We can't modify scene B, so our only option is to move container A as a whole, but then countering/inverting the movement of the ContainerA's connecting RoadPoint to container B so that A.rp_edge visually looks like it's not moving (even though relative to its parent, it is in fact moving)

That's how I see the default behavior. Let me know if that clarifies everything, and likewise if you agree this feels intuitive enough.

That being said, your idea to have some modifier to "sever" the connections is interesting, such that we would always "unsnap" instead of "Move connected". It could eventually even be a UI setting tool to specify which behavior. I'd say, if it's not a huge amount of extra work, you could secondarily work on this such that when the user presses ALT, it would unsnap everything it's connected to. ALT I think is a more similar action for something like this, where shift or control tend to be used for snapping movement to grid intervals or for slowing/speeding up the movement (since this would be happening during the middle of a user doing a transform).

But still, I'd do so secondarily, let's get the base functionality of "prefer dynamic" working first. I could imagine we release first without this tool - May is getting away from us, and I'd like us to get some time on Wheel Steal's next milestone too.

[bdog2112]

That's how I see the default behavior. Let me know if that clarifies everything, and likewise if you agree this feels intuitive enough.

The above makes sense. It covers the simplest scenarios. Although, we still need to address the more complex scenario previously mentioned:

We have a Saved Scene connected to both a Saved Scene and a RoadContainer. Current behavior is to Snap or Unsnap. Not both. In this scenario, when the Saved Scene is dragged, everything Unsnaps.

In the future, we're saying to always move the RoadPoint from the RoadContainer (don't Unsnap it). So, the Saved Scene will always have a connection. Therefore, it can no longer be Snapped. I'm fine with that. Just want to make sure you're ok with it.

We need a consistent way to determine if we're Snapping or Unsnapping.

• Snap if the dragged object has no connections (That's a no brainer.)
• Also Snap if ___ (fill in the blank)
• Unsnap if ___ (fill in the blank)

For now, when dragging a Container (i.e. Saved Scene or RoadContainer), I will assume that we always move the RoadContainer connections and we always Unsnap the Saved Scene connections. We will never Snap any open Edges if there is 1 or more RoadContainer connection. In order to Snap,, all Edges must be open.

[TheDuckCow]

Might be easier to talk over live later, but maybe this detail helps: with scenarios I listed in my prior post, I was assuming we would treat each connection independently. So in indeed you would have "both". That way, a complex case is just the simple case applied per connection. So one connection to a saved scene would unsnapped as a result (if the scene we are moving is also a saved scene), while another results in another RP moved in another container (if that connected container is in the open scene).

We will never Snap any open Edges if there is 1 or more RoadContainer connection. In order to Snap,, all Edges must be open.

This is also a good one to consider. We can also chat through this one, but I think it could work more nicely if we do allow snapping but only if either all edges are open, or if all of the used connections are to open scene containers (resulting in the edge roadpoints of those containers to also be moved as a result of the snapping).

[bdog2112]

Added drawing of Unsnapping lines for ALL containers.

Next up: Move connections to adjacent RoadContainers instead of Unsnapping them. i.e. When user drags Saved Scene 1 with a connection to Saved Scene 2 and a connection to a RoadContainer, Saved Scene 2 will show an Unsnapping line while the RoadContainer's attached RoadPoint will move along with Saved Scene 1. (Tricky. But, useful)

[bdog2112]

WIP diff: wip_diff.zip