Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Establish connections between RoadContainers #133

Merged
merged 13 commits into from
Nov 21, 2023
Merged

Conversation

TheDuckCow
Copy link
Owner

@TheDuckCow TheDuckCow commented Nov 3, 2023

Until now, users have not been able to make connections between two different RoadContainers with the interactive tool (and doing so manually with paths would create unexpected artifacts). In this branch, we first start to allow for connections. This is a stepping stone to fully support generalize custom prefab scene creation while retaining traffic AI guides (RoadLanes).

Edited at time of being review-ready:

  • When connecting RoadContainers, we detect if the two highlighted RoadPoints (on each respective RoadContainer) are already overlapping in 3D space
  • If yes, directly connect them through export var chaining.
  • If no, create a new roadpoint at the right position, then connect that new roadpoint to the new container.
    • This new roadpoint is added as a child of the container of the selected node;
    • Unless that node is a saved subscene, in which case it'll switch to making it a child of the other container involved
    • If both containers are saved subscenes, then: an error is pushed instead. In the future, we'll have snapping tools to connect these two saved scenes together, or the potential to create a new "bridging" RoadContainer between two segments
  • Undo/redo should be fully functioning.

Not yet implemented:

  • Correctly setting the prior/next point on the RoadLane's across containers. Right now, any vehicle following a RoadLane to the end of its RoadContainer will see "nothing is next". Will improve in the future.

Correctly identifies now when selecting a Container and finding the
closest point on that container to treat as the source connection, and
draw the line to another container restricted to only open edges.

Works inside container scene or between nested scenes.
@TheDuckCow TheDuckCow linked an issue Nov 3, 2023 that may be closed by this pull request
Base automatically changed from fix-connection-direction to dev November 3, 2023 01:00
@TheDuckCow
Copy link
Owner Author

While working on this, we need to address a few scenarios. Subscene below refers to a scene file saved to disk, added as an instance in the open scene where the editing is being done. Once we have all below scenarios implemented (along with some testing), this will conclude the v0.4.0 release:

  1. Connecting open scene to subscene:
    -> Create new roadpoint at same pos as the target edge roadpoint
    -> Perform the inter-container connection
  2. Connecting sub scene to sub scene:
    -> Create new container
    -> Add roadpoints at both edges in question
    -> pick a material at random?
  3. Connecting open scene/ContainerA to open scene/ContainerB
    -> Add new point as part of ContainerA
    -> Perform the inter-container connection with B and new point
  4. Add button when a subscene is selected
    -> Create new container
    -> Add 2 roadpoitns, once at point of click and one at connected edge
    -> Perform the inter-container connection
    -> glhf with undo/redo

Further future:

  • Pick a saved scene to add as the next segment, as opposed to placing a single RoadPoint

Also introduces a reusable utility for checking if a RoadContainer is
in a saved scene or is a scene-root.
This ensures consistency with the concept of saved scenes to prevent
conflicts. This also restores the ability to add points with only the
RoadManager selected, to easily start drawing new roads (with a new
RoadContainer added on the first click)
Added some tests as well, and some placeholder tests to get to. All
tests passing except the two marked as pending.
Also renames the getter/setter functions for the next/prior_pt_init. Also
fixes a bug of which index was being selected in the connect_container
code.
@TheDuckCow
Copy link
Owner Author

Some good progress so far, with new coverage tests included. We don't yet have auto creation of new RoadPoints on placement yet, but that's the next step, and should largely be able to leverage the current existing rp placement do/undo functions in sequence with the connect/disconnect functions.

What remains is a way to visualize the undo step, ie the "disconnect" tool between different RoadContainers, as right now there's no clear way to do that. Expecting that:

  • If the RoadPoints overlap (as they should), just display an X or maybe even an unlink chain icon
  • If they roadpoints don't overlap (ie the user manually moved them separately and our code did not keep up), could be a similar red connection line as the other disconnection tool.

@TheDuckCow
Copy link
Owner Author

Need to clean up the commit still, but this is great progress here, basically wrapping up the outstanding items above:

demo.cross.scene.connections.mp4

@TheDuckCow
Copy link
Owner Author

Oh and of course, right after posting the video do I first notice the issue of flipped lane order depending on the order of connections/directions the edge RoadPoints are facing. Hm will need to contemplate that one further how best to solve.

@TheDuckCow
Copy link
Owner Author

Just uncovered this issue, but after some investigating, this is not something introduced by the changes here. So will track it as a separate bug.

@TheDuckCow TheDuckCow marked this pull request as ready for review November 20, 2023 07:19
@TheDuckCow
Copy link
Owner Author

With this final push, I'm calling this good for now - there might still be some edge cases, and there are some user interactions which aren't really refined right now (e.g. disconnecting containers where they already have overlap), but it's a good enough starting point.

*** Run Summary ***
All tests passed

Totals
Scripts:          4
Passing tests     22
Failing tests     0
Risky tests       0
�Pending:          0
Asserts:          153 of 153 passed

22 passed 0 failed.  Tests finished in 1.7s

@TheDuckCow TheDuckCow merged commit e485801 into dev Nov 21, 2023
@TheDuckCow TheDuckCow deleted the container-connections branch November 21, 2023 04:22
TheDuckCow added a commit that referenced this pull request Nov 23, 2023
Establish connections between RoadContainers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create tool to connect RoadPoints and RoadContainers
1 participant