Kerbalism incompatible with NEOS

[mwerle]

• KSP 1.5.1
• NEOS 0.8.0 (plus dependencies - ModuleManager 3.1.0, KIS 1.14)
• Kerbalism 2.0.0.0

One of my users reported an issue with Keralism and NEOS installed on this forum post.

I've created a bug in mwerle/OrbitalMaterialScience#34 about this and attached a set of save-games which can reproduce the issue.

Basisally for at least one sub-set of experiments, the experiments are KIS objects which must be returned (with science data) to Kerbin in order for Contracts to complete. I believe Kerbalism intercepts the science data when it is generated and stores it in its own way.

I'm not sure what the best way to fix this would be; having some way for Kerbalism to exempt certain experiments from its own special handling would be ideal from my perspective. The [X]-Science mod does something similar so perhaps some of its code could be leveraged?

[mwerle]

Still an issue with Kerbalism 2.1.2.

As this has been raised several times, any chance of a comment, @steamp0rt ?

[ghost]

Kerbalism is completely revamping science experiments in 2.2. This'll be fun, not.

[mwerle]

Hi @steamp0rt, thanks for the reply.

So as far as this issue is concerned, wait until then to review?

[ghost]

Yes.

[SirMortimer]

Basisally for at least one sub-set of experiments, the experiments are KIS objects which must be returned (with science data) to Kerbin in order for Contracts to complete. I believe Kerbalism intercepts the science data when it is generated and stores it in its own way.

I'm not sure that I'm understanding the issue. Is this about a contract not being fulfilled? What is it that the contract requires to finish?

[mwerle]

What is it that the contract requires to finish?

The KEES code searches through a Vessel for KIS containers when the Vessel is Recovered, then inside the KIS containers it looks for the KEES-specific Experiment part which fulfills the KEES Contract. Only Experiment Parts which contain the "finalized" experiment data can complete a Contract.

My understanding is that Kerbalism extracts the experiment data from the parts when the experiment is "finalized" and stores it elsewhere in the Vessel. Thus the KEES Contract code can't find the experiment data and the Contract remains unfulfilled.

EDIT: And yes, I could recode KEES so that it iterates the entire Vessel just looking for the matching KEES ScienceData, but the point of Nehemiah's experiments was that you have to return the actual physical experiments, not just the data.

[SirMortimer]

Kerbalism has 2 types of science: files (like thermometer readings) and samples (goo, surface samples, particle dust, ...). Samples have to be analyzed in a lab or recovered on Kerbin. With the coming update, they also will have mass, a surface sample f.i. is 12kg, goo less than a kilo. What we don't have (yet?) is an option for mandatory recovery of a sample so that it is impossible to analyze in a lab but must be recovered. So, if it is not the part itself that needs to be recovered (maybe to sell it on Ebay...) that might be worth a reconsideration.

Are the KEES parts that need to be recovered supposed to have a ModuleScienceContainer in them that contains the science data?

Because what I'm assuming is happening is this: when the experiment finishes, you get that "KSP science dialog" and klick on "keep". It is at that point that Kerbalism snatches the ScienceData from whereever it came from and puts it into its own storage. We remove all ModuleScienceContainers, from all parts.

[mwerle]

Because what I'm assuming is happening is this: when the experiment finishes, you get that "KSP science dialog" and klick on "keep".

Yep. The data is stored inside a Module "KEESExperiment" which eventually implements ModuleScienceContainer. I haven't figured out a way yet to interact with / get events from the "KSP Science Dialog". Causes hassles if the player uses "Reset", for example, which just breaks the experiment (loses the existing data but not possible to re-run either).
It's perfectly fine for the player to remove the science data from the Part (again, Part can't be reset afterwards) and return it separately, but then it won't complete any KEES Contracts.

But as I said, the point of Nehemiahs experiments (I didn't create the mod, I'm just maintaining it) is to retrieve the actual physical experiments for processing back on Kerbin. In the case of KEES they are actual physical parts which the player manipulates using KIS, for Kemini and the larger Orbital Experiments they are never directly manipulated by the player; they are installed and removed using menu items.

Maybe we can figure something out (eg, Kerbalism "swallows" the KEES parts and KEES then looks for the swallowed parts on return?) or we'll just have to mark the mods as being incompatible.

It'd take a major re-write of NEOS to change the way the experiments are managed (which would impact not just the code but also the save-files) for which I don't really have the time at the moment, although I've considered it in the past. That would be a major break from just maintaining this mod though; Nehemiah is still hoping to eventually come back to it.

[SirMortimer]

I just skimmed through the code. To me it looks like KEESExperiment implements ModuleScienceExperiment which is fine because it won't get yanked by the new Kerbalism Science system. In KEESExperiment there is a variable completed which is set by a Unity Coroutine when the experiment had been exposed for long enough. KEESExperimentRecovery will then test for completed in all parts of the recovered vessel. But at that point it isn't about science being on the part, it is just about having that value set or not.

I don't yet see how this is interrupted by Kerbalism.

EDIT: aaaaah my bad. It then looks for scienceData in that part. Okay.

[SirMortimer]

Now also looked at the saves in the .zip attached to your issue (great job to whoever did that, btw). With Kerbalism, you can't expect science data to be in any part within the vessel. We store it in a seperate data structure because we have to access that data when the vessel is unloaded for the upcoming background science and already existing background transmission.

What you can do right now:

If Kerbalism is installed, you hook into GameEvents.OnScienceReceived and set the contract to complete whenever you receive a corresponding ScienceData with one of your experiment IDs (like NE_KEES_PPMD@KerbinInSpaceLow). However, that will also trigger when the results are transmitted back instead of being recovered. To avoid that you could set xmitDataScalar = 0 for all NEOS science modules via config, but then it won't be possible to transmit partial results any more. It also will trigger if the sample is being analyzed in a lab, which will convert it to data that can be transmitted.

What you can do with a not yet existing Kerbalism API function that could easily be provided in the next release which won't be too far out now:

Register a callback with Kerbalism that will be called each time science is recorded, but includes the information wether it was recovered or transmitted, and if it was recovered includes the protoVessel it was recovered from. If you need the part, your part and that part only to fulfill the contract, you can look at the protoVessel. If you don't care about the part itself but just want to make sure that the player did return a physical sample that had been removed from your part in orbit, you can ignore the protoVessel. But you can't look for the data inside the part, it won't be there.

[mwerle]

Thank you very much for your detailed analysis and suggestions; certainly something to consider.

However I do have one issue with your proposals and that is that it is Kerbalism that is modifying global behaviour. Yet you are expecting other mods to accommodate it instead of the other way around.

I appreciate NEOS does non-standard Science processing, however, it does so only for its own Experiments; it does not impact any other mods. Personally I think the onus is on the mod modifying global behaviour to be accommodating, not the other way around.

Otherwise I'd have to code in special support for Kerbalism, for X-Science, and half a dozen other mods which all do "something" with Science. In the past, the authors of those other mods have implemented an exclusion list for non-stock Experiments (NEOS isn't the only mod which has non-stock science behaviour) or added special processing (for example DMagic's Orbital Science was popular enough to have warranted that, NEOS wasn't).

Having said that, I'll try to get onto Nehemiah to get his thoughts on this.

[SirMortimer]

However I do have one issue with your proposals and that is that it is Kerbalism that is modifying global behaviour. Yet you are expecting other mods to accommodate it instead of the other way around.

So does StageRecovery, yet you support it.

[mwerle]

So does StageRecovery, yet you support it.

Fair point; just found the commit. Although the code to do so was provided by another developer; presumably the user who wanted that support. Anyway, as I said, I'll consider it and ask Nehemiah his thoughts.

Just thinking about this more, alternatively I could make the contracts complete just based on the physical part irrespective of whether or not the Science Data was transmitted or returned in some other way; I'll just need to tag the part with the situation of where it was performed.

[SirMortimer]

I can send you a PR if that is of any help (however cloning your repo to the machine I usually do this on will take a while). And maybe there's yet another possibility: NEOS could look at the science through Kerbalism's eyes, there is an API function to search for files/samples on a vessel. But that wants a Vessel, not a ProtoVessel...

public static double SampleSize(Vessel v, string subject_id)

I'll poke around a bit and see what can be done.

Re blacklisting of other mods... I'm not sure yet what the implications would be, and as I'm currently reworking the science system in a game-changing way. Adding a blacklist option might just be the way to go. But only as a backstop, if all other attempts fail (something I've learned from recent politics, heh). First I'd try to support other mods if at all possible.
And BTW, we're about to break compatibility with x Science, science now and KEI, possibly others. Some of the functionality will be incorporated in Kerbalism, others will just vanish.

One already can turn off Kerbalism's science part completely, in that case nothing at all is affected (btw, that would be a workaround for your users, if they want to do it. Not ideal, but it works).

[mwerle]

Please hold off any further work / PR for the time being; I'm in the middle of reworking the KEES code to handle alarms properly. Sounds like we're better off waiting for the new Kerbalism API anyway.

[mwerle]

So I've just realised something - the KEES experiments set "dataIsCollectable" to false.

It's a core-game flag, not a NEOS extension. The flag doesn't get saved in the savegame, but it is available at runtime inside ModuleScienceExperiment.

So shouldn't Kerbalism honour this flag when "snatching" the ScienceData? This way neither Kerbalism nor NEOS would have to have mod-specific code.

Thoughts?

[SirMortimer]

That's interesting. I just looked at all the stock experiment, there is not a single one that sets dataIsCollectible to False. I didn't know that flag, and the KSP API documentation is as discreet as always. It probably is meant to indicate wether the data is supposed to be removable from the part. And I think Kerbalism should honor it.

It took me a while to get behind ShotgunNinja's ninjitsu, but I think I've got it right. I set the mysteryGoo experiment to non-collectible for my test, run the goo on the launchpad - lo and behold: Kerbalism leaves it alone, and the part retains the data. As far as I can tell, this should work for all experiments that set that flag, the NEES issue should be fixed with this.

BTW, as far as I know the saved game just contains the bits that are different from the configuration of a part, so I'm not surprised that dataIsCollectible isn't found there.

[mwerle]

I only just found it myself :)

The KEES experiments had it set for ages, from way back when Nehemiah worked on the mod, but I never really paid any attention to it, not really knowing what it's for. Thanks very much for considering and implementing!

[ghost]

should be solved with 3.0