TheDuckCow / godot-road-generator

A godot plugin for creating 3D highways and streets.
MIT License
328 stars 17 forks source link

Implement lane transition geometry #24

Closed TheDuckCow closed 1 year ago

TheDuckCow commented 1 year ago

Creating the PR on behalf of @bdog2112 although it is still a WIP. I create draft PRs as it's easy to look at the net diff a branch

bdog2112 commented 1 year ago

Identified a bug in the lane matching algorithm during testing. This arose in the update to facilitate one-way road configurations. Originally, we said every lane setup must include both FORWARD and REVERSE lanes. But, that is no longer true. In some edge cases, an all REVERSE lane setup could be interpreted as all FORWARD.

Fix is forthcoming...

bdog2112 commented 1 year ago

The above fix should address the previously reported REVERSE lanes issue. Still need to finish up the test cases for this PR. Plan is to pare down the 80+ list of general "transition lanes" tests to 10ish and also update the "one-way lanes" test group.

TheDuckCow commented 1 year ago

Seeing the shoulder not being generated, but could be due to some array not being resized internally properly??

 res://addons/road-generator/road_segment.gd:373 - Invalid get index '1' (on base: 'Array').
Screen Shot 2023-01-07 at 12 25 10 PM

demo.tscn.zip

If I update this shoulder related code, it removes the error, but still then gets just a single should drawn. Makes sense we'd have to handle a single lane shoulders on both sides in a special if case:

--- a/addons/road-generator/road_segment.gd
+++ b/addons/road-generator/road_segment.gd
@@ -370,6 +370,8 @@ func _insert_geo_loop(

        for i in range(2):
                var dir = -1 if i==0 else 1
+               if len(lane_uvs_length) == 1:
+                       dir = -1
                var uv_y_start = lane_uvs_length[dir]
                var uv_y_end = lane_uvs_length[dir] + per_loop_uv_size
Screen Shot 2023-01-07 at 12 31 45 PM
TheDuckCow commented 1 year ago

Just pushed the push, nice in person peer debugging :)

bdog2112 commented 1 year ago

You recently suggested merging this PR into main. Hence, that's what I'm going to do. I gave it one final round of testing. All 4 GUT test sequences passed. I also performed 4 visual lane configuration tests:

  1. R > R > R
  2. RF > RFFF > RFFF
  3. RFF > RRFF > RRFF
  4. RFFF > RFFF > RF

(See screenshots, below.)

I didn't save those configurations. But, perhaps, in the future we could create a limited set of test scenes suitable for visual testing and save them into the test folder.

Noticed that the demo scene generated some errors on startup. However, those were to be expected due to an invalid lane configuration on Road Point "disc_3". (4 Lane Types and 5 Traffic Directions. Is that intentional?)

The short-term fix would be to re-configure the Road Point. However, the ultimate long-term fix would be to make it so that the user can't increment the number of Lane Types and Traffic Directions separately. There should be a single button to "Add" lanes and a single button to "Remove" them. We should probably also hide the separate arrays from the export variables.

Visual test screenshots:

R > R > R 1 to 1 lane

RF > RFFF > RFFF 2 to 4 lanes

RFF > RRFF > RRFF 3 to 4 lanes

RFFF > RFFF > RF 4 to 2 lanes