Add Travel Distance Analyzer function.

[DmytroMuravskyi]

New function have two model:

• Calculate route between a set of destinations.
• Calculate distance statistics from a point of rooms of 1 or more types.

Routes are visualized as representation instances and visible when selected.

Function works or without doors

• When doors are present, only doors and any open parts of room boundaries, touching a corridor, are used as exits. Multiple exits per side is supported.
• When they are not present - if room side has open passage - it's used. Otherwise midpoint of wall portion that touches a corridor is considered an exit.

Future work:

• Distance calculation function is using recursion that can lead to stack overflow if room layout is very large. It will be replaced by loop based function when initial feedback is received.

---

This change is 

[katehryhorenko]

Reviewable status: 1 change requests, 0 of 1 approvals obtained, 8 unresolved discussions (waiting on @DmytroMuravskyi)

---

TravelDistanceAnalyzer/dependencies/CirculationSegment.g.cs line 1 at r1 (raw file):

//----------------------

Please add *.g.cs files to the ignore list

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 32 at r1 (raw file):

        public AdaptiveGrid Build(IEnumerable<CirculationSegment> corridors,
                                  IEnumerable<SpaceBoundary> rooms,
                                  List<WallCandidate>? walls = null,

Why do you need to make List and List nullable? List is a reference type and can be null

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 32 at r1 (raw file):

        public AdaptiveGrid Build(IEnumerable<CirculationSegment> corridors,
                                  IEnumerable<SpaceBoundary> rooms,
                                  List<WallCandidate>? walls = null,

Can it be IEnumerable and IEnumerable?

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 67 at r1 (raw file):

            foreach (var room in _roomExits)
            {
                var p = room.Key.Boundary.Perimeter.TransformedPolygon(room.Key.Transform);

Try to alwaays use meaningful names (e.g. 'polygon' or 'roomBoundary' instead of 'p')

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 113 at r1 (raw file):

            }

            var mainConnection = _grid.GetVertex(exit.Edges.First().OtherVertexId(exit.Id));

Are you sure that exit.Edges always has at least one value? Maybe it makes sense to add an additional verification:

Code snippet:

if (exit.Edges.Count > 2 || exit.Edges.Count < 1)
{
    return;
}

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 148 at r1 (raw file):

                var directionIndex = -1;
                var dot = delta.Unitized().Dot(xDir);
                if (dot.ApproximatelyEquals(1, 0.01))

create a private tolerance const with 0.01 value

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 197 at r1 (raw file):

        }

        private Edge? ClosestEdgeOnElevation(Vector3 location, out Vector3 point)

It is better to add the action name into the function name.

Edge? FindClosestEdgeOnElevation(Vector3 location, out Vector3 point)

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 243 at r1 (raw file):

        /// <param name="centerlines">Corridor segments with precalculated center lines.</param>
        /// <param name="grid">AdaptiveGrid to insert new vertices and edge into.</param>
        private void Intersect(List<(CirculationSegment Segment, Polyline Centerline)> centerlines)

CreateConnectionEdges or CreateIntersections

[DmytroMuravskyi]

Reviewable status: 1 change requests, 0 of 1 approvals obtained, 8 unresolved discussions (waiting on @katehryhorenko)

---

TravelDistanceAnalyzer/dependencies/CirculationSegment.g.cs line 1 at r1 (raw file):

Previously, katehryhorenko (Kateryna Hryhorenko) wrote…

Please add *.g.cs files to the ignore list

HyparSpace stores *.g.cs in all functions. There is actually a benefit to that as every commit can be built even if schema is changed.

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 32 at r1 (raw file):

Previously, katehryhorenko (Kateryna Hryhorenko) wrote…

Why do you need to make List and List nullable? List is a reference type and can be null

It's to prevent the warnings, I wanted the function to have 0 warnings and when null is assigned to non nullable type - compiler always produces one. There is also a logical difference: if walls null - Walls function is not in workflow. If empty list - function is there but there are no walls. The same is true for doors.

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 32 at r1 (raw file):

Previously, katehryhorenko (Kateryna Hryhorenko) wrote…

Can it be IEnumerable and IEnumerable?

Done.

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 67 at r1 (raw file):

Previously, katehryhorenko (Kateryna Hryhorenko) wrote…

Try to alwaays use meaningful names (e.g. 'polygon' or 'roomBoundary' instead of 'p')

Done.

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 113 at r1 (raw file):

Previously, katehryhorenko (Kateryna Hryhorenko) wrote…

Are you sure that exit.Edges always has at least one value? Maybe it makes sense to add an additional verification:

Grid should not have free vertices, this is exceptional situation. If this is possible, I want the exception, and not silent return.

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 148 at r1 (raw file):

Previously, katehryhorenko (Kateryna Hryhorenko) wrote…

create a private tolerance const with 0.01 value

Done.

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 197 at r1 (raw file):

Previously, katehryhorenko (Kateryna Hryhorenko) wrote…

It is better to add the action name into the function name.

Edge? FindClosestEdgeOnElevation(Vector3 location, out Vector3 point)

Done.

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 243 at r1 (raw file):

Previously, katehryhorenko (Kateryna Hryhorenko) wrote…

CreateConnectionEdges or CreateIntersections

Done.

[katehryhorenko]

Reviewable status: 1 change requests, 0 of 1 approvals obtained, 8 unresolved discussions (waiting on @DmytroMuravskyi)

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 32 at r1 (raw file):

Previously, DmytroMuravskyi wrote…

It's to prevent the warnings, I wanted the function to have 0 warnings and when null is assigned to non nullable type - compiler always produces one. There is also a logical difference: if walls null - Walls function is not in workflow. If empty list - function is there but there are no walls. The same is true for doors.

I see. So id you are planning to call Build method only with two input parameters (Build(corridors, rooms)), then yes, it's a correct approach. But I can see why you are getting the warning. Lets talk tomorrow :)

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 113 at r1 (raw file):

Previously, DmytroMuravskyi wrote…

Grid should not have free vertices, this is exceptional situation. If this is possible, I want the exception, and not silent return.

Then it's better to check the edges count and throw the exception at the begin of the function. You don't need to wait for exit.Edges.First() throw an exception

[DmytroMuravskyi]

Reviewable status: 1 change requests, 0 of 1 approvals obtained, 8 unresolved discussions (waiting on @katehryhorenko)

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 32 at r1 (raw file):

Previously, katehryhorenko (Kateryna Hryhorenko) wrote…

I see. So id you are planning to call Build method only with two input parameters (Build(corridors, rooms)), then yes, it's a correct approach. But I can see why you are getting the warning. Lets talk tomorrow :)

Yes, let's discuss. No, it's not about default input parameters, I removed them, as they are no longer used. It's about optional model dependencies: when doors or walls are not there they are null and this is much cleaner than empty list.

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 113 at r1 (raw file):

Previously, katehryhorenko (Kateryna Hryhorenko) wrote…

Then it's better to check the edges count and throw the exception at the begin of the function. You don't need to wait for exit.Edges.First() throw an exception

Done.

[katehryhorenko]

Hey! Overall looks good!
I added a few comments - suggestions. You can ignore them.

I few comments that I would ask you to fix:

• Keep the order of the class members. My suggestion is
  {
  private cons
  private fields
  constructor
  public properties
  public methods
  private methods
  }
  But do not mix public methods and private methods :)
• Some methods have comments for parameters that these methods do not have.

Comments on anything public are welcome!

Reviewable status: 1 change requests, 0 of 1 approvals obtained, 11 unresolved discussions (waiting on @DmytroMuravskyi)

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 104 at r4 (raw file):

                }
            }
            return exitVertex == null ? 0u : exitVertex.Id;

It's an alternative: exitVertex?.Id ?? 0u
But it's up to you what to use :)

Code quote:

exitVertex == null ? 0u : exitVertex.Id

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 248 at r4 (raw file):

        /// </summary>
        /// <param name="centerlines">Corridor segments with precalculated center lines.</param>
        /// <param name="grid">AdaptiveGrid to insert new vertices and edge into.</param>

Method doesn't have 'grid' input parameter

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 302 at r4 (raw file):

                        }

                        if (closestLeftProximity == -1 || closestRightProximity == -1)

Just a suggestion

Code snippet:

private const int InvalidProximity = -1;
private const int MaxProximityDifference = 2;

// ...

if (closestLeftProximity == InvalidProximity || closestRightProximity == InvalidProximity)
{
    continue;
}

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 375 at r4 (raw file):

        }

        private Edge? FindOnCollinearEdges(GridVertex start, ulong endId, Vector3 direction, Vector3 destination)

FindCollinearEdgeTowardsEndpoin

Code quote:

 FindOnCollinearEdges

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 379 at r4 (raw file):

            while (start.Id != endId)
            {
                GridVertex otherVertex = start;

The variable name otherVertex might be misleading because it's actually the next vertex in the loop. Consider renaming it to something like nextVertex for clarity.

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 428 at r4 (raw file):

        }

        private void ExtendToCorridor(Line l, CirculationSegment segment)

line

Code quote:

l

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 436 at r4 (raw file):

                var trimLine = new Line(l.Start - l.Direction() * maxDistance,
                                        l.End + l.Direction() * maxDistance);
                var inside = trimLine.Trim(transformedPolygon, out _);

intersectionLines

Code quote:

inside

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 454 at r4 (raw file):

        /// </summary>
        /// <param name="room">Room geometry.</param>
        /// <param name="centerlines">Corridor segments with precalculated center lines.</param>

Please fix the comment. 'grid' and 'centerLines' parameters do not exist :)

---

TravelDistanceAnalyzer/src/AdaptiveGridBuilder.cs line 462 at r4 (raw file):

            IEnumerable<Door>? doors)
        {
            var roomExits = new List<GridVertex>();

roomExitVertices

Code quote:

roomExits

[katehryhorenko]

Reviewable status: 1 of 1 approvals obtained, 10 unresolved discussions (waiting on @DmytroMuravskyi)