pellinor0 / TweakScale

Forked from Gaius Goodspeed's Goodspeed Aerospace Part & TweakScale plugin
29 stars 23 forks source link

Root part reverting, and the "camera bug" #3

Closed pellinor0 closed 8 years ago

pellinor0 commented 9 years ago

These things seem to have a common cause, so I'll list them as one issue.

This is most likely a KSP bug, related to differences in the loading process between launch, quickload and revert. On load/quickload and revert to launch, scale changes in the transform of the root part are ignored/overwritten by KSP. Setting the change flag for this transform triggers the camera bug.

These are the code lines that trigger the bug (from Scale.cs):

part.transform.GetChild(0).localScale = newScale; part.transform.GetChild(0).hasChanged = true; part.transform.hasChanged = true;

  • The reverting affects only the visual model. All properties, resources and attachments are unaffected.
  • There is no corruption. If the part ceases to be the root part (e.g. by docking vessels), it will get the correct scale when loaded the next time. The save file always lists the correct scale.
  • The bug probably appeared in KSP 0.25.
  • My guess is a problem in the order of execution. It seems like something in the global scane/camera infrastructure is not correctly set up when the part module code is called. Then TweakScale touches the transform, and then the infrastructure code gets called and assumes that nobody touched this transform yet.

Post about reproduction: http://forum.kerbalspaceprogram.com/threads/94844-View-zoom-out-on-camera-mode-change?p=1441712&viewfull=1#post1441712

pellinor0 commented 8 years ago

Found a workaround for the camera breaking (don't try to scale the model of the root part on revert/load since this will fail anyway).

The distinction is a bit hacky (vessel=null on launch, always set on load/revert), but it works for now.

pellinor0 commented 8 years ago

Maybe @NathanKell could help with this (since he fixed the rescaleFactor): what is the correct way to scale the model of an individual part when loading the ship in the flight scene? And when should I do it?

Some more things I found out:

Any other ideas?

pellinor0 commented 8 years ago

Another idea: in the working case (Launch) the partModule seems to get called before the vessel is fully initialized (part.vessel==null, also part.parent==null). Since the transform of the root part seems to play a special role for the vessel, would it help to attach some code to the vessel instead of the part, which can act at that earlier time?

pellinor0 commented 8 years ago

Related issues on the squad bug tracker: http://bugs.kerbalspaceprogram.com/issues/3364 (camera bug) http://bugs.kerbalspaceprogram.com/issues/9444 (root part does not scale)

NathanKell commented 8 years ago

Try using .part.partTransform rather than .transform - does that help?

NathanKell commented 8 years ago

And yes, you want the child transform named 'model' to scale. Note that rescaleFactor etc are used when PartLoader loads (i.e. compiles) a part, not afterwards, so they're not relevant here.

NathanKell commented 8 years ago

Ok, did a quick test.

public class Scaler : PartModule
{
    public override void OnLoad(ConfigNode node)
    {
        base.OnLoad(node);
        if (part.partInfo != null && part.partInfo.partPrefab != null)
        {
            Transform source = null;
            foreach (Transform t in part.partInfo.partPrefab.transform)
            {
                if (t.name == "model")
                {
                    source = t;
                    break;
                }
            }
            if (source != null)
            {
                foreach (Transform t in part.partTransform.transform)
                {
                    if (t.name == "model")
                    {
                        t.localScale = source.localScale * 2f;
                    }
                }
            }
        }
    }
}

Works perfectly when added to a root part, survives revert with no issues or exceptions as far as I can tell.

pellinor0 commented 8 years ago

Looks like that was the missing puzzle piece!

I logged the name of the part.transform.GetChild(0), and in the error case it is "main camera pivot". So this both symptoms, the missing rescale and messing up the camera. So the difference is that the root part has an additional child in this transform, which messes up a hardcoded assumption in TweakScale.

pellinor0 commented 8 years ago

And the fix works. Many thanks, I would not have found this without your help.

Your code looks like a part is allowed to have more than one "model" transform, is that the case? Because TweakScale also assumes that there is only one transform to scale.

NathanKell commented 8 years ago

Anything loaded from a .mu will get all the stuff in the mu childed to a "model" gameobject. My guess is AssetBundle stuff will work the same, but I don't know about it.

You're most welcome!

pellinor0 commented 8 years ago

Fixed in version v2.2.7.2