From 6a100ef6e72e9c5e96e0e3bd832c6c68385e88bf Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 10 Apr 2026 16:52:19 +0200 Subject: [PATCH] refactor(app): harden shutdown per final review Addresses final code review of phase-1 branch (Important I-1, I-3): - Move plugin Enable() loop inside the same try block as GameWindow.Run, and wrap each Enable() in per-plugin try/catch mirroring the Disable loop. Previously, a plugin Enable() throwing would skip the finally block entirely: plugins that had already enabled would never get disabled, Serilog would never flush, and the exception would escape ungracefully. Now Enable failures are logged and contained, and shutdown always runs. - Add a comment at the Get call in GameWindow.OnLoad explaining why TryGet was avoided (the [MaybeNullWhen(false)] nullable-generic analysis trips TreatWarningsAsErrors). I-2 (camera aspect doesn't update on window resize) deferred to Phase 2. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/AcDream.App/Program.cs | 9 ++++++--- src/AcDream.App/Rendering/GameWindow.cs | 5 ++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/AcDream.App/Program.cs b/src/AcDream.App/Program.cs index c876cf1..10b3a6b 100644 --- a/src/AcDream.App/Program.cs +++ b/src/AcDream.App/Program.cs @@ -40,11 +40,14 @@ foreach (var result in PluginDiscovery.Scan(pluginsDir)) Log.Information("loaded plugin {Id} ({DisplayName})", result.Manifest!.Id, result.Manifest.DisplayName); } -foreach (var plugin in loaded) - plugin.Plugin!.Enable(); - try { + foreach (var plugin in loaded) + { + try { plugin.Plugin!.Enable(); } + catch (Exception ex) { Log.Error(ex, "plugin enable failed: {Id}", plugin.Manifest.Id); } + } + using var window = new GameWindow(datDir); window.Run(); } diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 1c395c9..59a4bb2 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -91,7 +91,10 @@ public sealed class GameWindow : IDisposable _dats = new DatCollection(_datDir, DatAccessType.Read); // Find ANY landblock ending in 0xFFFF. Holtburg 0xA9B4FFFF is a - // good default; fall back to the first one we find. + // good default; fall back to the first one we find. Using Get + // (returns null on miss) rather than TryGet to sidestep + // [MaybeNullWhen(false)] nullable-generic analysis under + // TreatWarningsAsErrors. uint landblockId = 0xA9B4FFFFu; var block = _dats.Get(landblockId); if (block is null)