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

EVA Kerbal science data and cargo parts recovery isn't correct in KSP 1.11+ #43

Closed
gotmachine opened this issue Jun 9, 2022 · 0 comments
Labels
kspBug Identified KSP issue

Comments

@gotmachine
Copy link
Contributor

gotmachine commented Jun 9, 2022

GetAllProtoPartsIncludingCargo() will exclude protoparts whose AvailablePart.name == "kerbalEVA".

This method was introduced in 1.11.0 with the stock inventory system. Prior to that, callers (mainly
Funding.onVesselRecoveryProcessing() and ResearchAndDevelopment.onVesselRecoveryProcessing())
were directly iterating on the protopart list without any filtering.

The reason this filter exists is because when recovering a part, the inventory of the part crew is
added to the list by calling GetAllProtoPartsFromCrew(), which is getting stored parts in the "hidden"
kerbal inventory modules. But in the case of an EVA kerbal, that inventory module is on the part, so
its cargo parts would be added twice to the resulting list.

The stock implementation is causing two bugs :

  • Since it exclude the kerbalEVA part from the list, onVesselRecoveryProcessing() callbacks aren't
    processing it, which results in science data and IPartCostModifier modules costs not being recovered.
  • Because it only check for the exact kerbalEVA AP name, it only filter that part and let all other
    EVA variants go through (kerbalEVAfemale, kerbalEVAFuture, etc). So for all variants, cargo parts are
    recovered in double quantity, and science data / cost modifiers are recovered.

The kerbalEVA filter was added in 1.12.0, very likely in an attempt to fix the initial "EVA inventories are recovered twice" 1.11.0 issue (but no mention of that in the changelog). But this fix doesn't entirely fix the original issue and introduce new ones.

To properly fix this :

  • We undo the kerbalEVA protopart filter from GetAllProtoPartsIncludingCargo() introduced in 1.12.0, so those protoparts
    are returned (consistent with pre-1.12 behavior for all parts, and EVA variants behavior post-1.12)
  • We don't call GetAllProtoPartsFromCrew() to append the cargo parts if the protopart is an EVA Kerbal
@gotmachine gotmachine added the kspBug Identified KSP issue label Jun 9, 2022
gotmachine added a commit that referenced this issue Jun 9, 2022
gotmachine added a commit that referenced this issue Jun 9, 2022
…e attached to a vessel (external seat, Klaw...)
gotmachine added a commit that referenced this issue Jan 30, 2023
gotmachine added a commit that referenced this issue Jan 30, 2023
…e attached to a vessel (external seat, Klaw...)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kspBug Identified KSP issue
Development

No branches or pull requests

1 participant