zer0Kerbal / OhScrap

Scrapyard based part failure mod for Kerbal Space Program.
https://forum.kerbalspaceprogram.com/index.php?/topic/192360-*/
GNU General Public License v2.0
5 stars 11 forks source link

OhScrap + RemoteTech = NREs and "Antenna Safety Rating: -1" #46

Open theslams opened 2 years ago

theslams commented 2 years ago

With RemoteTech installed, antennae are showing "Antenna Safety Rating: -1" in the right-click menus where the safety rating is shown. (Screenshot included). It's not clear what effect that may be having on failures, etc, but I haven't yet experienced an antenna failure since noticing this problem in. Antennae do not show this issue without RT installed.

Screenshot 2021-12-08 171757

Log files indicate NREs for OhScrap. Player.log attached.

Reproduced using only:

Thanks for maintaining this mod! Please let me know if you need any further information.

github-actions[bot] commented 2 years ago

Thank you. Kindly read contributiing.md, code_of_conduct.md and styleguide.md.\n These are boilerplate.

keyspace commented 1 year ago

This comment is probably useless to OhScrap devs, but might be useful to someone like me, trying to puzzle out what's happening under the hood. In particular, why there are separate failure MODULEs for whether RT is or isn't installed.

EDIT: This is with OhScrap v2.2.0.0, via CKAN.


Grepping the log:

$ cat KSP.log | grep "\[OhScrap\]" | grep RemoteTech
[LOG 10:23:51.857] Config(@PART[*]:HAS[@MODULE[ModuleDataTransmitter]]:NEEDS[!RemoteTech,OhScrap,ScrapYard]:FOR[OhScrap]) OhScrap/Patches/AntennaFailureModule/@PART[*]:HAS[@MODULE[ModuleDataTransmitter]]:NEEDS[!RemoteTech,OhScrap,ScrapYard]:FOR[OhScrap]
[LOG 19:51:16.394] Deleting root node in file OhScrap/Patches/AntennaFailureModule node: @PART[*]:HAS[@MODULE[ModuleDataTransmitter]]:NEEDS[!RemoteTech,OhScrap,ScrapYard]:FOR[OhScrap] as it can't satisfy its NEEDS
[LOG 10:23:54.850] [OhScrap]: RemoteTech Detected.

A few hints from here:

  1. Adding stock antenna failure mode is considered.
  2. It is not done, because RT will be loaded. A different module specifically for RT is considered instead.
  3. OhScrap detects RT later on.

All seems good here.


Looking at a save file, an active vessel in orbit, I see an antenna PART has MODULEs for ModuleRTAntenna, RTAntennaFailureModule, ModuleSYPartTracker... but no ModuleDataTransmitter!

Huh?

Digging on in RT's issue tracker, there's this dev comment:

... Also, if a part contains ModuleRTAntenna, you must remove ModuleDataTransmitter from the part. We have our own ModuleRTDataTransmitter in place of the stock module. ...

Indeed, if I'm reading correctly from their config:

@PART[*]:HAS[@MODULE[ModuleDataTransmitter]]:FOR[RemoteTech]
{
    !MODULE[ModuleDataTransmitter]{}
}

The file then goes on to modify the stock antennas to have ModuleRTAntenna, and have a TRANSMITTER node. The latter is then added dynamically elsewhere.

RT's own antenna parts also have ModuleRTAntenna.

Strangely, in my save file RT's RTShortAntenna1 part lacks a ModuleRTDataTransmitter, whereas the stock longAntenna (patched by RT) has it. 😕


None of this explains why antennas never fail, why safety rating is shown as -1, or which object accesses result in NREs.

Perhaps some ScrapYard digging is required...

keyspace commented 1 year ago

So, the NREs are of this form (from my own KSP.log, very messy, lots of other mods):

[EXC 16:59:21.142] NullReferenceException: Object reference not set to an instance of an object
        OhScrap.BaseFailureModule.Start () (at <33d1171cdc59483c937a1b33fa63bc5e>:0)
        UnityEngine.DebugLogHandler:LogException(Exception, Object)
        ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
        UnityEngine.Debug:CallOverridenDebugHandler(Exception, Object)
[EXC 16:59:21.152] NullReferenceException: Object reference not set to an instance of an object
        OhScrap.BaseFailureModule.Initialise () (at <33d1171cdc59483c937a1b33fa63bc5e>:0)
        OhScrap.BaseFailureModule.FixedUpdate () (at <33d1171cdc59483c937a1b33fa63bc5e>:0)
        UnityEngine.DebugLogHandler:LogException(Exception, Object)
        ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
        UnityEngine.Debug:CallOverridenDebugHandler(Exception, Object)

Start(), Initialise(), FixedUpdate() - that's fairly early, and safetyRating staying at -1 suggests it stays at the initial value.


To think of it, I'm playing with FAR, and I've also never had a winglet or parachute fail...

What surely distinguishes these from regular, "not-already-modded" parts is the way the appropriate module is acquired in Override(), e.g. when RT is in play:

https://github.com/zer0Kerbal/OhScrap/blob/ca27df71541dfb56678fe214e2b9cab1e8f92ced/source/OhScrap/FailureModules/RTAntennaFailureModule.cs#L24-L33

Whereas when it is not:

https://github.com/zer0Kerbal/OhScrap/blob/ca27df71541dfb56678fe214e2b9cab1e8f92ced/source/OhScrap/FailureModules/AntennaFailureModule.cs#L35

Even if not the cause, IMO this is the direction to dig in.

keyspace commented 1 year ago

After fixing current git master build issues as outlined in https://github.com/zer0Kerbal/OhScrap/issues/92#issuecomment-1528864438, and adding many debug prints, this narrows down to:

https://github.com/zer0Kerbal/OhScrap/blob/ca27df71541dfb56678fe214e2b9cab1e8f92ced/source/OhScrap/FailureModules/BaseFailureModule.cs#L178

The (dirty) logs look like:

[LOG 21:26:21.242] [OhScrap](XXXXX): Running overrides.
[LOG 21:26:21.242] [OhScrap](XXXXX): found a ModuleRTAntenna
[LOG 21:26:21.242] [OhScrap](XXXXX): Done running overrides.
[LOG 21:26:21.242] [OhScrap](XXXXX): part.FindModuleImplementing<ModuleUPFMEvents>
[LOG 21:26:21.242] [OhScrap](XXXXX): Referencing OhScrap instance...
[EXC 21:26:21.245] NullReferenceException: Object reference not set to an instance of an object
        OhScrap.BaseFailureModule.Start () (at <24c6e59a51634fe5ab17103afa5a3ee1>:0)
        UnityEngine.DebugLogHandler:LogException(Exception, Object)
        ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
        UnityEngine.Debug:CallOverridenDebugHandler(Exception, Object)
[LOG 21:26:21.257] [OhScrap](XXXXX): In FixedUpdate() and still not ready!
[LOG 21:26:21.257] [OhScrap](XXXXX): In Initialize()!
[LOG 21:26:21.257] [OhScrap](XXXXX): Are we ready?.. True
[LOG 21:26:21.257] [OhScrap](XXXXX): INIT 1
[EXC 21:26:21.258] NullReferenceException: Object reference not set to an instance of an object
        OhScrap.BaseFailureModule.Initialize () (at <24c6e59a51634fe5ab17103afa5a3ee1>:0)
        OhScrap.BaseFailureModule.FixedUpdate () (at <24c6e59a51634fe5ab17103afa5a3ee1>:0)
        UnityEngine.DebugLogHandler:LogException(Exception, Object)
        ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
        UnityEngine.Debug:CallOverridenDebugHandler(Exception, Object)

OhScrap is never set, resulting in a NullReferenceException immediately afterwards. So this part is not run:

https://github.com/zer0Kerbal/OhScrap/blob/ca27df71541dfb56678fe214e2b9cab1e8f92ced/source/OhScrap/FailureModules/BaseFailureModule.cs#L180-L185

However, FixedUpdate() runs almost immediately (as soon as the exception is "handled", I reckon), which calls Initialize(), which runs into OhScrap not being set again.

keyspace commented 1 year ago

Note: in comment above, I'm running a new "test" save in Sandbox mode, with all the same mods as previously (installed by CKAN), except for OhScrap - which was replaced manually.

Inspecting a TestCraft - consisting of 5 parts: pod, parachute, SRB, winglet, antenna - only the antenna lacks a ModuleUPFMEvents. That is, parts touched by FAR no longer seem immune to OhScrap with the git master version.

The antenna still being affected is probably related to:

https://github.com/zer0Kerbal/OhScrap/blob/ca27df71541dfb56678fe214e2b9cab1e8f92ced/GameData/OhScrap/Compatability/ModuleUPFMEvents.cfg#L1-L7

Here's the save file and KSP.log (with system / mod dll info).

persistent.sfs.txt KSP.log.txt

keyspace commented 1 year ago

The log (line 17690 and onwards) shows that:

  1. OhScrap does *FailureModule patching to a lot of parts - except RT antennas.
  2. OhScrap does ModuleUPFMEvents patching to those that already have *FailureModules, as per the rule in previous comment.
  3. RemoteTech applies its ModuleDataTransmitter patches.
  4. OhScrap applies RTAntennaFailureModule to parts that now have ModuleRTAntenna.

This is the culprit.

keyspace commented 1 year ago

Reviewing ModuleManager patch ordering docs, there may be an elegant solution here.

EDIT: See one-liner PR that follows.