TweakScale

TweakScale

1M Downloads

Misplacing parts when chain scaling clones.

Lisias opened this issue · 12 comments

commented

Fellow Kerbonaut Krazy1 found another one.

In a nutshell:

  1. drop Mk2 long fuel tank for root. Surface attach same tank to top.
    image

  2. rotate it 90 deg.
    image

  3. translate it up and center on the root part so they appear to be stacked vertically.
    image

  4. Critical step: use Alt-LMB to copy the top tank and node-attach it on the back.
    image

  5. chain scale the root to 1.875 m. The last part is offset vertically.
    image

  6. Note if you did not Alt-LMB copy at "critical step" but instead took a new part from the library, then this does not happen... there is no offset, it looks correct.
    image

Notes: Sounds like something TS needs to do about KSP-Recall, perhaps?

commented

NOPE!!! :D

Today I did another thingy that I should had done way before: I tested 1.3.1 with TweakScale 2.3.7, the last one compiled for 1.3.1 - and I could reproduce the problem the same!

It's something happening since forever, this use case never worked at all.

Note to myself: not everything is my fault, after all! I do things right now and then!! :P

Anyway, I was really chasing my tail on this one. It's something that TweakScale never did right, or it's something bugged on KSP since 1.3.1 (perhaps the OnCopy handling).

I think it's time to give KSP 1.2.2 a try just for the lulz. :)

commented

Saving and loading the craft file doesn't "fix" the problem. It's persistent.

commented

I reproduced the problem on KSP 1.4.3. Exactly the same problem.

Same TweakScale, same Recall.

This is "good" news, it means that almost surely it's something I'm doing wrong, something I'm not doing right, or a new and consistent misbehaviour that will be fixed for everybody at the same time.

commented

Attaching the part into another part also doesn't "fix" the problem. The part itself appears to be tainted.

screenshot97

commented

FIxing the part's position after scaling up, and then chain-scaling everything down again also presents the misbehaviour, but "inverted" - the displacement is now upwards.

screenshot98

Knowing the TweakScale code as I know, this suggests that the calculation of the attach node offset of the affected part is being "contaminated" somehow when the part is a clone. Such "contamination" doesn't happen on new parts using prefab data.

This appears to rule out TweakScale's scaling code from the problem. [edit] Nope… See below…

I don't think this is a missing use-case on Recall itself… Or it is? Perhaps a race condition, since both TweakScale and Recall handles the OnCopy event? [edit] There was a missing use case on Recall, but it is unrelated to this problem

commented

Oukey. KSP-Recall is missing an use case, but it's unrelated to this problem: by implementing the use-case, I found no change on the misbehaviour.

Well… Then I did something that I should had already done at first place: checked KSP 1.3.1. And the problem is there too.

That's settle the matter: I messed up something on the TweakScale code for sure. KSP 1.3.1 has none of the problems I'm fighting since forever on Editor.

commented

I'm considering, right now, that perhaps TweakScale should "normalize" the parts on attachment of cloned parts, so they would mimic the attributes of a new part from the Palette. This would avoid the need to mangle on a already stablished code (almost a decade old), not to mention making it easier to handle these parts for everybody.

I don't think I should do it on Recall, however. This is not a bug on Editor, and since it affects 1.3.x (and I'm betting everything downto the first KSP release to allow cloning parts), it would demand spreading Recall to these versions (down to 1.2.2 at least), something I'm pretty reticent on doing - I wanted to retire Recall once KSP's bugs would be fixed (how naive I was), not to spread the damned thing into everything as a pandemic.

commented

Instrumenting the PartModule.OnCopy and TweakScale.OnEditorAttach callbacks, I got:

For a cloned part:

[LOG 13:45:21.162] [TweakScale] TRACE: orgPos, attPos, attPos0 (0.0, 0.0, -1.4) (0.0, 0.0, 0.6) (0.0, 0.0, -2.0)
[LOG 13:45:21.162] [TweakScale] TRACE: orgRot, attRotation, attRotation0 (0.0, 0.0, 0.0, -1.0) (0.0, 0.7, 0.0, 0.7) (0.0, 0.0, 0.0, -1.0)

For a new part:

[LOG 13:45:24.659] [TweakScale] TRACE: orgPos, attPos, attPos0 (0.0, 0.0, -1.4) (0.0, 0.0, 0.6) (0.0, -3.8, 0.0)
[LOG 13:45:24.659] [TweakScale] TRACE: orgRot, attRotation, attRotation0 (0.0, 0.0, 0.0, -1.0) (0.0, 0.0, 0.0, 1.0) (0.0, 0.0, 0.0, -1.0)

In the end, it's all about a missing use case on TweakScale - it assumes that every new attached part will get the default, prefab values for these attribute, clearly a mistake.

The fix is straightforward.

commented

I'm an idiot. The diagnostic was under my nose all the time: if saving and loading the craft doesn't fixes the problem, all I need to do is to create a new similar craft without the problem (i.e., getting the 3rd MK2 tank from the Palette instead of cloning it) and compare them with KDIFF!!!

With the thing happening on 1.3.1, KDIFF will easily spot the differences.

commented

Here, the KDIFF between the two sample crafts:

Screen Shot 2023-07-15 at 04 02 47

The attpos appears to be the problem.

Craft files (both): Untitled Space Crafts.zip

Now I need to understand why the attpos is being screwed when cloned. There's a chance we found yet a new bug on KSP's Editor…[edit: nope, just a use case that no one thought of before]

commented

This is what I'm thinking it's happening:

  1. When we clone a part that was displaced from the "ideal" attachment point, the new attPos0 is calculated from the current attPos, instead of the ideal one specified on the prefab.

  2. When we take a new part from the Palette, the attPos is fetched directly from the prefab, and so the attPos0 is calculated in a more orthodox manner.

  3. The scaling code is getting confused because it uses the prefab as reference when recalculating positions, and gets screwed because the cloned part is using another position.

commented

In a way or another, it's a minor flaw. I will let this as is for while and priorize new features on the Companion series, then I'll be back to this.