-
-
Notifications
You must be signed in to change notification settings - Fork 3
Custom switches #136
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
base: master
Are you sure you want to change the base?
Custom switches #136
Conversation
…eate custom switches with track tools, add YardController
Mapify/Utils/Extensions.cs
Outdated
| RailTrack track = null; | ||
| var first = false; | ||
|
|
||
| foreach (var foundTrack in Resources.FindObjectsOfTypeAll<RailTrack>()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be Object.FindObjectsOfType<RailTrack>(), no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I think this was intentional:
Contrary to Object.FindObjectsOfType this function will also list disabled objects.
https://docs.unity3d.com/ScriptReference/Resources.FindObjectsOfTypeAll.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does it need to find disabled objects? There are never disabled tracks afaik. All tracks should be under the railway parent anyway, so using GetComponentsInChildren<RailTrack>(true), or using RailTrackRegistry, would accomplish the same goal in a safer and more performant manner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using GetComponentsInChildren now. The RailTracks really are inactive though, GetComponentsInChildren<RailTrack>(false) does not work.
|
Requesting a review from @WhistleWiz for the railway editor changes, I'm not familiar with that code at all. |
| return false; | ||
| } | ||
|
|
||
| //TODO bug: this check sometimes gives a false positive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are any handles being created with length 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check that next time this bug appears. I don't really know how to reproduce this.
| } | ||
|
|
||
| public bool IsSwitch => ParentSwitch != null; | ||
| public bool IsSwitch => GetComponentInParent<SwitchBase>() != null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is run every time, is that alright?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only used in the editor and at runtime in TrackSetup. So I think it's fine.
| } | ||
|
|
||
| private void DrawCurveOptions() | ||
| private void DrawCurveOptions(bool CustomSwitchBranch = false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a separate method, instead of shoving the entire method in ifs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is mostly the same for custom switches and vanilla switches. If I made a separate function for custom switches, there would be a lot of duplicated code. I honestly think it is better to leave it like this.
|
|
||
| /// <summary> | ||
| /// Returns the speed limit shown on track speed signs for a given radius. | ||
| /// source: DV.Signs.SignPlacer.GetMaxSpeedForRadius |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recheck comments so they follow capitalisation and spacing rules (not just in this file).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What rules are you referring to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These.
In this specific line it's more important, as it is an XML comment and shows when you hover any references to this method, so it really should be changed: source -> Source.
Then in other files like TrackToolsCreator.cs: //right of center -> // Right of center.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
|
@WhistleWiz this one is ready for review :) |
Adds custom switches: the ability to create switches in any shape you like, with any number of branches.
Demo video
Test map. Go to station 'yard test' in the south-west of the map.