RemoteTechnologiesGroup / RemoteTech

Community developed continuation of Kerbal Space Program's RemoteTech mod.
http://remotetechnologiesgroup.github.io/RemoteTech
GNU General Public License v2.0
230 stars 103 forks source link

Providing EVA suit with antenna creates NRE #770

Closed Gordon-Dry closed 5 years ago

Gordon-Dry commented 5 years ago

In my tests for the new RT API / Kerbalism implementaion of that API I found this 44x written to the log after leaving the pod for going EVA:

ArgumentOutOfRangeException: Argument is out of range.
Parameter name: index
  at System.Collections.Generic.List`1[RemoteTech.ISignalProcessor].get_Item (Int32 index) [0x00000] in <filename unknown>:0 
  at RemoteTech.VesselSatellite.get_SignalProcessor () [0x00000] in <filename unknown>:0 
  at RemoteTech.VesselSatellite.get_Guid () [0x00000] in <filename unknown>:0 
  at RemoteTech.API.API+<>c.<GetClosestDirectGroundStation>b__10_0 (RemoteTech.SimpleTypes.NetworkRoute`1 r) [0x00000] in <filename unknown>:0 
  at System.Linq.Enumerable+<CreateWhereIterator>c__Iterator1D`1[RemoteTech.SimpleTypes.NetworkRoute`1[RemoteTech.ISatellite]].MoveNext () [0x00000] in <filename unknown>:0 
  at System.Linq.Enumerable.IterateNullable[NetworkRoute`1] (IEnumerable`1 source, System.Func`3 selector) [0x00000] in <filename unknown>:0 
  at System.Linq.Enumerable.Min[NetworkRoute`1] (IEnumerable`1 source) [0x00000] in <filename unknown>:0 
  at RemoteTech.API.API.GetClosestDirectGroundStation (Guid id) [0x00000] in <filename unknown>:0 
  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] in <filename unknown>:0 
Rethrow as TargetInvocationException: Exception has been thrown by the target of an invocation.
  at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] in <filename unknown>:0 
  at System.Reflection.MethodBase.Invoke (System.Object obj, System.Object[] parameters) [0x00000] in <filename unknown>:0 
  at KERBALISM.RemoteTech.NameTargetsKSC (Guid id) [0x00000] in <filename unknown>:0 
  at KERBALISM.ConnectionInfo..ctor (.Vessel v, Boolean powered, Boolean storm) [0x00000] in <filename unknown>:0 
  at KERBALISM.Vessel_info..ctor (.Vessel v, UInt32 vessel_id, UInt64 inc) [0x00000] in <filename unknown>:0 
  at KERBALISM.Cache.VesselInfo (.Vessel v) [0x00000] in <filename unknown>:0 
  at KERBALISM.Kerbalism.FixedUpdate () [0x00000] in <filename unknown>:0 

(Filename:  Line: -1)

I'm not sure why this was written to the log exactly 44x - but perhaps all vessels in my career got a total of 44 hops?

On Discord @HaullyGames mentioned target_name = status == LinkStatus.direct_link ? Lib.Ellipsis("DSN: " + (RemoteTech.NameTargetsKSC(v.id) ?? ""), 20) :

this means the RT API is return true when called by your EVA API.GetMethod("HasDirectGroundStation");

The first API say that EVA has direct connection with DSN Then the Kerbalism try to get the DSN name from other API, then fail because the EVA has no direct connection with DSN This EVA antenna is breaking RT I think

Is this solvable?

Gordon-Dry commented 5 years ago

First this is patched to the kerbalEVA:

// ============================================================================
// add Antenna to EVA suits
// ============================================================================

@PART[kerbalEVA*]:FOR[zzzKerbalism]
{
    %MODULE[ModuleCommand]
    {
        %minimumCrew = 0
        %hasHibernation = False
    }
}

@PART[kerbalEVA*]:FOR[Kerbalism]
{
    %MODULE[ModuleDataTransmitter]
    {
        %antennaType = DIRECT
        %packetInterval = 0.2
        %packetSize = 1.35
        %packetResourceCost = 0.375
        %requiredResource = ElectricCharge
        %antennaPower = 35000
        %optimumRange = 5000
        %packetFloor = 0.1
        %packetCeiling = 5
    }
}

// ============================================================================
// EVA Kerbals can remote control a probe
// ============================================================================

@PART[kerbalEVA*]:FOR[Kerbalism]
{
    %MODULE[ModuleProbeControlPoint]
    {
        %minimumCrew = 1
        %multiHop = False
    }
}

then this:

// a Kerbal on EVA can transmit EVA report via the next pod within 35km range
@PART[kerbalEVA*]:NEEDS[RemoteTech]:AFTER[RemoteTech]
{
    !MODULE[ModuleDataTransmitter],* {}

    %MODULE[ModuleSPUPassive] {}

    %MODULE[ModuleRTAntenna]
    {
        %TechRequired = start
        %Mode0OmniRange = 500
        %Mode1OmniRange = 35000
        %EnergyCost = 0.0375
        %IsRTActive = true

        %TRANSMITTER {
            %PacketInterval = 0.2
            %PacketSize = 1.35
            %PacketResourceCost = 0.00375
        }
    }
}
HaullyGames commented 5 years ago

Ok, First problem guys You can't assume var will be 'null' without cause error, if any one send a invalid GUID, it will cause an IndexOutOfRangeException, since it is not in your Satellites array. replace var satellite = RTCore.Instance.Satellites[id]; to var satellite = RTCore.Instance.Satellites.Where(sat => sat.Guid.Equals(id)).FirstOrDefault(); This will solve the Exception if any other mod send invalid GUID. PR - https://github.com/RemoteTechnologiesGroup/RemoteTech/pull/772

Also, I think you could include some Log like "Invalid GUID" before return, I didn't add it.

KSP-TaxiService commented 5 years ago

Hi, I don't understand how IndexOutOfRangeException occurs on RTCore.Instance.Satellites[id];.

I tried to reproduce with a valid GUID (but not matched to anything) on RTCore.Instance.Satellites[id]; and receive satellite = null based on this method.

Can you give an example of the invalid GUID you observed at your end?

I am not too excited about this PR (seems like addressing the symptoms)

HaullyGames commented 5 years ago

I'm not 100% sure because I didn't create the EVA antenna, but looks like it is related to EVA, something between the process to go "onBoard", the EVA GUID still in our cache(Kerbalism), but doesn't exist in your Satellite list any more. Looks like RT updated before our Kerbalism, then we are sending a GUID to RT, but this GUID doesn't exist any more there.

Looks like that this behavior is more expected when the game has a lot of delay (Hardware or something like that).

Talking about the PR, it is just to prevent RT cause IndexOutOfRangeException when any other mod send an invalid GUID to RT API, otherwise your API will have a exception and people will think that is RT problem and not they mod that is sending an invalid GUID.

HaullyGames commented 5 years ago

Yesterday I simulated to send GUID that doesn't exists and RT started to report a lot of Exceptions, I can double check tonight, but it was the same error that Gordon had reported here ArgumentOutOfRangeException: Argument is out of range. Parameter name: index

HaullyGames commented 5 years ago

Also, your API is not calling the method that you said, but it is trying to go direct to an index in your array.

KSP-TaxiService commented 5 years ago

All right, I accept your PR (after fixing indent chars) on API.cs. It is going to cost bits in every API call.

I can't reproduce this issue no matter what I tried with different ways.

Edit: Please help to verify at your end on the updated RT API.

KSP-TaxiService commented 5 years ago

@HaullyGames Last call before I publish the 1.9.0 release to 1.5.1, 1.4.5 and 1.3.1

HaullyGames commented 5 years ago

Everything works fine with the last develop version, Thanks! We are waiting for this release!

Gordon-Dry commented 5 years ago

Besides my late night partly-stupidity and some conceptual misunderstanding by pov - yes I'm fine with it.