Replace CheckLane() with extensions

[originalfoo]

Adds new extensions:

• laneId.IsValid()
• laneId.IsValidWithSegment() -- this replaces CheckLanes(laneId)
• segmentId.IsValid()

Compared to existing extensions, these new extensions additionally check that the id's are non-zero.

The PR also simplifies some existing code in Flags.cs by using the new extensions, and also updates some xmldoc to be more consistent.

[kianzarrin]

I think we should wait until my laneconnectionmanager PR goes in to reduce conflicts.

[kianzarrin]

we already have this in lane extensions:

        public static bool IsValidWithSegment(this ref NetLane netLane) {
            return netLane.IsValid()
                && netLane.m_segment.ToSegment().IsValid();
        }

~~But it does not check for laneId < 0

there is the minor issue that we some times check for laneId < 0 and sometimes don't based on the mood of the programmer at the time!~~

[kianzarrin]

wait uint cannot be less than zero anyway so its fine.
I think we should use the old overload instead of introducing this new one.

[originalfoo]

Yeah, I know unsigned numbers can't be negative (no idea why so much old code has the <= check on ushort and uint).

But regardless, that's not why we check id != 0, is it?

[kianzarrin]

checking segmentId != 0 is redundant because m_segments[0] holds the default value and therefore is not valid.

[kianzarrin]

I think we should just use the old extension.
If checking segmentId != 0 is important - for example if you think someone can accidentally write something to m_segments[0] - then we should do it consistently everywhere.
But That is OCD in my opinion. the old interface would do.

[originalfoo]

If we're sure lane / segment zero will never have Created flag set, then yeah, we could ditch the id != 0 check.

@krzychu124 thoughts on this?

[krzychu124]

First element is always thrown away on load so it should have flag None unless other mod used it and then got serialized to the savegame (bug) (worth checking if they also skip first element or not)

[originalfoo]

So it should be ok to ditch the id != 0 checks?

[krzychu124]

I would check where CO is checking for != 0 -> they know where it is possible. We can only guess

[kianzarrin]

First element is always thrown away on load so it should have flag None unless other mod used it and then got serialized to the savegame (bug) (worth checking if they also skip first element or not)

If that is the case then we should always check for id != 0.
it would be inconsistent to have 2 extensions methods to check if valid and then we pick one or the other randomly.
In any case I consider this to be a low priority issue. I think its OK to merge the code like this but it might be worth improving it.

[originalfoo]

I've realised there are some other issues with this PR, notably that an IsValid() extension of a uint / ushort has no way to determine what class those ids refer to, which is a huge flaw in my thinking of this PR. That being the case, we should always be doing .ToWhatever() first and then IsValid() - but is there any way to, for example, get the lane id from a NetLane instance in the IsValid() stage?

If we need to check != 0 how do we implement that in the least crufty/cumbersome manner?

[kianzarrin]

That was what I was afraid of (I should have communicated that better). I thin we should minimize the use of (interger).Extensions(). I think ToNode() or ToSegment() is fine but more than can cause trouble.

[originalfoo]

One thing I was thinking to include id != 0 check in IsValid() is extensions such as uint.IsValidLane() and ushort.IsValidSegment(), example:

internal static bool IsValidLane(this uint laneId)
    => laneId != 0 && laneId.ToLane().IsValid();

// call site example:

if (!someLaneId.IsValidLane())
    return;