Skip to content

Commit

Permalink
Stop Mutagen environment from loading too early.
Browse files Browse the repository at this point in the history
This regressed in a previous version and was never detected. It was the reason why the app seemed to be taking longer to get to the first pixel, but spent no time at all creating the load order analyzer. The `SettingsViewModel` was depending on the `IGameSettings` which in turn caused the environment to be created. Effectively, this completely ignored any customizations being made to the plugin selections at startup.

It's too valuable to have global access to these types to consider moving them all into providers and factories and such, and doesn't really help that much anyway. Using `Lazy<T>` can alleviate the issue somewhat by deferring access to until the load order list is needed - but unfortunately this isn't quite good enough because starting the app in first-time mode will trigger this right away.

Instead, we add a "future"-like type, that's a little more intelligent than `Lazy<T>` and uses that under the hood, but requires a bit more wire-up and post-activation logic from the IoC container. Other types can now accept an `IAfterSetup<T>`, which will never actually cause the value to be resolved, instead will throw if the dependency has yet to be created - essentially a "weak reference" version of the Lazy type, with a callback version that is designed to work with property-change notifications.

Fixes #113
  • Loading branch information
focustense committed Sep 5, 2021
1 parent f5674a9 commit 1bd05e0
Show file tree
Hide file tree
Showing 9 changed files with 200 additions and 5 deletions.
12 changes: 8 additions & 4 deletions Focus.Apps.EasyNpc/Configuration/SettingsViewModel.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Focus.Apps.EasyNpc.Build;
using Focus.Apps.EasyNpc.Messages;
using Focus.Environment;
using Focus.ModManagers;
using Ookii.Dialogs.Wpf;
using PropertyChanged;
Expand All @@ -22,7 +23,7 @@ public class SettingsViewModel : INotifyPropertyChanged

public IEnumerable<string> AvailableModNames => modRepository.Select(x => x.Name);
public IEnumerable<string> AvailableMugshotModNames { get; private set; } = Enumerable.Empty<string>();
public IEnumerable<string> AvailablePlugins => gameSettings.PluginLoadOrder.OrderBy(p => p);
public IEnumerable<string> AvailablePlugins { get; private set; } = Enumerable.Empty<string>();
public ObservableCollection<BuildWarningSuppressionViewModel> BuildWarningWhitelist { get; init; }
public bool HasDefaultModRootDirectory { get; private init; }
[DependsOn(nameof(UseModManagerForModDirectory))]
Expand Down Expand Up @@ -69,17 +70,15 @@ public bool UseModManagerForModDirectory
}
}

private readonly IGameSettings gameSettings;
private readonly IMessageBus messageBus;
private readonly IModManagerConfiguration modManagerConfig;
private readonly IModRepository modRepository;
private readonly IMutableAppSettings settings;

public SettingsViewModel(
IMutableAppSettings settings, IModManagerConfiguration modManagerConfig, IModRepository modRepository,
IGameSettings gameSettings, IMessageBus messageBus)
IAfterGameSetup<IGameSettings> gameSettingsFuture, IMessageBus messageBus)
{
this.gameSettings = gameSettings;
this.messageBus = messageBus;
this.modManagerConfig = modManagerConfig;
this.modRepository = modRepository;
Expand All @@ -99,6 +98,11 @@ public SettingsViewModel(
.Select(x => new MugshotRedirectViewModel(x.ModName, x.Mugshots));
MugshotRedirects = new(mugshotRedirects);
WatchCollection(MugshotRedirects, SaveMugshotRedirects);

gameSettingsFuture.OnValue(gameSettings =>
{
AvailablePlugins = gameSettings.PluginLoadOrder.OrderBy(p => p).ToList().AsReadOnly();
});
}

public void AckWelcome()
Expand Down
1 change: 1 addition & 0 deletions Focus.Apps.EasyNpc/Main/LoaderModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public LoaderTasks Complete()
log.Information("Load order confirmed");
if (modRepositoryConfigureTask is null)
throw new InvalidOperationException("Mod repository has not been configured");
setup.Confirm();
var modRepositoryTask = modRepositoryConfigureTask.ContinueWith(_ => modRepository as IModRepository);
var loadOrderAnalysisTask = AnalyzeLoadOrder();
var profileTask = Task.WhenAll(loadOrderAnalysisTask, modRepositoryTask)
Expand Down
35 changes: 34 additions & 1 deletion Focus.Apps.EasyNpc/Modules/MutagenModule.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Autofac;
using Autofac.Core;
using Focus.Analysis.Execution;
using Focus.Analysis.Records;
using Focus.Apps.EasyNpc.Build.Pipeline;
Expand All @@ -12,6 +13,7 @@
using Mutagen.Bethesda.Skyrim;
using System;
using System.Collections.Generic;
using System.Linq;

namespace Focus.Apps.EasyNpc.Modules
{
Expand Down Expand Up @@ -42,19 +44,50 @@ protected override void Load(ContainerBuilder builder)
builder.Register(ctx => ctx.Resolve<IEnvironmentFactory>().CreateEnvironment())
.As<GameEnvironmentState<ISkyrimMod, ISkyrimModGetter>>()
.As<IGameEnvironmentState<ISkyrimMod, ISkyrimModGetter>>()
.OnActivated(NotifyAfterGameSetup)
.SingleInstance();
// Autofac doesn't know how to narrow open generics. Boo.
builder.RegisterType<GameEnvironmentWrapper<ISkyrimMod, ISkyrimModGetter>>()
.As<IMutableGameEnvironment<ISkyrimMod, ISkyrimModGetter>>()
.As<IReadOnlyGameEnvironment<ISkyrimModGetter>>()
.OnActivated(NotifyAfterGameSetup)
.SingleInstance();
builder.RegisterType<GameSettings<ISkyrimModGetter>>()
.As<IGameSettings>()
.OnActivated(NotifyAfterGameSetup)
.SingleInstance();
builder.RegisterType<GameSettings<ISkyrimModGetter>>().As<IGameSettings>().SingleInstance();
builder.RegisterType<DummyPluginBuilder>().As<IDummyPluginBuilder>();

// Futures
RegisterAfterGameSetup<GameEnvironmentState<ISkyrimMod, ISkyrimModGetter>>(builder);
RegisterAfterGameSetup<IGameEnvironmentState<ISkyrimMod, ISkyrimModGetter>>(builder);
RegisterAfterGameSetup<IMutableGameEnvironment<ISkyrimMod, ISkyrimModGetter>>(builder);
RegisterAfterGameSetup<IReadOnlyGameEnvironment<ISkyrimModGetter>>(builder);
RegisterAfterGameSetup<IGameSettings>(builder);

// Analysis types
builder.RegisterType<GroupCache>().As<IGroupCache>();
builder.RegisterType<RecordScanner>().As<IRecordScanner>();
builder.RegisterType<MutagenLoadOrderAnalyzer>().As<ILoadOrderAnalyzer>();
}

private static void NotifyAfterGameSetup<T>(IActivatedEventArgs<T> args)
{
var notifyTypes = typeof(T).GetInterfaces()
.Prepend(typeof(T))
.Distinct()
.Select(t => typeof(AfterGameSetup<>).MakeGenericType(t));
foreach (var notifyType in notifyTypes)
if (args.Context.TryResolve(notifyType, out var notifier))
((IAfterGameSetupNotifier)notifier).NotifyValue();
}

private static void RegisterAfterGameSetup<T>(ContainerBuilder builder)
{
builder.RegisterType<AfterGameSetup<T>>()
.AsSelf() // Needed for notifications
.As<IAfterGameSetup<T>>()
.SingleInstance();
}
}
}
82 changes: 82 additions & 0 deletions Focus.Environment.Tests/AfterGameSetupTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
using Moq;
using System;
using System.Collections.Generic;
using Xunit;

namespace Focus.Environment.Tests
{
public class AfterGameSetupTests
{
private readonly AfterGameSetup<string> afterGameSetup;
private readonly Mock<IGameSetup> gameSetupMock;
private readonly Lazy<string> lazy;

public AfterGameSetupTests()
{
gameSetupMock = new Mock<IGameSetup>();
lazy = new(() => "Dummy Value");
afterGameSetup = new AfterGameSetup<string>(gameSetupMock.Object, lazy);
}

[Fact]
public void WhenValueAccessedBeforeSetupConfirmed_Throws()
{
Assert.Throws<InvalidOperationException>(() => afterGameSetup.Value);
}

[Fact]
public void WhenValueAccessedAfterSetupConfirmed_Resolves()
{
gameSetupMock.SetupGet(x => x.IsConfirmed).Returns(true);

Assert.Equal("Dummy Value", afterGameSetup.Value);
}

[Fact]
public void WhenCallbacksRegistered_DoesNotInvokeBeforeNotify()
{
string cbResult1 = null;
string cbResult2 = null;
afterGameSetup.OnValue(x => cbResult1 = x + "1");
afterGameSetup.OnValue(x => cbResult2 = x + "2");

Assert.Null(cbResult1);
Assert.Null(cbResult2);
}

[Fact]
public void WhenCallbacksRegistered_InvokesAfterExplicitNotify()
{
string cbResult1 = null;
string cbResult2 = null;
afterGameSetup.OnValue(x => cbResult1 = x + "1");
afterGameSetup.OnValue(x => cbResult2 = x + "2");
afterGameSetup.NotifyValue();

Assert.Equal("Dummy Value1", cbResult1);
Assert.Equal("Dummy Value2", cbResult2);
}

[Fact]
public void WhenNotifiedMultipleTimes_InvokesCallbacksOnlyOnce()
{
var cbResults = new List<string>();
afterGameSetup.OnValue(x => cbResults.Add(x));
afterGameSetup.NotifyValue();
afterGameSetup.NotifyValue();
afterGameSetup.NotifyValue();

Assert.Collection(cbResults, x => Assert.Equal("Dummy Value", x));
}

[Fact]
public void WhenCallbacksRegisteredAfterAlreadyNotified_InvokesImmediately()
{
string cbResult = null;
afterGameSetup.NotifyValue();
afterGameSetup.OnValue(x => cbResult = x);

Assert.Equal("Dummy Value", cbResult);
}
}
}
1 change: 1 addition & 0 deletions Focus.Environment.Tests/Focus.Environment.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.7.1" />
<PackageReference Include="Moq" Version="4.16.1" />
<PackageReference Include="xunit" Version="2.4.1" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
Expand Down
61 changes: 61 additions & 0 deletions Focus.Environment/AfterGameSetup.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
using System;
using System.Collections.Generic;

namespace Focus.Environment
{
public interface IAfterGameSetup<T>
{
bool IsReady { get; }
T Value { get; }

void OnValue(Action<T> callback);
}

public interface IAfterGameSetupNotifier
{
void NotifyValue();
}

public class AfterGameSetup<T> : IAfterGameSetup<T>, IAfterGameSetupNotifier
{
public bool IsReady => setup.IsConfirmed;
public T Value => ValueOrThrow();

private readonly Lazy<T> lazy;
private readonly IGameSetup setup;
private readonly List<Action<T>> valueCallbacks = new();

private bool isNotified;

public AfterGameSetup(IGameSetup setup, Lazy<T> lazy)
{
this.lazy = lazy;
this.setup = setup;
}

public void NotifyValue()
{
if (isNotified)
return;
foreach (var cb in valueCallbacks)
cb(lazy.Value);
valueCallbacks.Clear();
isNotified = true;
}

public void OnValue(Action<T> callback)
{
if (isNotified)
callback(lazy.Value);
else
valueCallbacks.Add(callback);
}

private T ValueOrThrow()
{
if (!setup.IsConfirmed)
throw new InvalidOperationException("Value cannot be accessed until game setup is confirmed.");
return lazy.Value;
}
}
}
3 changes: 3 additions & 0 deletions Focus.Environment/IGameSetup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ public interface IGameSetup
{
IReadOnlyList<PluginInfo> AvailablePlugins { get; }
string DataDirectory { get; }
bool IsConfirmed { get; }
ILoadOrderGraph LoadOrderGraph { get; }

void Confirm();
}

public static class GameSetupExtensions
Expand Down
4 changes: 4 additions & 0 deletions Focus.Providers.Mutagen/EnvironmentFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using Mutagen.Bethesda.Plugins;
using Mutagen.Bethesda.Plugins.Order;
using Mutagen.Bethesda.Skyrim;
using System;
using System.Linq;

namespace Focus.Providers.Mutagen
Expand All @@ -28,6 +29,9 @@ public EnvironmentFactory(IGameSetup setup, GameSelection game)

public GameEnvironmentState<ISkyrimMod, ISkyrimModGetter> CreateEnvironment()
{
if (!setup.IsConfirmed)
throw new InvalidOperationException(
"Attempted to create the game environment before settings were confirmed.");
var loadOrderKeys = setup.AvailablePlugins
.Where(p => setup.LoadOrderGraph.IsEnabled(p.FileName) && setup.LoadOrderGraph.CanLoad(p.FileName))
.Select(p => ModKey.FromNameAndExtension(p.FileName));
Expand Down
6 changes: 6 additions & 0 deletions Focus.Providers.Mutagen/GameSetup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public class GameSetup : IGameSetup
{
public IReadOnlyList<PluginInfo> AvailablePlugins { get; private set; } = new List<PluginInfo>().AsReadOnly();
public string DataDirectory => game.DataDirectory;
public bool IsConfirmed { get; private set; }
public ILoadOrderGraph LoadOrderGraph { get; private set; } = new NullLoadOrderGraph();

private readonly IFileSystem fs;
Expand All @@ -28,6 +29,11 @@ public GameSetup(IFileSystem fs, ISetupStatics setupStatics, GameInstance game,
this.setupStatics = setupStatics;
}

public void Confirm()
{
IsConfirmed = true;
}

public void Detect(IReadOnlySet<string> blacklistedPluginNames)
{
var implicits = setupStatics.GetBaseMasters(game.GameRelease)
Expand Down

0 comments on commit 1bd05e0

Please sign in to comment.