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

Mass-edit road selection panel #631

Merged

Conversation

kianzarrin
Copy link
Collaborator

@kianzarrin kianzarrin commented Jan 31, 2020

fixes #542

I was unable to fully fix mass edit overlay so I disabled it and created issue #789
I fixed all other problems.
This is ready for review.

Added Identical road selection UI to roadAdjust panel and roadWorldInfoPanel. See #542 (comment) for pictures.

expected behavior

  • Both panels trigger tutorial message (same).
  • activation of the panel in roadAdjust activates mass edit overlay so that user can see relevant traffic rules. same is true when user clicks a button. note that roadAdjust is in turn automatically activated when user tries to modify road selection.
  • clicking a button will apply traffic rules and puts it in active state. clicking it again deactivates it and clears relevant traffic rules. Clear does not have undo mode.

Scope:

Since there is so much new user interface here there is a lot of potential for new feature [EDIT: see #691]. Therefore I have to severely limit the scope of this review by limiting some of the functionality. The objective is not to implement all missing features in TMPE! Its to provide the base UI interface and lay out the foundation for the rest of the work.
EDIT: speaking of scope priority roads that turn on junctions are not supported. Roundabout paths on the other hand can go through different routes.
EDIT: paths that turn left/right on junctions are supported.

further changes

Translation keys:

These keys have been added in another PR.:

Menu.csv
"RoadSelection.Tooltip:Clear"=>"Clear traffic rules related to high priority road" // for now!
"RoadSelection.Tooltip:Stop entry"=>"applies stop signs on connected roads"
"RoadSelection.Tooltip:Yield entry"=>"applies Yield signs on connected roads"
"RoadSelection.Tooltip:High priority"=>"Applies traffic rules for high priority road(customizable in options)"
"RoadSelection.Tooltip:Roundabout"=>"applies all traffic rules for roundabout. button is disabled when selection is not a roundabout."

tutorial.csv
"TMPE_TUTORIAL_BODY_RoadSelection"=>"   Clicking each of the buttons(except the clear button) applies traffic rules. clicking the same button again clears those traffic rules. Each button is enabled only when there is a valid selection of road/roundabout.\n\n   To select a road, click on the green road selection button."
"TMPE_TUTORIAL_HEAD_RoadSelection"=>"TM:PE Mass-edit Road Selection Panel"

@originalfoo originalfoo added this to the 11.1 milestone Feb 2, 2020
@kianzarrin
Copy link
Collaborator Author

I fixed a couple of problems please see commit message.

Copy link
Member

@originalfoo originalfoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

TLM/TLM/UI/TrafficManagerTool.cs Outdated Show resolved Hide resolved
TLM/TLM/UI/TrafficManagerTool.cs Outdated Show resolved Hide resolved
TLM/TLM/Util/RoundaboutMassEdit.cs Outdated Show resolved Hide resolved
TLM/TLM/Util/RoundaboutMassEdit.cs Outdated Show resolved Hide resolved
TLM/TLM/Util/RoundaboutMassEdit.cs Outdated Show resolved Hide resolved
@kianzarrin
Copy link
Collaborator Author

@krzychu124 Edit:
While menu is visible I'm noticing significant fps drop ~15-20ms/frame in my case, so from 30fps it drops to ~20fps
only TM:PE subscribed and enabled

I cannot reproduce this problem

You didn't answer this. I take it that this is OK by now?

@kianzarrin
Copy link
Collaborator Author

@krzychu124 I made a few lines of simple changes. Can you please approve this.

@originalfoo
Copy link
Member

Regarding the performance issue while tmpe toolbar is open, like Kian I also don't see any drop in frame rate. Could be hardware specific though.

@krzychu124
Copy link
Member

Regarding the performance issue while tmpe toolbar is open, like Kian I also don't see any drop in frame rate. Could be hardware specific though.

Fps drop was caused by excessive log spam.

Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following questions are still without answers nor even comment (yes/no)
I'm not really perfectionist but I see possible bugs/typos and I'm not sure if they were intent(I can't read in minds, sorry).

TLM/TLM/Util/RoundaboutMassEdit.cs Outdated Show resolved Hide resolved
TLM/TLM/Util/RoundaboutMassEdit.cs Outdated Show resolved Hide resolved
TLM/TLM/UI/TrafficManagerTool.cs Outdated Show resolved Hide resolved
@kianzarrin
Copy link
Collaborator Author

kianzarrin commented Apr 17, 2020

I'm not really perfectionist but I see possible bugs/typos and I'm not sure if they were intent(I can't read in minds, sorry).

You are absolutely right!

The following questions are still without answers nor even comment (yes/no)

@krzychu124 this is github being stupid. I did put comments but it does not show up in the conversation tab. you should go to the files changed tab to see the comments I put in the files changed tab. Can you please do that and respond to me?

@kianzarrin
Copy link
Collaborator Author

@krzychu124 one more thing. Sometimes you have to press Load Diff for big files otherwise you might not see the comments.

renamed segmentList to prevent variable name collision.
added extra protections agaionst null segmentList
@originalfoo
Copy link
Member

image

@kianzarrin kianzarrin merged commit 4fa6a94 into CitiesSkylinesMods:master Apr 18, 2020
@kianzarrin kianzarrin deleted the 542-edit-selected-roads branch April 18, 2020 05:08
@originalfoo originalfoo linked an issue Apr 18, 2020 that may be closed by this pull request
@originalfoo originalfoo added the MASS EDIT The mass edit tool label Apr 21, 2020
@kianzarrin kianzarrin mentioned this pull request May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JUNCTION RESTRICTIONS Feature: Junction restrictions LANE ROUTING Feature: Lane arrows / connectors MASS EDIT The mass edit tool Overlays Overlays, data vis, etc. PRIORITY SIGNS Feature: Stop / Yield / Priority signs UI User interface updates
Projects
None yet
4 participants