Bounds calculation should consider the default or active variant given by a part config for parts with varying sizes

[JadeOfMaar]

Concerning a conversation with @JonnyOThan and AlphaMensae on the subject of parts with mesh switch configs and various sizes it was highlighted that KSP doesn't adjust the bounds of a part depending on its variant and the related change of size. KSP always assumes the maximum size and this is reflected in the Engineer Report in the ship editors. Parts that animate do not suffer this issue so maybe a cue can be taken from how those are handled.

I figured and confirmed that this issue means that parts with variants will not agree with the cargo shielding module if any variant exceeds a given cargo bay's shielding radius. In this test craft a ruler set to its shortest size but still produces drag. Its greatest size is also shown for reference.

To reproduce this issue, place a part with varying sizes in a cargo bay, set it to its smallest that can fit (this part must have a size that's clearly longer than the cargo bay is), launch it, turn on aero info in PAWs, get the craft moving and watch the drag value in the part's PAW.

Required mods:

• Measurizer
• B9 Part Switch

Craft file

[gotmachine]

Gave a quick look at this, and first for clarity, this is a problem introduced by mesh switcher plugins, things are fine in a stock environment.

Part bounds used in the engineer report and for cargo bay occlusion are obtained by a call to PartGeometryUtil.GetPartRendererBounds(), which indeed doesn't ignore disabled renderers. It does however handle the exclusion of specific renderers by populating the Part.partRendererBoundsIgnore list with GameObject names. It also has a specific handling for the stock variant switcher. Note that this method is also used for a few flight scene code paths, notably the Vessel.size field update.

Ideally, non stock mesh switchers could be populating Part.partRendererBoundsIgnore to handle that issue, but maintaining that list when multiple sources are involved could be a bit messy, and realistically this isn't going to be implemented/fixed in every mesh switcher anytime soon.

The KSPCF solution would be to re-implement PartGeometryUtil.GetPartRendererBounds() (re-implementing the whole thing, we shouldn't touch the utility methods it is calling) so it ignore disabled renderers by default.

Given that this is used in many code paths exposing the results in various public ways, it could potentially have some unwanted side effects, but in this specific case I feel this is relatively safe to do, and would definitely fix many issues where non-stock switchers are causing incorrect bounds to be used.

Edit : Patching PartGeometryUtil.GetPartRendererBounds() to ignore disabled renderers would actually introduce a quite breaking change : the method would return nothing when called on a part prefab, since the whole gameobject is disabled in that case. In addition to some stock code paths, there are multiple plugins doing that at the moment (JanitorsCloset, NeatherdyneMassDriver, Sandcastle...). There might also be some issues with stock part construction calling the method on disabled parts. So not so great of an idea...

I guess the best KSPCF can do here would be to implement a modding API allowing mesh switchers to easily specify a list a of disabled GameObjects/Transforms. Calling a specific method signature through reflection feels like a viable option here in terms of performance (we could cache an open delegate and a type list), and would avoid the need of a middleman redistributed assembly. Could be something like List<Transform> GetDisabledTransformsForPartBounds(), to be implemented as a PartModule instance method.

But this introduce a (soft) dependency on KSPCF and would still require work on the switchers side, as well as introducing a performance hit as KSPCF would have to iterate over every PartModule on every call to PartGeometryUtil.GetPartRendererBounds(), so in the end, implementing direct manipulation of Part.partRendererBoundsIgnore from those plugins would be my preference...

[gotmachine]

So... actually, after thinking about it, it might be possible to directly patch PartGeometryUtil.GetPartRendererBounds() to solve the issue, by traversing the model hierarchy and checking if any gameobject is disabled (checking activeSelf), excluding it and its childs. Since prefab are disabled by disabling their top-level GameObject, this shouldn't affect them.

Also, for reference : blowfishpro/B9PartSwitch#245

[gotmachine]

Patch seems to be working, very little testing done.

[gotmachine]

Released in 1.35.0