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

Turn-on-red feature: cleaned up code and translations #121

Merged
merged 38 commits into from
Mar 16, 2019

Conversation

VictorPhilipp
Copy link
Collaborator

Let's try this again

(cherry picked from commit 97ac58b)

# Conflicts:
#	TLM/TLM/Custom/AI/CustomCargoTruckAI.cs
#	TLM/TLM/Custom/AI/CustomRoadAI.cs
#	TLM/TLM/Custom/AI/CustomVehicleAI.cs
#	TLM/TLM/Custom/Data/CustomVehicle.cs
#	TLM/TLM/Custom/PathFinding/CustomPathFind2.cs
#	TLM/TLM/Traffic/Data/VehicleState.cs
#	TLM/TLM/TrafficManagerMod.cs
@originalfoo originalfoo added the enhancement Improve existing feature label Mar 2, 2019
Copy link
Collaborator

@FireController1847 FireController1847 left a comment

Choose a reason for hiding this comment

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

Overall it looks good (and glad to see my formatting fixed), just some things that appear to have been accidentally removed.

@krzychu124
Copy link
Member

Here we have problem, because my hidden link to game dlls archive won't work because this PR was made from outside this repo (Appveyor won't let me enable resolving hidden link due to security reasons...)
I have to find other way to this problem.

@VictorPhilipp
Copy link
Collaborator Author

I could close the PR and make a new one but I wanted to keep the review comments.

@FireController1847
Copy link
Collaborator

So I assume the current status on this is postponed until we implement #69?

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.

LGTM!

Copy link
Collaborator

@FireController1847 FireController1847 left a comment

Choose a reason for hiding this comment

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

Actually, I take back my approval temporarily. Cars need to be checking for priority to prevent, well, this...
ezgif-1-d27af0bc7cba

Previously I would use the prioMan.HasPriority to check for priority, maybe we need to do this within VehicleBehaviorManager?

Also, I know we discussed this, but I want to put this here for notekeeping, we found a yield velocity of 2.5 looks much better than 3. (Edit: He already committed it, so just keeping it for notes, lol)

@VictorPhilipp
Copy link
Collaborator Author

VictorPhilipp commented Mar 16, 2019

grafik
You could enable this option (for timed traffic lights only, at least for now)

Copy link
Collaborator

@FireController1847 FireController1847 left a comment

Choose a reason for hiding this comment

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

Enabled priority rules and the issue still persists but is less apparent, probably an issue with priority and not turn-on-red, looks good to me. I'm done testing.

Future testing should be done to try and implement priority rules to vanilla traffic lights. It's a huge necessity. Either that, or we should look into having the ability to automatically setup manual traffic lights, otherwise it'd quickly become too unrealistic to try and set up manual traffic lights for every intersection just because I don't want cars cutting in front of other cars.

@VictorPhilipp
Copy link
Collaborator Author

VictorPhilipp commented Mar 16, 2019

Priority rules require traffic measurement logic (class SegmentEnd). Currently, these structs are only instantiated for timed traffic lights.
#240

@krzychu124 krzychu124 merged commit 273fc7d into CitiesSkylinesMods:master Mar 16, 2019
@VictorPhilipp VictorPhilipp deleted the turn-on-red branch March 24, 2019 18:49
@originalfoo originalfoo added this to the 1.10.17 milestone Jul 16, 2019
@originalfoo originalfoo added Localisation Localised text and features UI User interface updates Usability Make mod easier to use technical Tasks that need to be performed in order to improve quality and maintainability labels Jul 16, 2019
@originalfoo originalfoo added JUNCTION RESTRICTIONS Feature: Junction restrictions TRAFFIC LIGHTS Feature: Traffic lights - toggle, timed, etc code cleanup Refactor code, remove old code, improve maintainability labels Aug 13, 2019
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 enhancement Improve existing feature JUNCTION RESTRICTIONS Feature: Junction restrictions Localisation Localised text and features technical Tasks that need to be performed in order to improve quality and maintainability TRAFFIC LIGHTS Feature: Traffic lights - toggle, timed, etc UI User interface updates Usability Make mod easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants