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

Add support for uncrewed vessels (e.g. ProbeControlRoom) #331

Open
sovetskysoyuz opened this issue Jan 31, 2021 · 6 comments
Open

Add support for uncrewed vessels (e.g. ProbeControlRoom) #331

sovetskysoyuz opened this issue Jan 31, 2021 · 6 comments
Assignees

Comments

@sovetskysoyuz
Copy link

sovetskysoyuz commented Jan 31, 2021

Several important parts of MASFlightComputer and MASVesselComputer only run during the FixedUpdate if vesselCrewed is true. This blocks compatibility with the Probe Control Room IVA, which is inherently used on uncrewed vessels.

Beyond the big FixedUpdate blocks, there are a few other functions (onVesselChange, onVesselWasModified, onVesselDestroy, onVesselCreate, onVesselCrewWasModified) that also only run on crewed vessels.

Other parts of MAS may assume that a vessel is crewed and throw exceptions for an uncrewed vessel - I already found that FindCurrentKerbal() in MASFlightComputer doesn't like it if there are no kerbals to find, and anything else that calls IVACameraActiveKerbal without checking if the vessel is uncrewed would have the same issue.

Recompiling the current MAS source-code with the crew-count checks removed, and some protection on IVACameraActiveKerbal calls to avoid NREs, produces a plugin that seems to work fine with the current version of Probe Control Room on KSP 1.11.1. I haven't tried to make these changes conditional on whether PCR is installed, since I always play with it and don't mind having these changes be permanent for my own installation, but such a condition would remove any performance impact for MAS users who don't use Probe Control Room.

Edit: a few other patches that I've made on my local copy - FYI since some of these took a while for me to tease out:

  • The functions in MASComponentAudioPlayer that use audioSource.mute should mute if the currentCameraMode is neither IVA nor Internal (since Probe Control Room uses Internal, not IVA, camera). This allows props to play sounds like alarm tones in Probe Control Room without being muted.
  • For an uncrewed vessel, the pod's referenceTransform is RemoteCommand, not Self; UpdateDockingNode should check for both of these when deciding whether to set shipFlightNumber = referencePart.launchID. This change allows MAS props to properly set the own-vessel reference part (since otherwise none of the conditions in UpdateDockingNode will be satisfied, because IVA camera mode isn't active).
  • MAS encounters problems when a new Probe Control Room vessel is created in-flight (e.g. through undocking or decoupling). The new vessel's first call to UpdateAttitude() fails with a NRE, followed by a bunch of other NREs, and this results in the MAS IVA initializing incompletely. From what I can figure out, the issue is related to PCR's StartIVA() method using SetReferencePart() to set a placeholder part that it spawns as the reference part, but it creates the part as private. This causes MAS to choke when it looks up the reference part for the new vessel. I am able to eliminate the NRE by adding a call to UpdateReferenceTransform() at the beginning of UpdateAttitude, and modifying UpdateReferenceTransform() so that it sets the root part as the reference part if it encounters a null reference part. There's probably a more efficient way to do this.
@MOARdV MOARdV self-assigned this Feb 1, 2021
@StoneBlue
Copy link
Contributor

StoneBlue commented Feb 2, 2021

I have a Probe Control Room IVA I am making, and also wanted to do a MAS version, so I'm up against the same wall as sovetskysoyuz. So I would also like to see MAS be able to be used in PCR w/crewed craft.
I know its extra work for you MOARdV, so no expectations.
I just wanted to voice that I would also make use of any updates to address the MAS-PCR issue.

Also, IIRC, @JonnyOThan has had a working version of a PCR .dll, which allows use of PCR on crewed as well as uncrewed crafts/IVAs with RPM, for quite awhile.
Idk if he might have insight into a method that could be easily adapted for MAS to also work with PCR and both crewed/uncrewed craft?

In any case, As Always, Thanx for even considering to look into the "issue".

@JonnyOThan
Copy link
Collaborator

It’s been a while since I looked at that code, and who knows if it’ll ever see a public release - so I wouldn’t put too much effort into making sure MAS was compatible.

But if it’s easy and you’re interested, all you really need to do is base all your logic off of whether the Internal camera is active, and remove any checks that require kerbals.

@baldamundo
Copy link

Is the Probe Control Room compatible version of the dll available for download anywhere?

@sovetskysoyuz
Copy link
Author

After a long delay, my changes discussed above are included in #368

@JonnyOThan
Copy link
Collaborator

I'ts worth noting that PCR now actually puts kerbals in the seats, so 🤷

@Sheepdog2142
Copy link

Why will the fix not get added. Probe control room is a really popular mod. To not support it on the main branch is crazy.

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

No branches or pull requests

6 participants