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

Organized lane marker UI classes #701

Merged
merged 2 commits into from
Feb 16, 2020

Conversation

kianzarrin
Copy link
Collaborator

@kianzarrin kianzarrin commented Feb 14, 2020

as preparation for #681 and #682 I organized lane marker UI classes a bit. this review is mostly renaming stuff and a bit of encapsulation. the code path is the same.

I moved SegmentLaneMarker and NodeLaneMarker into separate files turning them into independent classes.
I introduced a private LaneEnd master class that is responsible for Sheath overlay in lane connector. The LaneEnd class has stolen most of the functionality of the NodeLaneMarker.

EDIT: test result:
Screenshot (488)

About that HOT REALOD label in the image above, it really means in game hot reload:

  • if you see HOT RELOAD it means that there has been hot reload (in game)
  • if you don't see HOT RELOAD that does not mean there was no hot-reload ... there could be a hot reload in the main menu.

I hope this does not mislead anyone in the feature. maybe I should rename it to IGHR(in game hot reload).

@kianzarrin kianzarrin self-assigned this Feb 14, 2020
@kianzarrin kianzarrin added code cleanup Refactor code, remove old code, improve maintainability DO NOT MERGE YET Don't merge this PR, even if approved, until further notice LANE ROUTING Feature: Lane arrows / connectors Overlays Overlays, data vis, etc. triage Awaiting issue categorisation labels Feb 14, 2020
@originalfoo
Copy link
Member

originalfoo commented Feb 14, 2020

Regarding splitting out the overlays stuff, see also: #630 - if things like segment and node highlights could be split out too, it would be good step towards achieving tool-agnostic overlays (which in turn would facilitate things like rendering vehicle paths and ultimately a visual pathfinder debugger).

For the in-game hot reload, I'd suggest ditching the mod name prefix (as a dev will already know exactly what they just built) so the toolbar title would be just In-game hot-swap {Version.Revision} - the revision just gives some indication of change if there are multiple hotloads.

Copy link
Collaborator

@kvakvs kvakvs left a comment

Choose a reason for hiding this comment

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

Nice so far.
Consider making a subdirectory there somewhere and move files to it, don't mix highlight modules with tool modules. You don't have to, but maybe makes sense?

TLM/TLM/Constants.cs Outdated Show resolved Hide resolved
@kianzarrin
Copy link
Collaborator Author

Consider making a subdirectory there somewhere and move files to it, don't mix highlight modules with tool modules. You don't have to, but maybe makes sense?

Its already in the helpers directory.

@kianzarrin kianzarrin requested a review from kvakvs February 16, 2020 17:38
@kianzarrin
Copy link
Collaborator Author

Is this ready to merge? multiple other tasks rely on this.

@kianzarrin kianzarrin merged commit d7015a5 into CitiesSkylinesMods:master Feb 16, 2020
@kianzarrin kianzarrin deleted the laneUI branch February 16, 2020 18:09
@originalfoo originalfoo removed the DO NOT MERGE YET Don't merge this PR, even if approved, until further notice label Feb 16, 2020
@originalfoo originalfoo added this to the 11.1.1 milestone Feb 19, 2020
@originalfoo originalfoo removed the triage Awaiting issue categorisation label Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup Refactor code, remove old code, improve maintainability LANE ROUTING Feature: Lane arrows / connectors Overlays Overlays, data vis, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants