Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception Errors for MH engines with RealPlume in KA LH2NTR patch #74

Closed
GHrynk opened this issue Jun 9, 2019 · 3 comments
Closed

Exception Errors for MH engines with RealPlume in KA LH2NTR patch #74

GHrynk opened this issue Jun 9, 2019 · 3 comments

Comments

@GHrynk
Copy link

GHrynk commented Jun 9, 2019

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

@Wyzard256
Copy link
Contributor

Wyzard256 commented Jul 22, 2019

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.)

@Wyzard256
Copy link
Contributor

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.

@ChrisAdderley
Copy link
Collaborator

In latest release version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants