NRE engaging Ascent Autopilot via RasterPropMonitor or MOARdV's Avionics Systems
MOARdV opened this issue ยท 7 comments
RPM and MAS use the following technique to control the MJ ascent autopilot from IVA (all relevant interactions are through delegates created from reflection, and I throw exceptions myself if any of the MJ modules are null):
- Get the 'users' field for MechJebModuleAscentAutopilot.
- Query the AscentAutopilot.enabled property.
- If 'true', remove MechJebModuleAscentGuidance from AscentAutopilot.users (turn off AAP).
- If 'false', add AscentGuidance to AscentAutopilot.users (engage AAP).
In KSP 1.3.1 and earlier, this approach successfully enabled and disabled the ascent autopilot.
In KSP 1.4.x with MJ 2.7.3, when I engage the AAP, AscentAutopilot.enabled returns 'true', but the throttle doesn't ramp to 100%, and I get the following log spam:
[ERR 10:48:54.985] MechJeb module MechJebModuleAscentAutopilot threw an exception in Drive: System.NullReferenceException: Object reference not set to an instance of an object
at MuMech.MechJebModuleAscentAutopilot.DriveAscent (.FlightCtrlState s) [0x00000] in <filename unknown>:0
at MuMech.MechJebModuleAscentAutopilot.Drive (.FlightCtrlState s) [0x00000] in <filename unknown>:0
at MuMech.MechJebCore.Drive (.FlightCtrlState s) [0x00000] in <filename unknown>:0
I thought maybe I needed to set AAP.enabled = true before adding AscentGuidance as a user, but even that doesn't resolve the issue.
If I open the AAP GUI and close it again (or leave it open), then I don't get the NREs in the log, and the MJ AAP engages and functions normally.
Can you give me some guidance on what else MAS needs to do in order to successfully activate the ascent autopilot without the player needing to open and close the MJ GUI? I know the AAP had some substantial changes made between 1.3.1 and 1.4.0, but I haven't followed the MJ code changes closely enough to pin down what I need to do.
I think #1100 fixes this, although I probably just broke all the hacks to work around it, but now the AscentAutopilot is more fully responsible for its own state and wiring, and the AscentGuidance class should only have UI responsibilities. I think you should be able to go back to the old code.
I've confirmed with MJ official release 2.8.2 that the old code does indeed work, so I'll go ahead and close this issue. Thank you.
@lamont-granquist - thanks for the info. It looks like that was the answer I needed. The fix isn't too horrible - query the ascentPathIdx, enable / disable the profile modules as appropriate, and assign the active module to the AP's ascentPath. Just a few lines of code.
The Ascent Guidance code you highlighted is getting/setting fields from Ascent Autopilot and the MJ core, so maybe when this code gets refactored, it could be consolidated into something like public void AscentAutopilot.ConfigureProfile()
?
Yeah, something like that. I'd need to meditate on what the responsibilities of the different modules should really be. I think the AscentAutopilot / AscentGuidance split makes more sense to me now as a Controller-vs-View split though, and I sort of imagine it looks like that kind of API... Kinda knew it was bad code when I wrote it but I didn't have any better sense of what it should look like at the time...
presumably ascentPath is null. yeah that's set via ascentPathIdx and that's setup via some horrible code in AscentGuidance:
https://github.com/MuMech/MechJeb2/blob/dev/MechJeb2/MechJebModuleAscentGuidance.cs#L26-L80
that's likely the reason that opening the GUI and closing it fixes the problem because that code runs.
IDK exactly what to suggest as a solution offhand since i had no idea AscentAutopilot was being used standalone as a public API. the tight coupling that i wrote across all the modules here really does suck though and i'm not remotely proud of that code... its certainly a bug but its not a one-liner to fix it.
and because its a bug and needs fixing, whatever you do to work around it will probably break when it gets fixed right in the future... you can probably do the minimal amount to act like AscentGuidance and wire up the ascentPathIdx with the appropriate value, and ascentPath with the appropriate object.
I'm thinking the public API for this should just be writing to autopilot.ascentPathIdx
does the correct wiring of the subclasses.
most of the per-ascent-style menus are going to go away, but that could continue to be the responsibility of AscentGuidance (keeping the GUI windowing functions as separate concerns). then you could just interact with AscentAutopilot which would pick AscentClassic/AscentPEG/AscentGT as appropriate based on the PathIdx.