TMPE icon indicating copy to clipboard operation
TMPE copied to clipboard

Make more use of LaneUtil, etc.

Open originalfoo opened this issue 2 years ago • 7 comments

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.

originalfoo avatar Mar 24 '22 17:03 originalfoo

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

kianzarrin avatar Mar 24 '22 18:03 kianzarrin

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)) {
   // ...
}

originalfoo avatar Apr 03 '22 14:04 originalfoo

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.

kianzarrin avatar Apr 03 '22 20:04 kianzarrin

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

krzychu124 avatar Apr 04 '22 16:04 krzychu124

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.

kianzarrin avatar Apr 04 '22 16:04 kianzarrin

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

krzychu124 avatar Apr 04 '22 20:04 krzychu124

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.

kianzarrin avatar Apr 04 '22 21:04 kianzarrin