Patching the correct nodes definition for the procedural parts.

[Lisias]

Fixing the issue https://github.com/KSP-RO/ProceduralParts/issues/315

( that ended up causing the problem on https://github.com/net-lisias-ksp/KSP-Recall/issues/41 )

This patch set was extensively tested, documentation on :

https://github.com/net-lisias-ksp/KSP-Recall/issues/41#issuecomment-1103055074

[DRVeyl]

A PR targeting the part.cfg attachment node positions will not be merged.

ProcParts enables other mods to change the default shape, specified e.g. https://github.com/KSP-RO/ProceduralParts/blob/master/GameData/ProceduralParts/Parts/Tanks/1TankLiquid.cfg#L50 It also permits changing the default parameters of any shape [whether it is the default shape or not]. See https://github.com/KSP-RO/RP-0/blob/master/GameData/RP-0/Tree/ProceduralAvionics.cfg#L76 as an example that exhibited the problem originally and was identified in the RO Discord.

I do not consider it reasonable to expect other mods changing these configurations -- either the default shape, or the default parameters of any shape -- to also need to set the part level node positions in the ConfigNode that will make it to the GameDatabase / MM cache. The node positions are handled properly in code in ProcParts regardless of the values in the part-level config. Unless some 3rd party mod chooses to interfere and breaks the functioning, expressly intended workflow.

I do consider https://github.com/net-lisias-ksp/KSP-Recall/commit/5d3056e78b85d2ba6fa462002ead237010d01bfe a valid fix. Perhaps ensuring that enforcement in both ProcParts and KSP-Recall is a good idea.

[Lisias]

You didn't understood. It's the procedural parts who's deviating.

On every part I checked (but PP, of course), the PartModule.OnLoad has access to the correct nodes definition of the part being instantiated.

On PP, the nodes definition doesn't matches the nodes that will effectively be used when the part is instantiated on the World on OnLoad, this happens only later. So anyone in need to have access to the corrects values at OnLoad will not get it from PP (but will get it from any other behaving Part on the game).

The commit https://github.com/net-lisias-ksp/KSP-Recall/commit/5d3056e78b85d2ba6fa462002ead237010d01bfe does not fixes the problem, it only sweeps it under the rug. I'm just washing my hands, and letting someone else to take the heat if the problem happens again.

[Lisias]

Yes, I think it's really it.

https://github.com/KSP-RO/ProceduralParts/blob/952ed1ae3e17bb299777f9cf6a78f1937f62d5e3/Source/ProceduralHeatshield.cs#L86

At least for the proceduralHeatshield, PP only updates the attachment nodes on the OnStart - so during the whole OnLoad process and until proceduralHeatshield's OnStar is called, everybody else is reading bogus values from the part.

I didn't found even a KSPEvent to announce interested parts about the changes on the nodes, so there's no practical way to be notified about the part's changing attachment nodes other than pooling it all the time - please correct me if I'm wrong.

On a side note, you listen the PartAttachNodeSizeChanged so if anyone else announces changes on the nodes, you are notified.

https://github.com/KSP-RO/ProceduralParts/blob/952ed1ae3e17bb299777f9cf6a78f1937f62d5e3/Source/ProceduralHeatshield.cs#L116

[DRVeyl]

https://github.com/KSP-RO/ProceduralParts/blob/master/Source/ProceduralAbstractShape.cs#L457

Entirely possible there are flaws in the procedural heatshield, which is probably one of the least tested parts except perhaps for the procedural SRB. (The SRB having such laughable flaws as documented in the current GitHub Issues and with a pending PR fixing them.)

[Lisias]

This specific flaw, explained above, is shared by all Procedural PartModules.

[DRVeyl]

About the attachment node positions not valid in OnLoad? WontFix. I see no reason to make that a design requirement.

As for notification about node position changes, those are done in the line I referenced.

[Lisias]

About the attachment node positions not valid in OnLoad? WontFix. I see no reason to make that a design requirement.

Understood. So I'm closing this as CLOSED/WONTFIX and I will not bother you about again.

Cheers.