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

Integration of Vanilla and TMPE priority roads #542

Closed
kianzarrin opened this issue Nov 2, 2019 · 39 comments · Fixed by #631
Closed

Integration of Vanilla and TMPE priority roads #542

kianzarrin opened this issue Nov 2, 2019 · 39 comments · Fixed by #631
Assignees
Labels
enhancement Improve existing feature MASS EDIT The mass edit tool PRIORITY SIGNS Feature: Stop / Yield / Priority signs
Milestone

Comments

@kianzarrin
Copy link
Collaborator

Currently when using the priority signs tool, holding shift highlights the road (or at least it should). if you Shift+click then it turns the whole highlighted road to priority road by requiring the connected roads to yield. It distinguishes between priority road and connected road by angle. This is done using SegmentTraverser class.

The vanilla game also has a feature to create priority road. but its done slightly differently. It allows you to click on a road and choose a road segment. then puts stop signs on connecting roads.

As you can see in the pictures bellow there is a disconnect between Vanila and TMPE priority roads:
round about segment
semi roundabouts
hover issue long road
hover issue long road2

Each approach has advantages and disadvantages:

  • Vanila: slower but more flexible in terms of selecting what constitutes a priority road (lets call it vanilla road selector). It also cannot add yield signs.
  • TMPE: Its faster to use but less flexible. many people will resort to deleting part of their road in order to limit the length of the priority road. also in complex situation its hard to select what is the priority road. Its also inconsistent with Vanilla.

I suggest to integrate the TMPE and vanilla approaches. We can discuss what the final solution would look like. possible solutions are:

  1. Best of both worlds: when you shift-click with TMPE priority sign tool, it modifies the path selected by the vanilla road selector.
  2. Get rid of the TMPE shift modifier key and instead change the Vanilla UI so that you can choose what to do with the priority road ( eg: yield, stop, etc), .
  3. Don't touch the game UI. instead modify the TMPE settings in terms of what a priority road should do( eg: yield, stop, etc).
    in approach 2 or 3 we can also add functionality for these closely related issues: closely related issues : shortcut to fix traffic rules where minor road joins a main avenue #541 Shortcut for fixing all round about rules in one click #539 .

other related issues #29 #7

@kianzarrin kianzarrin added feature A new distinct feature triage Awaiting issue categorisation labels Nov 2, 2019
@kianzarrin
Copy link
Collaborator Author

please assign to me.

@kianzarrin
Copy link
Collaborator Author

I am trying to figure out how to get the selected road segment in my code.

@kianzarrin
Copy link
Collaborator Author

Found it!
Screenshot (5)

@kianzarrin
Copy link
Collaborator Author

I cannot access it from the code:

namespace testspce {
    using System.Diagnostics;
    using ColossalFramework;
    using UnityEngine;
    using UnityEngineInternal;
    using ColossalFramework;
    using ICities;
    public class testclass {
        private void testcode(uint segmentId, uint nodeId) {
            var a = Singleton<NetAdjust>.instance.m_segmentQueue; //error : m_segmentQueue does not exist
            var b = Singleton<NetManager>.instance.NetAdjust.m_segmentQueue; //error : m_segmentQueue does not exist
            var c = NetAdjust.m_segmentQueue; //error : m_segmentQueue does not exist
        }
    }
}

any ideas?

@kianzarrin
Copy link
Collaborator Author

OK Its a private member so I need to use reflection

@kianzarrin
Copy link
Collaborator Author

kianzarrin commented Nov 3, 2019

OK got reflection to work. the variable I am looking for is NetAdjust.tempPath there are other similar variables there too but tempPath is the one we can most easily traverse.

The game can store multiple paths for different roads (Segments cannot be part of 2 paths). I had no luck so far trying to find out where all these paths are stored. I can only get the active path. but maybe that is good enough depending on what we want to do.

@brunoais
Copy link

brunoais commented Nov 4, 2019

I would keep the multi-click for different outcomes TMPE has. Usually, much faster to use than selecting from a menu.
I like the idea of integrating the vanilla priority road builder with TMPE's priority road.
But... I do want to keep the options for yield and stop signs the vanilla game has.

@kianzarrin
Copy link
Collaborator Author

@brunoais That's what I want too. I suspect that's how vast majority of people would like to use this tool in vast majority of situations.
So its the best of both worlds option I talked above. If we want to do that I think I only need to look at the active NetAdjust path. less work for me :)

@originalfoo originalfoo added enhancement Improve existing feature PRIORITY SIGNS Feature: Stop / Yield / Priority signs and removed feature A new distinct feature triage Awaiting issue categorisation labels Nov 5, 2019
@kianzarrin
Copy link
Collaborator Author

kianzarrin commented Nov 21, 2019

I am nearly done with this:
The screenshot bellow shows my new UI. The drop box values are : None, Yield, Stop, All. The "ALL" option will fix all traffic rules. The arrears both in Road adjust panel and Road world info panel because usually only one of them is visible.
Screenshot (73)

Any suggestions?

edit:

  • The algorithm shall auto-detect roundabouts (i.e. one way loop)

@originalfoo
Copy link
Member

originalfoo commented Nov 21, 2019

It's not very clear what the options are doing (unless you already know what they are doing).

Instead of a drop down list, why not use icon buttons? Something like:

When joining this road, drivers should: 🛑 🔻

In case of a roundabout detection, a third icon could be added when applicable, for example: ⚪️. Clicking the roundabout button would apply special ruleset applicable to roundabouts.

The buttons would have useful tooltips, for example (this just off top of my head, needs to be much more concise):

  • Yield: Drivers must yield before joining this road
  • Stop: Drivers must stop before joining this road
  • Roundabout: Apply advanced rules specific to roundabouts

When panel opens, or route altered, scan the "smaller" side-roads linked to the route to see what most of them are using: If that's stop or yield show the associated button as selected, otherwise neither button is selected.

  • Clicking a selected button deselects it, effectively returning smaller-road junctions to vanilla
  • Clicking a non-selected button will select it, applying all relevant customisations to smaller-road junctions

@originalfoo
Copy link
Member

Note also: When adding yield or stop rules, the vanilla stop rule (Junctions tab of Traffic Routes info view) should also be applied. TM:PE will still take precedence but most segment prefabs will show additional on-map signage/decals when the vanilla stop flag is set.

@kianzarrin
Copy link
Collaborator Author

THanks @aubergine10 . you are right. I have to trash my work and use your approach. I need One more Sprite for #541 with the icon of go through blocked junction and/or no left turn.

@originalfoo
Copy link
Member

The "ALL" option will fix all traffic rules.

Could you elaborate on this?

I assume that if user clicks one of the other three (yield, stop, roundabout) it would automatically apply relevant customisations for:

@kianzarrin
Copy link
Collaborator Author

@aubergine10

The "ALL" option will fix all traffic rules.

Could you elaborate on this?

it does exactly what #541 does but for the segments selected by the user. that means no left turns go through blocked junctions no zebra crossing on avenue, yield, etc ... and its according to the settings set it in the options.

@originalfoo
Copy link
Member

Are user's choices retained for the road? For example, if I click Yield button, it will set the existing smaller-roads to yield when entering the road, but what if I add new connecting roads, or alter the road route?

@kianzarrin
Copy link
Collaborator Author

kianzarrin commented Nov 21, 2019

Are user's choices retained for the road? For example, if I click Yield button, it will set the existing smaller-roads to yield when entering the road, but what if I add new connecting roads, or alter the road route?

No, but this is not the case in vanilla case either. the vanilla priority road option does not remember your last choice. this is how The vanilla works:

  • if the user turns on the priority road toggle, then put stop signs on all the roads that join the priory road.
  • the next time the user selects the road if: all the roads connected to the current selection have stop signs then automatically turn on the priority button.
  • if the user turns off the priority toggle then remove the stop signs.
  • I think there is also a bug that sometimes it forgets to remove traffic lights.

I am not planning to remember the last choice either. not in my first commit at least.

EDIT: I don't even know where the computer stores selected segments (although I know it does remember the selection). I only know how to access a private variable that has the last selection.

EDIT 2: The vanilla functionality is way too basic and stupid. I never use.

@kianzarrin
Copy link
Collaborator Author

road-edit-btns
My new button textures to put in the adjust road selection panel

@originalfoo
Copy link
Member

Some of those textures already exist, so we should reuse existing ones if possible.

@kianzarrin
Copy link
Collaborator Author

kianzarrin commented Nov 25, 2019

Screenshot (107)
My new UI.

Intended functionality:

  • clicking on any button will highlight that button and set traffic rules. it also removes highlights from other buttons.
  • clicking again on the button removes the highlight and unsets traffic rules.
  • clicking on clear button will clear all traffic rules. This button cannot get activated
  • modifying the road selection will remove highlight from all buttons
  • the code does not remember past road selections. clicking on a different road selection removes highlight from all buttons.
  • clikcing on roundabout button is like Shortcut for fixing all round about rules in one click #539
  • clickin on enterblocked_allowed button is like shortcut to fix traffic rules where minor road joins a main avenue #541

tagging @aubergine10

things left to do:

@originalfoo
Copy link
Member

originalfoo commented Nov 26, 2019

Looking good!

Roundabout button

I would expect the roundabout button to only appear if the named road forms a closed loop.

Enter blocked junction button

Do we need the separate Enter blocked junction button? My assumption (and I think users will make the same assumption) would be that selecting either Yield, Stop, or Roundabout, would automatically add/remove the Enter blocked junction where applicable.

Vanilla Stop signs

When applying our Yield/Stop/Roundabout rules, I would expect the vanilla Stop sign (as seen on Junctions tab of Traffic Routes info view) to be applied at any segments that are given a TM:PE-styled Stop or Yield sign.

This is to integrate with roads assets designed to adapt to yeilding/stopping traffic. A good example is the UK Roads Revived roads, which will show a "Give Way" sign and additional decals when vanilla Stop sign is placed.

What if junction has traffic lights?

My assumption is that any junctions with traffic lights will remain unaltered; they will not get Yield, Stop (or vanilla Stop) signs, and will retain their traffic light.

However, I note that TM:PE timed traffic lights has some integration with TM:PE priority signs (if enabled in mod options) so that might need some extra thought?

Vanilla priority road checkbox

I agree that the vanilla "Priority road" checkbox should be removed.

Note however, that vanilla game "remembers" the priority road setting - I just did a quick test and confirmed this.

At a guess, it just checks to see if all applicable side roads have a vanilla Stop sign set. Or it could be setting an as-yet undiscovered flag somewhere (on the road segments perhaps)? Either way, you will have to track down code that sets default state to avoid exceptions once that checkbox has been removed from UI.

If checkbox is removed from UI, disabling or unsubbing TM:PE from main menu should revive it.

Display existing state if possible

This could be done as a later PR, as I expect it is somewhat cumbersome to implement.

I would expect it, if possible, to scan the road to see if applicable side roads are set to one of the configurations. That way I can click a road and see if anything needs updating (if icon is highlighted, I know all the side roads are already configured, otherwise I can choose what config to apply).

Note: If vanilla sets an as-yet undiscovered 'Priority Road' flag somewhere, then we should ideally also set that flag should user choose either Yield, Stop or Roundabout.

Without this, I have no way of knowing if the whole road is configured as I expect it to be and would end up re-applying rules each time "just in case".

UI layout

In terms of UI, it might be worth having a method that adds an extra panel (with the lighter grey background like that shown on Adjust Roads tab) and then put everything in to that. That way, all the functionality will be gathered together coherently and we can add extra stuff to it in future if applicable.

Rough mockup of panel contents:

Which traffic rules should be applied to this road?

🔻 🛑 ⚫️ =

Tooltips of icons could be something like (just an initial draft):

  • Yield: Traffic on the road has priority; joining traffic must yield
  • Stop: Traffic on the road has priority; joining traffic must stop
  • Roundabout: Apply special roundabout rules
  • Equal: Treat all traffic equally; remove any priority rules

On the Adjust Roads tab, the panel would be added below the existing vanilla panel. On the Road info popup, the panel would be added below the existing content.

@kianzarrin
Copy link
Collaborator Author

I am struggling to find a good sprite to use for the button responsible for quick-setup of priority road. any ideas?
tagging @kvakvs @aubergine10

@kvakvs
Copy link
Collaborator

kvakvs commented Jan 23, 2020

Make a temporary ugly sprite. Show it to us and we might be able to improve it.
Or someone can feel inspiration and combine 1-2 free icons for you in Ms Paint.

@kianzarrin
Copy link
Collaborator Author

kianzarrin commented Jan 23, 2020

@kvakvs well now I am going to use this:
sign_priority
EDIT: the edges are supposed to be white.

@kianzarrin
Copy link
Collaborator Author

New sprites
road-edit-btns2

@kianzarrin
Copy link
Collaborator Author

I modified LinearSpirte to add support for disabled buttons. The buttons can now have 3 states(the number of states is configurable).

  • State disabled: when user has not selected any road/roundabout
  • State deactive: when the user has a valid road selection. Clicking will results in setting up traffic rules and sending the button to active state.
  • State active: Clicking will clears the changes the user has done. ( in future we can have a more clever undo. related: Undo/delete feature #568).
    road-edit-btns3.

Added Created OnSelectionChange event for when user changes path selection.

@kianzarrin
Copy link
Collaborator Author

kianzarrin commented Jan 25, 2020

This task is nearly complete.

The images bellow show the round about button in disabled(selection is not a round about), enabled (selection is a roundabout), active states(round about rules applied). clicking an active button again will undo traffic rules (undo is simplistic pending #568)

Screenshot (335)
Screenshot (337)
Screenshot (338)
Screenshot (339)

@kvakvs
Copy link
Collaborator

kvakvs commented Jan 25, 2020

Ok now that i've seen it on the real UI not just sprites.
The style you've chosen for icons, background, and size does not match game UI at all. Can you copy game colors more closely at least for the backgrounds?

@kianzarrin
Copy link
Collaborator Author

@kvakvs thanks for the feedback.
I wish to create pull review for this soon. Is there anything else you want to add?
tagging @aubergine10

@kianzarrin
Copy link
Collaborator Author

New sprites
road-edit-btns

@kianzarrin
Copy link
Collaborator Author

screenshots show how buttons appear.
Screenshot (349)\

Mass edit overlay is enable while road selection panel is active.
Screenshot (348)

this work is complete. I will polish the code and put it for review.

@ItzNxthaniel
Copy link

I don't even play this game, nor use any mods, but just some feedback. Those icons DO NOT match at all. If I used this mod, I would unsubscribe to this if those icons were posted.

@originalfoo
Copy link
Member

And what would you make them look like @Goomig ? We're always open to suggestions :)

@ItzNxthaniel
Copy link

Idk man they look really poppy

@ItzNxthaniel
Copy link

They look to 'noticeable'

@kianzarrin
Copy link
Collaborator Author

kianzarrin commented Feb 20, 2020

I can make them smaller. Icon design is not my specialty.
The colors are the same as game colors. The size is bigger. I was struggling with circular borders so I made it rectangle.

The game buttons have fewer states than my. when the game button is in active state it does not change face when mouse hovers or mouse is pressed.

@originalfoo
Copy link
Member

The UI stuff @kvakvs is working on should make button states easier.

kianzarrin added a commit to kianzarrin/Cities-Skylines-Traffic-Manager-President-Edition that referenced this issue Apr 4, 2020
- remove buttons from the road adjust panel
- hovering over button should show netadjust path
- show mass edit overlay when we are in the road adjust mode but the panel is invisible.
- hide RoadWoldInfoPanel in All traffic routes pannels but road adjust. (patch SetMode)
- clicking road adjust panel should not hide road info view but modifying the path should.
- openning netadjust -> the road info panel should show
- TMPE normal view with green hover selecting roads.
@originalfoo originalfoo modified the milestones: Priority road, 11.5.0 Apr 18, 2020
@originalfoo originalfoo added the MASS EDIT The mass edit tool label Apr 21, 2020
This was referenced May 7, 2020
@ItzNxthaniel
Copy link

ItzNxthaniel commented Sep 1, 2020

I can make them smaller. Icon design is not my specialty.
The colors are the same as game colors. The size is bigger. I was struggling with circular borders so I made it rectangle.

The game buttons have fewer states than my. when the game button is in active state it does not change face when mouse hovers or mouse is pressed.

I apologize for being a douche about the icons. They do look really nice, the only thing I would change is the gradient on the boxes. I'm getting a very nice Flat UI Feel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing feature MASS EDIT The mass edit tool PRIORITY SIGNS Feature: Stop / Yield / Priority signs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants