-
-
Notifications
You must be signed in to change notification settings - Fork 50
add road branching nodes #224
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
base: main
Are you sure you want to change the base?
Conversation
TheDuckCow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just leaving some quick thoughts before I'm out for most of the day - thanks a ton for diving head deep into this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice icon 😎
| # Workaround for cyclic typing | ||
| func is_road_branch() -> bool: | ||
| return true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw this might be no longer needed, a relic of the 3.x era! (but, to be seen later I suppose)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if I got this right, this is a parent of one or more RoadContainers - is it a middle man between the RoadManager and the RoadContainers? I wonder if the functionality should just exist on one or the other in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RoadPath could most easily be combined with RoadContainer, but I wasn't sure that is the best course. I'm still unclear on exactly what bounds we want (or don't want) to enforce on RoadContainers, which is why I didn't wrap these items into the Container specifically.
This is the current hierarchy I have - RoadPath just sits over RoadContainer here in all cases in a 1:1. I just don't want to box in RoadContainer if there are use cases where the bounds shouldn't be drawn so rigidly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checking - this is more like a "feature flag" to use one or the other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This just exists to satisfy the issue I mentioned above where Paths and Containers could be merged into a singular object, but I want to allow getting containers optionally through a RoadPath.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious where this ends up living in the scene hierarchy, are these children of RoadPaths (siblings of RoadContainers)?
Another thought I had while reviewing this briefly: Maybe RoadBranches could actually be a node which RoadLanes connect to explicitly (ie their "next_lane" reference would be the branch node rather than an actual lane), and visualized in the editor as a point to snap one or more RoadLane control points to? But that would only make sense if this is meant to live on the individual lane level; we'd still need a RoadContainer or other class to inspect the contents of itself to figure out how all the pairings map together, which I think does make sense to "compile" at the roadcontainer level, basically describing how someone can traverse through it (based on given starting lanes). So maybe you're focussed more on that part, which is fine, I'm just thinking ahead to what the user experience will be to construct it while being clear and intuitive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't put a ton of thought in how to interact correctly within the editor, my knowledge in that space is pretty shaky. I think the RoadPath <--> RoadBranch mapping should more or less work as-is, since the addition of a branch to a Path object is reflexive via the property setter. I am generally approaching things in a code-first / editor-later manner so I can prove out concepts.
There is value in making the "next lane" (or roadpoint, in lane agnostic terms) separate from the upcoming branch. Agents will need to take path termination/branch entry into account ahead of the actual transition. If I'm in the far left lane and I need to turn right to traverse the branch, I need my agent to know ahead of time that the next branch transition requires lane changes before I actually get there.
I briefly attempted to make RoadBranchPathMapping / RoadBranchLanePairs a data structure that simply existed on the branch node, but couldn't really get around the restrictions of needing to set the custom classes as part of an export variable. I thought about switching to a Resource, but direct node references here are way too valuable to drop.

Just a draft PR to show direction I'm going with road branching/paths/lane connections. No intent to merge.