TheDuckCow / godot-road-generator

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

Updated RP panel with button placeholder positions #35

Closed TheDuckCow closed 1 year ago

TheDuckCow commented 1 year ago

Basic theme in place as well, nothing connected up.

Visual demo:

Screen Shot 2023-01-15 at 8 36 37 PM
TheDuckCow commented 1 year ago

Adding you as a reviewer @bdog2112 more since it's relevant for our discussion tomorrow. This is mean to be a placeholder UI added on top of your existing branch (which hopefully we can just merge tomorrow).

bdog2112 commented 1 year ago

This looks good and I approve the merge. Aesthetically it looks great!

You previously requested having "Forward" and "Reverse" labels above the buttons. Those might be helpful to users as things could be, potentially, confusing when RoadPoints are rotated sideways or upside down. But, the labels are not "critical". It's totally up to you.

There are also a number of considerations that would go into how adding/removing RoadPoints should work. The implication being that more settings (checkboxes, etc...) may be necessary in order for users to achieve their desired outcomes.

Questions to consider:

TheDuckCow commented 1 year ago

You may find some of the answers to that in this issue task in fact, I actually made this PR as I was attempting to describe that issue and needed the visual, so I just ended up making the visual change. Indeed there's more details I'm probably missing, so we can discuss that tomorrow.

TheDuckCow commented 1 year ago

Realized I linked the wrong one above, I meant: https://github.com/TheDuckCow/godot-road-generator/issues/36

bdog2112 commented 1 year ago

Hey @TheDuckCow, are all of your changes checked in? Would you like me to merge these Road Point panel changes to main?

There is one minor issue in that the linkage to the placeholder button still exists in road_point_panel.gd. But, that can be quickly resolved. Please let me know what you want to do. Thanks.

TheDuckCow commented 1 year ago

Merged in now! Yup, in your task for adding / removing lanes, that'll be good to fix that connection, happy to have you do that. Thanks