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

Module indexing mismatch on config changes #14

Closed
gotmachine opened this issue Oct 31, 2021 · 1 comment
Closed

Module indexing mismatch on config changes #14

gotmachine opened this issue Oct 31, 2021 · 1 comment
Labels
kspBug Identified KSP issue

Comments

@gotmachine
Copy link
Contributor

gotmachine commented Oct 31, 2021

When partmodules are added/removed in a part config following a mod installation/removal/update, the index of the persisted protomodules in existing saves won't match the prefab module index anymore. Note that this also apply to loading a ShipConstruct.

This mean that next time a changed part will be loaded from it persisted protopart, the protomodule list isn't matching the prefab module list. KSP has some provision to handle that. However, it fundamentally can't handle the case of multiple same type modules being present in a part.

The "reconnection" code is implemented in Part.LoadModule(ConfigNode node, ref int moduleIndex).
Before attempting to load a protomodule node, it check if :

  • moduleIndex is within the range of the part modules count
  • the module type at moduleIndex match

So this will be triggered for all modules whose index is greater than the index of the module that was added/removed from the part config.

If the conditions is true, it will attempt to find the right module by iterating on all the part modules, and will then load the node in the first found whose type match. This mean that if multiple modules of the same type exists, all protomodule nodes will be loaded in the first occurence, and other occurences will be reset to the prefab configuration.

There is no way to totally resolve that issue without requiring that all "multiple occurence" modules in a part config define a per-part unique identifier that would be propagated to the protopart. However, the stock "reconnection" logic could be improved to massively reduce the probability of an issue.

Instead of blindly applying the config to the first module whose type match, it could count how many modules of the same type exists in each module list (the saved and instance ones), and load the nth node into the nth module of the same type. This wouldn't work in case the config change is removing or adding a module that exist/existed in multiple occurences, but at least this will cover all cases of another module type being responsible for the indexing mismatch.

A further refinement to cover that last case could be to Save() the part modules, and to compare the resulting ConfigNodes to the saved ones to determine the index of the added/removed module. Wouldn't be 100% fail-proof, but would further reduce a mismatch probability.

The difficulty for implementing this is that we don't have access to the saved module nodes list from Part.LoadModule(ConfigNode node, ref int moduleIndex), so the logic would have to be moved further up the call stack, in
ShipConstruct.LoadShip() and ProtoPartModuleSnapshot.ConfigurePart().

Moreover, modules in a ShipConstruct part node aren't grouped in a subnode, they are put side by side with all other part nodes (resources, actions, etc...), which make inserting/removing module nodes a bit awkward.

@gotmachine
Copy link
Contributor Author

Fix + API added in 1.3

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