Convoy: Error

Hi Adem,
 

I got the below error:

9/17 6:44:50 PM [Info] NullReferenceException: Object reference not set to an instance of an object
9/17 6:44:50 PM [Info] at Oxide.Plugins.Convoy+PathManager.DefineSpawnRotation () [0x0005b] in /server/carbon/plugins/Convoy.cs:4054
9/17 6:44:50 PM [Info] at Oxide.Plugins.Convoy+PathManager.GenerateNewPath () [0x00071] in /server/carbon/plugins/Convoy.cs:4001
9/17 6:44:50 PM [Info] at Oxide.Plugins.Convoy+EventLauncher.StartEvent (Oxide.Plugins.Convoy+EventConfig eventConfig) [0x0000f] in /server/carbon/plugins/Convoy.cs:1570
9/17 6:44:50 PM [Info] at Oxide.Plugins.Convoy+EventLauncher+<DelayedStartEventCoroutine>d__4.MoveNext () [0x0009f] in /server/carbon/plugins/Convoy.cs:1564
9/17 6:44:50 PM [Info] at UnityEngine.SetupCoroutine.InvokeMoveNext (System.Collections.IEnumerator enumerator, System.IntPtr returnValueAddress) [0x00026] in <...>:0
9/17 6:44:50 PM [Info] (Filename: /server/carbon/plugins/Convoy.cs Line: 4054)

NOTE: Below are the use of Fable to get a quick fix.

DefineStartPoint() returns null when it picks a road endpoint that has no navmesh within 2m. That happens on some roads, depending on the map: steep ground, bridges, water, custom monuments -- were running a custom map from here.

DefineSpawnRotation() runs first and dereferences the null. So instead of logging "route not found" and skipping this start, the event throws partway through startup. Because the start point is picked at random, it happens now and then rather than every time on our server.

This is the same in both versions, around line 4008 in 3.0.3 and 4010 in 3.0.4, the two fixes that at lest stop the error message are;

if (CurrentPath != null)
{
    CurrentPath.StartPathPoint = DefineStartPoint();

    if (CurrentPath.StartPathPoint != null)
        CurrentPath.SpawnRotation = DefineSpawnRotation();
}

With this change, a bad start point produces the "RouteNotFound" error. SpawnConvoy already checks for a null CurrentPath (Convoy.cs:1904) and stops the event cleanly, so the next scheduled event gets a new random attempt.

The same method has a second weakness. If the chosen start point has no connected points, secondPoint stays null and the return line throws the same exception. That's unlikely on normal roads, but a safe version of the last line is:

if (secondPoint == null)
    return Vector3.forward;
return (secondPoint.Position - CurrentPath.StartPathPoint.Position).normalized;

BTW; On the same code review spotted

1. There is test code for a specific steamid in there
 

                BasePlayer testPlayer = BasePlayer.activePlayerList.FirstOrDefault(x => x != null && x.userID == 76561198999206146);

                if (CurrentPath.IsRoundRoad)
                    newStartPoint = CurrentPath.Points.Where(x => PositionDefiner.GetNavmeshInPoint(x.Position, 2, out navMeshHit)).ToList().GetRandom();
                else if (testPlayer != null)
                    newStartPoint = CurrentPath.Points.Where(x => x.ConnectedPoints.Count == 1).ToList().Min(x => Vector3.Distance(x.Position, testPlayer.transform.position

2.  Issuing the command /convoyshowpath also seems to cause it to crash
 

Failed executing chat command 'convoyshowpath' in 'Convoy v3.0.4 by Adem' [callback] (Object reference not set to an instance of an object)
   at void Oxide.Plugins.Convoy+PathManager.DrawPath(EventPath eventPath, BasePlayer player) in /home/rust/rustcarbon/carbon/plugins/Convoy.cs:line 3971
   at void Oxide.Plugins.Convoy.ChatShowPathCommand(BasePlayer player, string command, string[] arg) in /home/rust/rustcarbon/carbon/plugins/Convoy.cs:line 1398
   at object System.Reflection.RuntimeMethodInfo.Invoke(object obj, BindingFlags invokeAttr, Binder binder, object[] parameters, CultureInfo culture)

3. Possibly routes arent cleared between events so the convoy can do weird things like travel a path, then turn and head back because of stale road closed flags -- is the intended?  And I think maybe mixed up in this its possible that the check only drops a route if another one covering the same roads has more points, so identical copies all stay.  I didnt have time to debug that one or test it.