CitiesSkylinesMods / TMPE

Cities: Skylines Traffic Manager: President Edition
https://steamcommunity.com/sharedfiles/filedetails/?id=1637663252
MIT License
574 stars 85 forks source link

Make more use of LaneUtil, etc. #1489

Open originalfoo opened 2 years ago

originalfoo commented 2 years ago

_Originally posted by @kianzarrin in https://github.com/CitiesSkylinesMods/TMPE/pull/1467#discussion_r834235771_ :

Not sure if its within the scope but we can GetSegmentLaneIdsAndLaneIndexes() here and several other places in this file.

Also we can make use of LaneUtil.GetLaneIndex() LaneUtil.GetLaneInfo() LaneUtil.GetLaneId() in several places in this file.

Also I think maybe we should convert LaneUtil to extensions, eg. laneId.GetLaneIndex()? - the extension name makes it clear we're working with lanes.

kianzarrin commented 2 years ago

I think we should keep extensions for numeric types to a minimum because of type safety. ToLane/Segment/Node and nothing more.

originalfoo commented 2 years ago

Another thing that might be useful is creating iterators/enumerators (can't remember the name) for common tasks?

For example, stuff like this is very common:

for (int segmentIndex = 0; segmentIndex < Constants.MAX_SEGMENTS_OF_NODE; ++segmentIndex) {
    ushort segmentId = selectedNode.GetSegment(segmentIndex);
    if (segmentId == 0) {
        continue;
    }

    // ...
}

Would it be better to have something like:

foreach (ushort segmentId in segmentsAtNode(nodeId)) {
   // ...
}
kianzarrin commented 2 years ago
foreach (ushort segmentId in node.SegmentIds()) `

I did that for my mods because I am really lazy! also this might save a few lines of code and a local variable :). but its not important because chance of mistake is low.

for lanes there is high chance of mistake so an iterator was very useful.

krzychu124 commented 2 years ago

@aubergine10 keep in mind that enumerators may create garbage each use when they capture parameter or variable 😉

kianzarrin commented 2 years ago

keep in mind that enumerators may create garbage each use when they capture parameter or variable 😉

may is the keyword! if we properly define them using struct then they won't create garbage.

krzychu124 commented 2 years ago

may was intentional. It silently hides complexity. You can see nice example in ExtPathManager.FindPathPositionWithSpiralLoop. Calling that method recurrently may result in StackOverflowException if it goes too deep.

image

kianzarrin commented 2 years ago

depends on the definition of garbage. it does not create anything for GC. but if by garbage you mean excessive use of stack that could be avoided if we don't use enumerable, then I don't know if we can significantly reduce stack size if we don't use iterator.