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

Scaling Part with Variants that change attachment nodes is not working as expecnted #139

Closed
Lisias opened this issue Sep 1, 2020 · 12 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Lisias
Copy link

Lisias commented Sep 1, 2020

That's the history:

Implementing the scaling for Variants with Mass and Cost was easy, but while doing it I detect a old bug (mysteriously unreported - or it's selective memory from my part?).

You can scale Parts with Variants that change nodes as long you apply the variant first. But if you scale the part later, the attachment nodes are not scaled appropriately, They are scaled, but not the right way.

You need to select the default scaling, change the variant, then scale again to force this crap into behaving.

This is not even a regression, the scaling code never worked as intended.

Changing Variant, then scaling

  1. screenshot1
  2. screenshot2
  3. screenshot3

Scaling, then changing Variant.

  1. screenshot1
  2. screenshot4
  3. screenshot5
@Lisias Lisias added the bug Something isn't working label Sep 1, 2020
@Lisias Lisias self-assigned this Sep 1, 2020
@Lisias
Copy link
Author

Lisias commented Sep 1, 2020

Interestingly.... The inherited code that supposedly would handle the situation apparently does nothing. I bluntly commented out the thing, and the scaling behavior didn't changed at all.

But on a first glance, the code should work.

@7ranceaddic7
Copy link

It looks like a node (but which, top? bottom?) is getting reset.

@Lisias
Copy link
Author

Lisias commented Sep 1, 2020

Behaviour consistent on KSP 1.10.1, KSP 1.7.3 and KSP 1.4.4 (the first to support Mass and Cost on Variants).

So, yeah. That code never worked at all. :P

@Lisias
Copy link
Author

Lisias commented Sep 1, 2020

It looks like a node (but which, top? bottom?) is getting reset.

All of them. The Rhino engine also suffer this problem, but on the top node (the tube changes the bottom node).

Now that I know that there's no code to be fixed (as that code never worked at all), I can focus on write new code. As soon as I understand where the variant nodes to be scaled are.

@7ranceaddic7
Copy link

Well, it looks as though one of the nodes is not affected, but the other loses it's "tweaked" position value and is reset to it's original part position value. I'd bet that has something to do with one of two things: (1) the ordering in the config file, i.e. the first defined node is unaffected or (2) which node you attach to first.

@Lisias
Copy link
Author

Lisias commented Sep 1, 2020

I had considered this, so I spend some time trying to debug the Variant specific code that was scaling these nodes.

What I realized is that the Variant nodes are being completely disregarded - the "base" ones are being scaled instead.

The tubes' variants change only the bottom stack attach point, and so only the things attached to it are misplaced.

The Rhino's variant change the top stack attach point, and so only the things attached to it are misplaced.

But since when the variant is applied before scaling things work as expected, the problem is not the scaling code itself. I'm missing something to be applied/used/called when the Variants are applied... humm.. or perhaps when it is changed? I may be hooking the wrong handler....

Let's see what I get on the next time window for it.

@Lisias
Copy link
Author

Lisias commented Sep 5, 2020

I gave up this issue for release 2.4.4.0 - this bug is there for ages now, this can wait a couple releases. :)

@Lisias
Copy link
Author

Lisias commented Dec 28, 2020

Just a convenient place to answer a question on the forum.

Well, the engine plates are parts with Variants that change Attachment Nodes, so they will work almost fine - as long you don't change the variant after scaling, or you descale it to the default size before changing variant (and then scale it again). Yeah, kinda a pain in the ass - but it's a viable workaround until I fix the symmetry I broke when I fixed the attachment points on the current Beta.

screenshot8

@Lisias Lisias added this to the 2.4.5.3 milestone Jul 23, 2021
Lisias added a commit that referenced this issue Oct 24, 2021
@Lisias
Copy link
Author

Lisias commented Oct 24, 2021

Interesting enough, commit 2965f4c also fixed this one!

In essence, #163 is a DUPLICATE of this one! :)

@Lisias
Copy link
Author

Lisias commented Oct 24, 2021

Reopening and pushing back to 2.4.6.3, as this is not critical and I want to ship the critical fixes (#175 in special).

The commit 9783c4b reopened this issue. Caution to prevent reopening #175 must be taken while fixing this one.

@Lisias Lisias reopened this Oct 24, 2021
Lisias added a commit that referenced this issue Oct 24, 2021
@Lisias
Copy link
Author

Lisias commented Nov 20, 2021

Being embarrassing enough, I'm postponing this again to 2.4.6.4, as I need to rush 2.4.6.3 to publish the fix for some borks of mine ASAP.

@Lisias Lisias modified the milestones: 2.4.6.3, 2.4.6.4 Nov 20, 2021
@Lisias
Copy link
Author

Lisias commented Nov 20, 2021

Still more embarrasing, is that I just realised this issue is fixed!!! :D

The fix for #167 also fixed this one, so I'm moving it back to 2.4.6.3.

I'm also marking it as a duplicated for #167 (besides #167 being created later) as I already fixed that one mentioning it on the commit df0fef2 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants