search

TheDuckCow / godot-road-generator

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

Enforce bi-directional RoadPoint connections on path updates #108

Closed TheDuckCow closed 1 year ago

TheDuckCow commented 1 year ago

Right now, if someone tries to manually update the reference paths of one RoadPoint to another (while auto refresh is on), it won't work at all, because it fails validations for proper connections (as only one road point will connect to the other, and not the reverse as it hasn't been updated yet).

What we should do is, in the on prior/next point init function, do the call to see if we can properly auto-connect to the target roadpoint in question if it has an equally missing next "slot". And thereafter, ensure on making roadpoint updates that the changes of prior on one point are reflected as the next on the other, and vice versa, to make them properly bidirectional lists.

Demo video of the net result of this change:

https://github.com/TheDuckCow/godot-road-generator/assets/2958461/9bb9e25d-467b-47cf-b89e-184f3439e373

TheDuckCow commented 1 year ago

Update pushed now, with all tests passing:

12 passed 0 failed. Tests finished in 1.4s

This includes unit test updates that took awhile to get right, but are working now and nicely ensure that signals are updated as few times as possible during moments of updates. Signals for updates can now be reliably depended on.

TheDuckCow commented 1 year ago

I've added another commit, which WheelSteal now depends on: having groups automatically added for generated lanes. I'm noticing there are still some excessive road segment updates being processed, particularly when switching to a scene. We really should ideally see about caching and saving generated mesh data to disk so it's not required that it always fully reloads even on game start.

TheDuckCow commented 1 year ago

Added the file now @bdog2112 good catch, let me know if you're good to approve now (knowing I'll add the docstring update as recommended, and possibly a variable name change)

bdog2112 commented 1 year ago

Yeah, that's a hard one to condense into a single variable name. An updated description should be enough to help the reader understand what's going on.

On 7/2/2023 12:31 PM, Patrick W. Crawford wrote:

@.**** commented on this pull request.

The meaning is in essence to say if we're doing the prior or next point's validation fix. So it's a flag which makes me think bool, rather than trying to artificially use one of our existing ENUMs which I think would just make it more confusing.

Message ID: @.***>