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.

Sign up for GitHub

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

Add WithAllParentMasters binary write builder option #547

Merged
merged 2 commits into from
Sep 7, 2024
Merged

Add WithAllParentMasters binary write builder option #547

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
265e368
Add WithAllParentMasters
Elscrux Aug 29, 2024
3cc9d14
Remove WithConstructionKitRequiredMasters
Elscrux Aug 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 40 additions & 13 deletions Mutagen.Bethesda.Core/Plugins/Binary/Translations/BinaryWriteBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using System.IO.Abstractions;
using DynamicData;
using Mutagen.Bethesda.Installs.DI;
using Mutagen.Bethesda.Plugins.Binary.Parameters;
using Mutagen.Bethesda.Plugins.Binary.Streams;
using Mutagen.Bethesda.Plugins.Exceptions;
using Mutagen.Bethesda.Plugins.Meta;
using Mutagen.Bethesda.Plugins.Order;
using Mutagen.Bethesda.Plugins.Records;
Expand Down Expand Up @@ -790,11 +792,12 @@ IFileBinaryModdedWriteBuilder WithMastersListOrdering(
IFileBinaryModdedWriteBuilder WithExplicitOverridingMasterList(IEnumerable<ModKey> modKeys);

/// <summary>
/// The Construction Kit complains when loading mods that do not have base masters listed. <br />
/// This call includes base masters even if they are not needed by the mod's content
/// The Creation Kit complains when loading mods without all transitive masters listed. <br />
/// This call makes sure to include all transitive masters, even if they are not needed by the mod's content
/// to avoid issues when loading the plugin in the Creation Kit.
/// </summary>
/// <returns>Builder object to continue customization</returns>
IFileBinaryModdedWriteBuilder WithConstructionKitRequiredMasters();
IFileBinaryModdedWriteBuilder WithAllParentMasters(ILoadOrderGetter<IModListing<IModGetter>> loadOrder);

/// <summary>
/// Executes the instructions to write the mod.
Expand Down Expand Up @@ -1375,18 +1378,42 @@ public FileBinaryModdedWriteBuilder<TModGetter> WithExplicitOverridingMasterList
}
IFileBinaryModdedWriteBuilder IFileBinaryModdedWriteBuilder.WithExplicitOverridingMasterList(IEnumerable<ModKey> modKeys) => WithExplicitOverridingMasterList(modKeys);

/// <summary>
/// The CK complains when loading mods that do not have base masters listed. <br />
/// This call includes base masters even if they are not needed by the mod's content
/// </summary>
/// <returns>Builder object to continue customization</returns>
public FileBinaryModdedWriteBuilder<TModGetter> WithConstructionKitRequiredMasters()
public IFileBinaryModdedWriteBuilder WithAllParentMasters(ILoadOrderGetter<IModListing<IModGetter>> loadOrder)
{
var implicits = Implicits.Get(this._mod.GameRelease);
return this.WithExtraIncludedMasters(implicits.BaseMasters);
// Collect all transitive masters
var masters = new HashSet<ModKey>();
var remainingMasters = new Queue<IMasterReferenceGetter>(_mod.MasterReferences);
while (remainingMasters.Count > 0)
{
var master = remainingMasters.Dequeue();
masters.Add(master.Master);
loadOrder.AssertListsMod(master.Master);

var masterMod = loadOrder.ListedOrder.First(mod => mod.ModKey == master.Master).Mod ?? throw new MissingModException(master.Master);
foreach (var parent in masterMod.MasterReferences)
{
remainingMasters.Enqueue(parent);
masters.Add(parent.Master);
}
}

// Ensure the masters are in the same order as the load order
var modKeyOrder = loadOrder.ListedOrder.Select(listing => listing.ModKey);
var sortedMasters = masters.OrderBy(m => modKeyOrder.IndexOf(m));

return this with
{
_params = _params with
{
_param = _params._param with
{
ExtraIncludeMasters = _params._param.ExtraIncludeMasters.EmptyIfNull().And(sortedMasters).Distinct().ToArray()
}
}
Comment on lines +1384 to +1412
Copy link
Member

@Noggog Noggog Aug 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we refactor this logic to an internal component that takes in the LO and returns the IEnumerable<ModKey>?

Once it's its own component, it'll be easier to test correctness, too, etc.

Main thing is that this builder call shouldn't be the one specifying the LO as an input parameter. The LO is set via other builder calls. Since not all the info is known at the time of THIS call, the logic written here should be run "lazily" at the end once all the bits are set and the Write is actually called

To get the "laziness", a system like the _loadOrderSetter will need to be added like a _mastersContentSetter. When called during the "pull together" phase, it would pull in the LO set right before, and then use that to calculate the masters. I can do this part if you want, as it's pretty fiddly and error prone

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at the _loadOrderSetter before, but the load order it has just has a limited IModFlags interface or so, which didn't allow master references to be looked at. I didn't want to edit the existing builder code so I went this way for now.

Copy link
Member

@Noggog Noggog Sep 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True true. hmm

Well. I think the idea is that load order is mainly used for ordering. Whereas the Data folder is used for looking up info about mods. So the presence of ILoadOrderGetter<IModFlagsGetter> loadOrder is maybe deceiving?

I think someone like yourself using WithAllParentMasters() would:

  • not supply anything in the WithAllParentMasters() call itself
  • Would include WithDataFolder(...) to let the system know where to read mods from to find all the parent masters
  • Would include WithLoadOrder(...) to let the system know how to order all the parent masters

Maybe as a followup, we could remove the ILoadOrderGetter<IModFlagsGetter> loadOrder, but I would want to deep dive on it to make sure that's a good cut.

};
}
IFileBinaryModdedWriteBuilder IFileBinaryModdedWriteBuilder.WithConstructionKitRequiredMasters() => WithConstructionKitRequiredMasters();
IFileBinaryModdedWriteBuilder IFileBinaryModdedWriteBuilder.WithAllParentMasters(ILoadOrderGetter<IModListing<IModGetter>> loadOrder) => WithAllParentMasters(loadOrder);

/// <summary>
/// Executes the instructions to write the mod.
/// </summary>
Expand Down
Loading