post-kerbin-mining-corporation/KerbalAtomics

Exception Errors for MH engines with RealPlume in KA LH2NTR patch

GHrynk opened this issue · 3 comments

KA LH2NTR patch for Missing History engines throws Exception Errors when using RealPlume. See details in forum here:
https://forum.kerbalspaceprogram.com/index.php?/topic/130503-17x-kerbal-atomics-fancy-nuclear-engines-may-29/&do=findComment&comment=3615139

The exceptions are triggered by this stanza in the LH2NTR patches for the nuclearEngine_size0 and nuclearEngine_1p5 MissingHistory parts:

@EFFECTS
{
        fx-sc-core
        {
                // Copy from the corresponding effect.
                #../running_closed/AUDIO {}
                #../running_closed/MODEL_MULTI_PARTICLE {}
        }
}

The # lines are ModuleManager syntax for copying stuff from some other node (identified by a relative path) into the current node. Those engines' EFFECTS blocks have a running_closed effect that defines audio and particles, and the fx-sc-core block wants to use the same audio and particles.

The problem is that RealPlume replaces the running_closed block with a different one, called Hydrogen-NTR-HighTemp, so running_closed doesn't exist when the patch above tries to copy from it. (And MM apparently doesn't handle that situation very well; it throws an exception instead of just reporting that the patch has an error.)

The way RealPlume makes its changes is:

  • At :FOR[RealPlume] in MM's sort order, the RealPlume-Stock/MissingHistory/nuclearEngineKandl.cfg patch adds a PLUME node named Hydrogen-NTR-HighTemp. (The nuclearEngineBKN.cfg patch does the same sort of thing, but names the node Hydrogen-NTR instead.)
  • At :AFTER[RealPlume], the RealPlume/000_Generic_Plumes/000_EFFECTS_Remover.cfg patch removes the entire EFFECTS node from all engines that have a PLUME node (as added above). This is where the running_closed effect gets lost.
  • At :AFTER[zRealPlume], the RealPlume/000_Generic_Plumes/Hydrogen-NTR-HighTemp.cfg patch adds a whole new EFFECTS node based on configuration in the PLUME[Hydrogen-NTR-HighTemp] node. (And Hydrogen-NTR.cfg does the same sort of thing for the PLUME[Hydrogen-NTR].)

It's not clear why RealPlume removes the entire EFFECTS block — as long as it adds the new effect nodes, and patches the engine modules to use them, there shouldn't be any harm in leaving the old, unused effects in the database too. But that's what it does, and changing it would be a significant design change in RealPlume that could introduce other bugs in other parts, so that's probably not a solution here.

There are few potential ways to fix this, with varying degrees of awkwardness. A few options that come to mind:

  • Change the LH2NTR patches from :BEFORE[zzLH2NTR] to :BEFORE[RealPlume]. Elegant, but making the patches run earlier could potentially introduce other bugs related to interaction with other patches. Would need testing.
  • Have a :BEFORE[RealPlume] patch that copies the running_closed block to somewhere outside of EFFECTS, and then have the main patch move (copy and then delete) it from there instead, to sidestep RealPlume's deletion of the EFFECTS. (Note that a :BEFORE[RealPlume] patch will work even if RealPlume isn't installed.) This would avoid the exception, but it'd also mean that the engine's LH2 mode would use the original, non-RealPlume effects.
  • Have separate fx-sc-core:NEEDS[RealPlume] and fx-sc-core:NEEDS[!RealPlume] blocks, with different paths for the effect to copy from. This seems a little more awkward, but might be preferable since both engine modes will get the RealPlume effects.

BTW, the reason for copying the running_closed effect to fx-sc-core in the first place is that KSP doesn't support sharing the same effect between several engine modules; only one of them plays the effect. That used to be a bug in KerbalAtomics, and I added this effect-copying trick in merge #44 (specifically, commit 6f8d28b) to fix it.

That was back in KSP 1.3, though. There's a chance that the KSP limitation has been fixed in the meantime, and that engine modules can share effects now. It'd be worth checking on that: if we could use running_closed for both engine modules, there'd be no need to copy it as fx-sc-core at all, and both would automatically pick up RealPlume's change correctly. (Some of the other LH2NTR patches use the same copying trick with different effect names; the same goes for those.)

I checked and found that KSP 1.7.3 still doesn't correctly handle multiple engine modules sharing the same effect. That rules out my first option listed above: if the LH2NTR patch runs before RealPlume, RealPlume will see both the LF and LH2 engine modules, and will change both to use the same effect.

I tested the third option, copying from RealPlume's effect name instead of running_closed when applicable, and it works well; both engine modes get the new effect. I want to investigate whether any of the other LH2NTR patches (besides the MissingHistory one) need similar changes, and then I'll submit a PR.

In latest release version.