Mutagen-Modding / Mutagen

A .NET library for analyzing, creating, and manipulating Bethesda mods
GNU General Public License v3.0
121 stars 32 forks source link

Respect Master Sorting #477

Closed krypto5863 closed 3 months ago

krypto5863 commented 1 year ago

I used mutagen to load an existing mod and do edits directly to it and then save it back as follows:

var mod = SkyrimMod.CreateFromBinary(PathtoMod, SkyrimRelease.SkyrimSE);
var linkCache = mod.ToMutableLinkCache();

//generates a bunch of dialog topics and makes minor changes to some of the fields within existing topics.
DoWork(linkCache, mod,CSVDirectory);

//Uncompress needed for Mutagen.
foreach (var rec in mod.EnumerateMajorRecords())
{
    if (rec.IsCompressed)
    {
        Console.WriteLine($"Uncompressing record {rec.EditorID} :: {rec.FormKey}");
    }

    rec.IsCompressed = false;
}

mod.WriteToBinary(PathtoMod);

Console.ReadKey();

However, upon trying to load the product into the CK, I found it would crash. It turned out to be I needed to sort the masters in xEdit, even though they were already sorted going into this process.

Noggog commented 1 year ago

There are a decent amount of options for WriteToBinary, which takes in a BinaryWriteParameters object with the special instructions.

One of the more semi-required ones is providing the load order to sort the masters against.
https://mutagen-modding.github.io/Mutagen/plugins/Binary-Exporting/ (docs somewhat unfinished, haha)

By default, it populates the master list by the contents of the records, but sorts them first come first serve, which causes the problems you're experiencing. I could potentially not populate the master list by content, but then you run the risk of the user missing masters that exist, causing similar issues.

I think the best alternative would've been to design the API to require the load order as input by default, with somehow optionality to override it for other behavior. Not sure how that call would look, though

Let me know if you have any ideas, though!

EDIT: To be clear if you dont have a load order to pass it, and just want the masters untouched, there's options in the BinaryWriteParameters to instruct the system to do no changes to the masters

Noggog commented 1 year ago

I just updated the docs in Binary Exporting to show how to instruct the system to do no changes to masters. Reopened to consider changing the default behaviors to match that behavior.

I think that would be a big backward compatibility issue, though, as users that added the load order specification would inherit the default value for the MastersListContent being set to NoCheck, which would change their expected results. This kind of backwards incompatibility is not ideal, as it's silent. It compiles, but changes behavior.

Another option might be:

Will give it a spin!

krypto5863 commented 1 year ago

Yeah, I see your dilemma. Well, it's best to prevent compilation and make the error easily seen when devs open up their IDE. As such, I think doing non-optional parameters is the best way around it as it also forces devs to be aware of what is going on. However, I think an overload with no parameters would also be a very good solution but only in cases where there's an original master list to reference.

Hope my opinion helps. Thanks for looking into this! Appreciate it.

Noggog commented 3 months ago

This I believe is addressed in the new write builder concepts.

mod.BeginWrite
   .WithLoadOrder(...)
   ...OtherOptions
   .Write()

Better docs on the official wiki incoming