DeepCopyIn sets some values to default when used with copy mask containing a partial copy mask

[Benna96]

Mutagen version: 0.42
Game: Skyrim SE, probably affects everything though

Was experimenting with Translation Masks and DeepCopyIn, and encountered an issue.

The relevant part of my mask was

ArmorAddon.TranslationMask myMask = new(false)
{
    BodyTemplate = new(false)
    {
        FirstPersonFlags = true
    }
}

The record's BodyTemplate.ArmorType was Clothing, but DeepCopyIn set it to LightArmor, which is the default value for that field.

Searching around source code, I found the code that handles this. It does essentially the following:

if ((copyMask?.GetShouldTranslate((int)ArmorAddon_FieldIndex.BodyTemplate) ?? true))
{
    if(rhs.BodyTemplate is {} rhsBodyTemplate)
    {
        item.BodyTemplate = rhsBodyTemplate.DeepCopy(
            errorMask: errorMask,
            copyMask?.GetSubCrystal((int)ArmorAddon_FieldIndex.BodyTemplate));
    }
    else
    {
        item.BodyTemplate = default;
    }
}

DeepCopy returns a default object, with the masked values applied to it. By setting bodyTemplate to that, any unmasked values are set to default, rather than leaving them alone as the documentation for TranslationMasks would suggest.

I think this should be changed to use DeepCopyIn, something like this:

if ((copyMask?.GetShouldTranslate((int)ArmorAddon_FieldIndex.BodyTemplate) ?? true))
{
    if(rhs.BodyTemplate is {} rhsBodyTemplate)
    {
        if (item.BodyTemplate is not null)
        {
            item.BodyTemplate.DeepCopyIn(
                rhsBodyTemplate,
                errorMask: errorMask,
                copyMask?.GetSubCrystal((int)ArmorAddon_FieldIndex.BodyTemplate));
        }
        else
        {
            item.BodyTemplate = rhsBodyTemplate.DeepCopy(
                errorMask: errorMask,
                copyMask?.GetSubCrystal((int)ArmorAddon_FieldIndex.BodyTemplate));
        }
    }
    else
    {
        item.BodyTemplate = default;
    }
}

Since this is generated code, I imagine the issue affects a ton of record types.

[Noggog]

Hey! Thanks a ton for getting into the gritty details and asking questions.

I'm hoping it's just a miscommunication of API. The section of interest in the docs is the "onOverall" parameter: https://mutagen-modding.github.io/Mutagen/plugins/Translation-Masks/#onoverall-parameter

Specifying onOverall to false will make the GetShouldTranslate return false, and thus leave BodyTemplate as it is

The documentation goes over why there is both default and onOverall and hopefully the difference between the two. Give that a spin, and let me know if it works for you, and if so, if there's any tweaks to the docs you think we could make.

[Benna96]

Apologies if I wasn't clear. What I want to do in this case is copy in just FirstPersonFlags.

Perhaps a picture would help. Consider the following setup: On the left is the original record from Skyrim.esm.
In the middle is a mod that changes FirstPersonFlags.
On the right is a mod that changes ArmorType.

This is the result I want:

If I set the BodyTemplate mask's onOverall to false, BodyTemplate is left as is, as expected. This is not what I want though.

Setting onOverall to true does cause the FirstPersonFlags to be copied... but reverts all other fields to their default values, since internally, it's setting the field using DeepCopy, which in turn uses GetNew() and DeepCopyIn(), meaning whatever values were there before are not taken into account. In this case, general flags happened to already be the default, but armor type got changed to something neither of the considered mods had it as.

With the proposed change, using DeepCopyIn instead of DeepCopy, I get the result I want.
(That said, I'm not sure if the null handling of the proposed change is desired. The docs only address DeepCopy, not DeepCopyIn, when it comes to nullability.)