From 20041be27c3311b5ae0b2596a574c295e63ec1be Mon Sep 17 00:00:00 2001 From: goaaats Date: Mon, 17 Nov 2025 23:55:55 +0100 Subject: [PATCH 01/12] Convert ReliableFileStorage to async --- .../Storage/ReliableFileStorageTests.cs | 94 +++++++++---------- .../Internal/DalamudConfiguration.cs | 20 ++-- Dalamud/Configuration/PluginConfigurations.cs | 9 +- Dalamud/EntryPoint.cs | 3 +- .../Windows/Data/Widgets/VfsWidget.cs | 4 +- Dalamud/Storage/ReliableFileStorage.cs | 49 +++++----- 6 files changed, 94 insertions(+), 85 deletions(-) diff --git a/Dalamud.Test/Storage/ReliableFileStorageTests.cs b/Dalamud.Test/Storage/ReliableFileStorageTests.cs index ff56293e0..8a955e430 100644 --- a/Dalamud.Test/Storage/ReliableFileStorageTests.cs +++ b/Dalamud.Test/Storage/ReliableFileStorageTests.cs @@ -31,19 +31,19 @@ public class ReliableFileStorageTests .Select( i => Parallel.ForEachAsync( Enumerable.Range(1, 100), - (j, _) => + async (j, _) => { if (i % 2 == 0) { // ReSharper disable once AccessToDisposedClosure - rfs.Instance.WriteAllText(tempFile, j.ToString()); + await rfs.Instance.WriteAllTextAsync(tempFile, j.ToString()); } else if (i % 3 == 0) { try { // ReSharper disable once AccessToDisposedClosure - rfs.Instance.ReadAllText(tempFile); + await rfs.Instance.ReadAllTextAsync(tempFile); } catch (FileNotFoundException) { @@ -54,8 +54,6 @@ public class ReliableFileStorageTests { File.Delete(tempFile); } - - return ValueTask.CompletedTask; }))); } @@ -112,41 +110,41 @@ public class ReliableFileStorageTests } [Fact] - public void Exists_WhenFileInBackup_ReturnsTrue() + public async Task Exists_WhenFileInBackup_ReturnsTrue() { var tempFile = Path.Combine(CreateTempDir(), TestFileName); using var rfs = CreateRfs(); - rfs.Instance.WriteAllText(tempFile, TestFileContent1); + await rfs.Instance.WriteAllTextAsync(tempFile, TestFileContent1); File.Delete(tempFile); Assert.True(rfs.Instance.Exists(tempFile)); } [Fact] - public void Exists_WhenFileInBackup_WithDifferentContainerId_ReturnsFalse() + public async Task Exists_WhenFileInBackup_WithDifferentContainerId_ReturnsFalse() { var tempFile = Path.Combine(CreateTempDir(), TestFileName); using var rfs = CreateRfs(); - rfs.Instance.WriteAllText(tempFile, TestFileContent1); + await rfs.Instance.WriteAllTextAsync(tempFile, TestFileContent1); File.Delete(tempFile); Assert.False(rfs.Instance.Exists(tempFile, Guid.NewGuid())); } [Fact] - public void WriteAllText_ThrowsIfPathIsEmpty() + public async Task WriteAllText_ThrowsIfPathIsEmpty() { using var rfs = CreateRfs(); - Assert.Throws(() => rfs.Instance.WriteAllText("", TestFileContent1)); + await Assert.ThrowsAsync(async () => await rfs.Instance.WriteAllTextAsync("", TestFileContent1)); } [Fact] - public void WriteAllText_ThrowsIfPathIsNull() + public async Task WriteAllText_ThrowsIfPathIsNull() { using var rfs = CreateRfs(); - Assert.Throws(() => rfs.Instance.WriteAllText(null!, TestFileContent1)); + await Assert.ThrowsAsync(async () => await rfs.Instance.WriteAllTextAsync(null!, TestFileContent1)); } [Fact] @@ -155,26 +153,26 @@ public class ReliableFileStorageTests var tempFile = Path.Combine(CreateTempDir(), TestFileName); using var rfs = CreateRfs(); - rfs.Instance.WriteAllText(tempFile, TestFileContent1); + await rfs.Instance.WriteAllTextAsync(tempFile, TestFileContent1); Assert.True(File.Exists(tempFile)); - Assert.Equal(TestFileContent1, rfs.Instance.ReadAllText(tempFile, forceBackup: true)); + Assert.Equal(TestFileContent1, await rfs.Instance.ReadAllTextAsync(tempFile, forceBackup: true)); Assert.Equal(TestFileContent1, await File.ReadAllTextAsync(tempFile)); } [Fact] - public void WriteAllText_SeparatesContainers() + public async Task WriteAllText_SeparatesContainers() { var tempFile = Path.Combine(CreateTempDir(), TestFileName); var containerId = Guid.NewGuid(); using var rfs = CreateRfs(); - rfs.Instance.WriteAllText(tempFile, TestFileContent1); - rfs.Instance.WriteAllText(tempFile, TestFileContent2, containerId); + await rfs.Instance.WriteAllTextAsync(tempFile, TestFileContent1); + await rfs.Instance.WriteAllTextAsync(tempFile, TestFileContent2, containerId); File.Delete(tempFile); - Assert.Equal(TestFileContent1, rfs.Instance.ReadAllText(tempFile, forceBackup: true)); - Assert.Equal(TestFileContent2, rfs.Instance.ReadAllText(tempFile, forceBackup: true, containerId)); + Assert.Equal(TestFileContent1, await rfs.Instance.ReadAllTextAsync(tempFile, forceBackup: true)); + Assert.Equal(TestFileContent2, await rfs.Instance.ReadAllTextAsync(tempFile, forceBackup: true, containerId)); } [Fact] @@ -183,7 +181,7 @@ public class ReliableFileStorageTests var tempFile = Path.Combine(CreateTempDir(), TestFileName); using var rfs = CreateFailedRfs(); - rfs.Instance.WriteAllText(tempFile, TestFileContent1); + await rfs.Instance.WriteAllTextAsync(tempFile, TestFileContent1); Assert.True(File.Exists(tempFile)); Assert.Equal(TestFileContent1, await File.ReadAllTextAsync(tempFile)); @@ -195,38 +193,38 @@ public class ReliableFileStorageTests var tempFile = Path.Combine(CreateTempDir(), TestFileName); using var rfs = CreateRfs(); - rfs.Instance.WriteAllText(tempFile, TestFileContent1); - rfs.Instance.WriteAllText(tempFile, TestFileContent2); + await rfs.Instance.WriteAllTextAsync(tempFile, TestFileContent1); + await rfs.Instance.WriteAllTextAsync(tempFile, TestFileContent2); Assert.True(File.Exists(tempFile)); - Assert.Equal(TestFileContent2, rfs.Instance.ReadAllText(tempFile, forceBackup: true)); + Assert.Equal(TestFileContent2, await rfs.Instance.ReadAllTextAsync(tempFile, forceBackup: true)); Assert.Equal(TestFileContent2, await File.ReadAllTextAsync(tempFile)); } [Fact] - public void WriteAllText_SupportsNullContent() + public async Task WriteAllText_SupportsNullContent() { var tempFile = Path.Combine(CreateTempDir(), TestFileName); using var rfs = CreateRfs(); - rfs.Instance.WriteAllText(tempFile, null); + await rfs.Instance.WriteAllTextAsync(tempFile, null); Assert.True(File.Exists(tempFile)); - Assert.Equal("", rfs.Instance.ReadAllText(tempFile)); + Assert.Equal("", await rfs.Instance.ReadAllTextAsync(tempFile)); } [Fact] - public void ReadAllText_ThrowsIfPathIsEmpty() + public async Task ReadAllText_ThrowsIfPathIsEmpty() { using var rfs = CreateRfs(); - Assert.Throws(() => rfs.Instance.ReadAllText("")); + await Assert.ThrowsAsync(async () => await rfs.Instance.ReadAllTextAsync("")); } [Fact] - public void ReadAllText_ThrowsIfPathIsNull() + public async Task ReadAllText_ThrowsIfPathIsNull() { using var rfs = CreateRfs(); - Assert.Throws(() => rfs.Instance.ReadAllText(null!)); + await Assert.ThrowsAsync(async () => await rfs.Instance.ReadAllTextAsync(null!)); } [Fact] @@ -236,40 +234,40 @@ public class ReliableFileStorageTests await File.WriteAllTextAsync(tempFile, TestFileContent1); using var rfs = CreateRfs(); - Assert.Equal(TestFileContent1, rfs.Instance.ReadAllText(tempFile)); + Assert.Equal(TestFileContent1, await rfs.Instance.ReadAllTextAsync(tempFile)); } [Fact] - public void ReadAllText_WhenFileMissingWithBackup_ReturnsContent() + public async Task ReadAllText_WhenFileMissingWithBackup_ReturnsContent() { var tempFile = Path.Combine(CreateTempDir(), TestFileName); using var rfs = CreateRfs(); - rfs.Instance.WriteAllText(tempFile, TestFileContent1); + await rfs.Instance.WriteAllTextAsync(tempFile, TestFileContent1); File.Delete(tempFile); - Assert.Equal(TestFileContent1, rfs.Instance.ReadAllText(tempFile)); + Assert.Equal(TestFileContent1, await rfs.Instance.ReadAllTextAsync(tempFile)); } [Fact] - public void ReadAllText_WhenFileMissingWithBackup_ThrowsWithDifferentContainerId() + public async Task ReadAllText_WhenFileMissingWithBackup_ThrowsWithDifferentContainerId() { var tempFile = Path.Combine(CreateTempDir(), TestFileName); var containerId = Guid.NewGuid(); using var rfs = CreateRfs(); - rfs.Instance.WriteAllText(tempFile, TestFileContent1); + await rfs.Instance.WriteAllTextAsync(tempFile, TestFileContent1); File.Delete(tempFile); - Assert.Throws(() => rfs.Instance.ReadAllText(tempFile, containerId: containerId)); + await Assert.ThrowsAsync(async () => await rfs.Instance.ReadAllTextAsync(tempFile, containerId: containerId)); } [Fact] - public void ReadAllText_WhenFileMissing_ThrowsIfDbFailed() + public async Task ReadAllText_WhenFileMissing_ThrowsIfDbFailed() { var tempFile = Path.Combine(CreateTempDir(), TestFileName); using var rfs = CreateFailedRfs(); - Assert.Throws(() => rfs.Instance.ReadAllText(tempFile)); + await Assert.ThrowsAsync(async () => await rfs.Instance.ReadAllTextAsync(tempFile)); } [Fact] @@ -278,7 +276,7 @@ public class ReliableFileStorageTests var tempFile = Path.Combine(CreateTempDir(), TestFileName); await File.WriteAllTextAsync(tempFile, TestFileContent1); using var rfs = CreateRfs(); - rfs.Instance.ReadAllText(tempFile, text => Assert.Equal(TestFileContent1, text)); + await rfs.Instance.ReadAllTextAsync(tempFile, text => Assert.Equal(TestFileContent1, text)); } [Fact] @@ -290,7 +288,7 @@ public class ReliableFileStorageTests var readerCalledOnce = false; using var rfs = CreateRfs(); - Assert.Throws(() => rfs.Instance.ReadAllText(tempFile, Reader)); + await Assert.ThrowsAsync(async () => await rfs.Instance.ReadAllTextAsync(tempFile, Reader)); return; @@ -303,7 +301,7 @@ public class ReliableFileStorageTests } [Fact] - public void ReadAllText_WithReader_WhenReaderThrows_ReadsContentFromBackup() + public async Task ReadAllText_WithReader_WhenReaderThrows_ReadsContentFromBackup() { var tempFile = Path.Combine(CreateTempDir(), TestFileName); @@ -311,10 +309,10 @@ public class ReliableFileStorageTests var assertionCalled = false; using var rfs = CreateRfs(); - rfs.Instance.WriteAllText(tempFile, TestFileContent1); + await rfs.Instance.WriteAllTextAsync(tempFile, TestFileContent1); File.Delete(tempFile); - rfs.Instance.ReadAllText(tempFile, Reader); + await rfs.Instance.ReadAllTextAsync(tempFile, Reader); Assert.True(assertionCalled); return; @@ -335,17 +333,17 @@ public class ReliableFileStorageTests var tempFile = Path.Combine(CreateTempDir(), TestFileName); await File.WriteAllTextAsync(tempFile, TestFileContent1); using var rfs = CreateRfs(); - Assert.Throws(() => rfs.Instance.ReadAllText(tempFile, _ => throw new FileNotFoundException())); + await Assert.ThrowsAsync(async () => await rfs.Instance.ReadAllTextAsync(tempFile, _ => throw new FileNotFoundException())); } [Theory] [InlineData(true)] [InlineData(false)] - public void ReadAllText_WhenFileDoesNotExist_Throws(bool forceBackup) + public async Task ReadAllText_WhenFileDoesNotExist_Throws(bool forceBackup) { var tempFile = Path.Combine(CreateTempDir(), TestFileName); using var rfs = CreateRfs(); - Assert.Throws(() => rfs.Instance.ReadAllText(tempFile, forceBackup)); + await Assert.ThrowsAsync(async () => await rfs.Instance.ReadAllTextAsync(tempFile, forceBackup)); } private static DisposableReliableFileStorage CreateRfs() diff --git a/Dalamud/Configuration/Internal/DalamudConfiguration.cs b/Dalamud/Configuration/Internal/DalamudConfiguration.cs index 241a08d90..da0d7c2c6 100644 --- a/Dalamud/Configuration/Internal/DalamudConfiguration.cs +++ b/Dalamud/Configuration/Internal/DalamudConfiguration.cs @@ -503,13 +503,13 @@ internal sealed class DalamudConfiguration : IInternalDisposableService /// Path to read from. /// File storage. /// The deserialized configuration file. - public static DalamudConfiguration Load(string path, ReliableFileStorage fs) + public static async Task Load(string path, ReliableFileStorage fs) { DalamudConfiguration deserialized = null; try { - fs.ReadAllText(path, text => + await fs.ReadAllTextAsync(path, text => { deserialized = JsonConvert.DeserializeObject(text, SerializerSettings); @@ -580,8 +580,6 @@ internal sealed class DalamudConfiguration : IInternalDisposableService { this.Save(); this.isSaveQueued = false; - - Log.Verbose("Config saved"); } } @@ -630,16 +628,20 @@ internal sealed class DalamudConfiguration : IInternalDisposableService // Wait for previous write to finish this.writeTask?.Wait(); - this.writeTask = Task.Run(() => + this.writeTask = Task.Run(async () => { - Service.Get().WriteAllText( - this.configPath, - JsonConvert.SerializeObject(this, SerializerSettings)); + await Service.Get().WriteAllTextAsync( + this.configPath, + JsonConvert.SerializeObject(this, SerializerSettings)); + Log.Verbose("DalamudConfiguration saved"); }).ContinueWith(t => { if (t.IsFaulted) { - Log.Error(t.Exception, "Failed to save DalamudConfiguration to {Path}", this.configPath); + Log.Error( + t.Exception, + "Failed to save DalamudConfiguration to {Path}", + this.configPath); } }); diff --git a/Dalamud/Configuration/PluginConfigurations.cs b/Dalamud/Configuration/PluginConfigurations.cs index 8e32fa992..fa2969d31 100644 --- a/Dalamud/Configuration/PluginConfigurations.cs +++ b/Dalamud/Configuration/PluginConfigurations.cs @@ -2,6 +2,8 @@ using System.IO; using System.Reflection; using Dalamud.Storage; +using Dalamud.Utility; + using Newtonsoft.Json; namespace Dalamud.Configuration; @@ -9,6 +11,7 @@ namespace Dalamud.Configuration; /// /// Configuration to store settings for a dalamud plugin. /// +[Api13ToDo("Make this a service. We need to be able to dispose it reliably to write configs asynchronously. Maybe also let people write files with vfs.")] public sealed class PluginConfigurations { private readonly DirectoryInfo configDirectory; @@ -36,7 +39,7 @@ public sealed class PluginConfigurations public void Save(IPluginConfiguration config, string pluginName, Guid workingPluginId) { Service.Get() - .WriteAllText(this.GetConfigFile(pluginName).FullName, SerializeConfig(config), workingPluginId); + .WriteAllTextAsync(this.GetConfigFile(pluginName).FullName, SerializeConfig(config), workingPluginId).GetAwaiter().GetResult(); } /// @@ -52,12 +55,12 @@ public sealed class PluginConfigurations IPluginConfiguration? config = null; try { - Service.Get().ReadAllText(path.FullName, text => + Service.Get().ReadAllTextAsync(path.FullName, text => { config = DeserializeConfig(text); if (config == null) throw new Exception("Read config was null."); - }, workingPluginId); + }, workingPluginId).GetAwaiter().GetResult(); } catch (FileNotFoundException) { diff --git a/Dalamud/EntryPoint.cs b/Dalamud/EntryPoint.cs index 9fc09a56b..15077f3d8 100644 --- a/Dalamud/EntryPoint.cs +++ b/Dalamud/EntryPoint.cs @@ -144,7 +144,8 @@ public sealed class EntryPoint // Load configuration first to get some early persistent state, like log level var fs = new ReliableFileStorage(Path.GetDirectoryName(info.ConfigurationPath)!); - var configuration = DalamudConfiguration.Load(info.ConfigurationPath!, fs); + var configuration = DalamudConfiguration.Load(info.ConfigurationPath!, fs) + .GetAwaiter().GetResult(); // Set the appropriate logging level from the configuration if (!configuration.LogSynchronously) diff --git a/Dalamud/Interface/Internal/Windows/Data/Widgets/VfsWidget.cs b/Dalamud/Interface/Internal/Windows/Data/Widgets/VfsWidget.cs index f1f2476c9..f044b2989 100644 --- a/Dalamud/Interface/Internal/Windows/Data/Widgets/VfsWidget.cs +++ b/Dalamud/Interface/Internal/Windows/Data/Widgets/VfsWidget.cs @@ -52,7 +52,7 @@ internal class VfsWidget : IDataWindowWidget for (var i = 0; i < this.reps; i++) { stopwatch.Restart(); - service.WriteAllBytes(path, data); + service.WriteAllBytesAsync(path, data).GetAwaiter().GetResult(); stopwatch.Stop(); acc += stopwatch.ElapsedMilliseconds; Log.Information("Turn {Turn} took {Ms}ms", i, stopwatch.ElapsedMilliseconds); @@ -70,7 +70,7 @@ internal class VfsWidget : IDataWindowWidget for (var i = 0; i < this.reps; i++) { stopwatch.Restart(); - service.ReadAllBytes(path); + service.ReadAllBytesAsync(path).GetAwaiter().GetResult(); stopwatch.Stop(); acc += stopwatch.ElapsedMilliseconds; Log.Information("Turn {Turn} took {Ms}ms", i, stopwatch.ElapsedMilliseconds); diff --git a/Dalamud/Storage/ReliableFileStorage.cs b/Dalamud/Storage/ReliableFileStorage.cs index 9791b9e45..d9f8526c3 100644 --- a/Dalamud/Storage/ReliableFileStorage.cs +++ b/Dalamud/Storage/ReliableFileStorage.cs @@ -1,5 +1,6 @@ using System.IO; using System.Text; +using System.Threading.Tasks; using Dalamud.Logging.Internal; using Dalamud.Utility; @@ -92,8 +93,9 @@ internal class ReliableFileStorage : IInternalDisposableService /// Path to write to. /// The contents of the file. /// Container to write to. - public void WriteAllText(string path, string? contents, Guid containerId = default) - => this.WriteAllText(path, contents, Encoding.UTF8, containerId); + /// A representing the asynchronous operation. + public async Task WriteAllTextAsync(string path, string? contents, Guid containerId = default) + => await this.WriteAllTextAsync(path, contents, Encoding.UTF8, containerId); /// /// Write all text to a file. @@ -102,10 +104,11 @@ internal class ReliableFileStorage : IInternalDisposableService /// The contents of the file. /// The encoding to write with. /// Container to write to. - public void WriteAllText(string path, string? contents, Encoding encoding, Guid containerId = default) + /// A representing the asynchronous operation. + public async Task WriteAllTextAsync(string path, string? contents, Encoding encoding, Guid containerId = default) { var bytes = encoding.GetBytes(contents ?? string.Empty); - this.WriteAllBytes(path, bytes, containerId); + await this.WriteAllBytesAsync(path, bytes, containerId); } /// @@ -114,7 +117,8 @@ internal class ReliableFileStorage : IInternalDisposableService /// Path to write to. /// The contents of the file. /// Container to write to. - public void WriteAllBytes(string path, byte[] bytes, Guid containerId = default) + /// A representing the asynchronous operation. + public Task WriteAllBytesAsync(string path, byte[] bytes, Guid containerId = default) { ArgumentException.ThrowIfNullOrEmpty(path); @@ -123,7 +127,7 @@ internal class ReliableFileStorage : IInternalDisposableService if (this.db == null) { FilesystemUtil.WriteAllBytesSafe(path, bytes); - return; + return Task.CompletedTask; } this.db.RunInTransaction(() => @@ -149,6 +153,8 @@ internal class ReliableFileStorage : IInternalDisposableService FilesystemUtil.WriteAllBytesSafe(path, bytes); }); } + + return Task.CompletedTask; } /// @@ -161,8 +167,8 @@ internal class ReliableFileStorage : IInternalDisposableService /// The container to read from. /// All text stored in this file. /// Thrown if the file does not exist on the filesystem or in the backup. - public string ReadAllText(string path, bool forceBackup = false, Guid containerId = default) - => this.ReadAllText(path, Encoding.UTF8, forceBackup, containerId); + public Task ReadAllTextAsync(string path, bool forceBackup = false, Guid containerId = default) + => this.ReadAllTextAsync(path, Encoding.UTF8, forceBackup, containerId); /// /// Read all text from a file. @@ -175,9 +181,9 @@ internal class ReliableFileStorage : IInternalDisposableService /// The container to read from. /// All text stored in this file. /// Thrown if the file does not exist on the filesystem or in the backup. - public string ReadAllText(string path, Encoding encoding, bool forceBackup = false, Guid containerId = default) + public async Task ReadAllTextAsync(string path, Encoding encoding, bool forceBackup = false, Guid containerId = default) { - var bytes = this.ReadAllBytes(path, forceBackup, containerId); + var bytes = await this.ReadAllBytesAsync(path, forceBackup, containerId); return encoding.GetString(bytes); } @@ -191,8 +197,9 @@ internal class ReliableFileStorage : IInternalDisposableService /// The container to read from. /// Thrown if the file does not exist on the filesystem or in the backup. /// Thrown here if the file and the backup fail their read. - public void ReadAllText(string path, Action reader, Guid containerId = default) - => this.ReadAllText(path, Encoding.UTF8, reader, containerId); + /// A representing the asynchronous operation. + public async Task ReadAllTextAsync(string path, Action reader, Guid containerId = default) + => await this.ReadAllTextAsync(path, Encoding.UTF8, reader, containerId); /// /// Read all text from a file, and automatically try again with the backup if the file does not exist or @@ -205,7 +212,8 @@ internal class ReliableFileStorage : IInternalDisposableService /// The container to read from. /// Thrown if the file does not exist on the filesystem or in the backup. /// Thrown here if the file and the backup fail their read. - public void ReadAllText(string path, Encoding encoding, Action reader, Guid containerId = default) + /// A representing the asynchronous operation. + public async Task ReadAllTextAsync(string path, Encoding encoding, Action reader, Guid containerId = default) { ArgumentException.ThrowIfNullOrEmpty(path); @@ -216,7 +224,7 @@ internal class ReliableFileStorage : IInternalDisposableService // 1.) Try without using the backup try { - var text = this.ReadAllText(path, encoding, false, containerId); + var text = await this.ReadAllTextAsync(path, encoding, false, containerId); reader(text); return; } @@ -233,7 +241,7 @@ internal class ReliableFileStorage : IInternalDisposableService // 2.) Try using the backup try { - var text = this.ReadAllText(path, encoding, true, containerId); + var text = await this.ReadAllTextAsync(path, encoding, true, containerId); reader(text); } catch (Exception ex) @@ -253,7 +261,7 @@ internal class ReliableFileStorage : IInternalDisposableService /// The container to read from. /// All bytes stored in this file. /// Thrown if the file does not exist on the filesystem or in the backup. - public byte[] ReadAllBytes(string path, bool forceBackup = false, Guid containerId = default) + public async Task ReadAllBytesAsync(string path, bool forceBackup = false, Guid containerId = default) { ArgumentException.ThrowIfNullOrEmpty(path); @@ -265,15 +273,12 @@ internal class ReliableFileStorage : IInternalDisposableService var normalizedPath = NormalizePath(path); var file = this.db.Table().FirstOrDefault(f => f.Path == normalizedPath && f.ContainerId == containerId); - if (file == null) - throw new FileNotFoundException(); - - return file.Data; + return file == null ? throw new FileNotFoundException() : file.Data; } // If the file doesn't exist, immediately check the backup db if (!File.Exists(path)) - return this.ReadAllBytes(path, true, containerId); + return await this.ReadAllBytesAsync(path, true, containerId); try { @@ -282,7 +287,7 @@ internal class ReliableFileStorage : IInternalDisposableService catch (Exception e) { Log.Error(e, "Failed to read file from disk, falling back to database"); - return this.ReadAllBytes(path, true, containerId); + return await this.ReadAllBytesAsync(path, true, containerId); } } From 05648f019be5365f1c98479da67428e83192711f Mon Sep 17 00:00:00 2001 From: goaaats Date: Tue, 18 Nov 2025 20:37:57 +0100 Subject: [PATCH 02/12] First draft of IReliableFileStorage service --- .../Plugin/Services/IReliableFileStorage.cs | 163 ++++++++++++++++++ .../ReliableFileStoragePluginScoped.cs | 120 +++++++++++++ 2 files changed, 283 insertions(+) create mode 100644 Dalamud/Plugin/Services/IReliableFileStorage.cs create mode 100644 Dalamud/Storage/ReliableFileStoragePluginScoped.cs diff --git a/Dalamud/Plugin/Services/IReliableFileStorage.cs b/Dalamud/Plugin/Services/IReliableFileStorage.cs new file mode 100644 index 000000000..3a3c070f0 --- /dev/null +++ b/Dalamud/Plugin/Services/IReliableFileStorage.cs @@ -0,0 +1,163 @@ +using System.IO; +using System.Text; +using System.Threading.Tasks; + +using Dalamud.Storage; + +namespace Dalamud.Plugin.Services; + +/// +/// Service to interact with the file system, as a replacement for standard C# file I/O. +/// Writes and reads using this service are, to the best of our ability, atomic and reliable. +/// +/// All data is synced to disk immediately and written to a database, additionally to files on disk. This means +/// that in case of file corruption, data can likely be recovered from the database. +/// +/// However, this also means that operations using this service duplicate data on disk, so we don't recommend +/// performing large file operations. The service will not permit files larger than +/// (64MB) to be written. +/// +public interface IReliableFileStorage : IDalamudService +{ + /// + /// Gets the maximum file size, in bytes, that can be written using this service. + /// + /// + /// The service enforces this limit when writing files and fails with an appropriate exception + /// (for example or a custom exception) when a caller attempts to write + /// more than this number of bytes. + /// + long MaxFileSizeBytes { get; } + + /// + /// Check whether a file exists either on the local filesystem or in the transparent backup database. + /// + /// The file system path to check. Must not be null or empty. + /// + /// True if the file exists on disk or a backup copy exists in the storage's internal journal/backup database; + /// otherwise false. + /// + /// Thrown when is null or empty. + bool Exists(string path); + + /// + /// Write the given text into a file using UTF-8 encoding. The write is performed atomically and is persisted to + /// both the filesystem and the internal backup database used by this service. + /// + /// The file path to write to. Must not be null or empty. + /// The string contents to write. May be null, in which case an empty file is written. + /// A that completes when the write has finished and been flushed to disk and the backup. + /// Thrown when is null or empty. + Task WriteAllTextAsync(string path, string? contents); + + /// + /// Write the given text into a file using the provided . The write is performed + /// atomically (to the extent possible) and is persisted to both the filesystem and the internal backup database + /// used by this service. + /// + /// The file path to write to. Must not be null or empty. + /// The string contents to write. May be null, in which case an empty file is written. + /// The text encoding to use when serializing the string to bytes. Must not be null. + /// A that completes when the write has finished and been flushed to disk and the backup. + /// Thrown when is null or empty. + /// Thrown when is null. + Task WriteAllTextAsync(string path, string? contents, Encoding encoding); + + /// + /// Write the given bytes to a file. The write is persisted to both the filesystem and the service's internal + /// backup database. Avoid writing extremely large byte arrays because this service duplicates data on disk. + /// + /// The file path to write to. Must not be null or empty. + /// The raw bytes to write. Must not be null. + /// A that completes when the write has finished and been flushed to disk and the backup. + /// Thrown when is null or empty. + /// Thrown when is null. + Task WriteAllBytesAsync(string path, byte[] bytes); + + /// + /// Read all text from a file using UTF-8 encoding. If the file is unreadable or missing on disk, the service + /// attempts to return a backed-up copy from its internal journal/backup database. + /// + /// The file path to read. Must not be null or empty. + /// + /// When true the service prefers the internal backup database and returns backed-up contents if available. When + /// false the service tries the filesystem first and falls back to the backup only on error or when the file is missing. + /// + /// The textual contents of the file, decoded using UTF-8. + /// Thrown when is null or empty. + /// Thrown when the file does not exist on disk and no backup copy is available. + Task ReadAllTextAsync(string path, bool forceBackup = false); + + /// + /// Read all text from a file using the specified . If the file is unreadable or + /// missing on disk, the service attempts to return a backed-up copy from its internal journal/backup database. + /// + /// The file path to read. Must not be null or empty. + /// The encoding to use when decoding the stored bytes into text. Must not be null. + /// + /// When true the service prefers the internal backup database and returns backed-up contents if available. When + /// false the service tries the filesystem first and falls back to the backup only on error or when the file is missing. + /// + /// The textual contents of the file decoded using the provided . + /// Thrown when is null or empty. + /// Thrown when is null. + /// Thrown when the file does not exist on disk and no backup copy is available. + Task ReadAllTextAsync(string path, Encoding encoding, bool forceBackup = false); + + /// + /// Read all text from a file and invoke the provided callback with the string + /// contents. If the reader throws or the initial read fails, the service attempts a backup read and invokes the + /// reader again with the backup contents. If both reads fail the service surfaces an exception to the caller. + /// + /// The file path to read. Must not be null or empty. + /// + /// A callback invoked with the file's textual contents. Must not be null. + /// If the callback throws an exception the service treats that as a signal to retry the read using the + /// internal backup database and will invoke the callback again with the backup contents when available. + /// For example, the callback can throw when JSON deserialization fails to request the backup copy instead of + /// silently accepting corrupt data. + /// + /// A that completes when the read (and any attempted fallback) and callback invocation have finished. + /// Thrown when is null or empty. + /// Thrown when is null. + /// Thrown when the file does not exist on disk and no backup copy is available. + /// Thrown when both the filesystem read and the backup read fail for other reasons. + Task ReadAllTextAsync(string path, Action reader); + + /// + /// Read all text from a file using the specified and invoke the provided + /// callback with the decoded string contents. If the reader throws or the initial + /// read fails, the service attempts a backup read and invokes the reader again with the backup contents. If + /// both reads fail the service surfaces an exception to the caller. + /// + /// The file path to read. Must not be null or empty. + /// The encoding to use when decoding the stored bytes into text. Must not be null. + /// + /// A callback invoked with the file's textual contents. Must not be null. + /// If the callback throws an exception the service treats that as a signal to retry the read using the + /// internal backup database and will invoke the callback again with the backup contents when available. + /// For example, the callback can throw when JSON deserialization fails to request the backup copy instead of + /// silently accepting corrupt data. + /// + /// A that completes when the read (and any attempted fallback) and callback invocation have finished. + /// Thrown when is null or empty. + /// Thrown when or is null. + /// Thrown when the file does not exist on disk and no backup copy is available. + /// Thrown when both the filesystem read and the backup read fail for other reasons. + Task ReadAllTextAsync(string path, Encoding encoding, Action reader); + + /// + /// Read all bytes from a file. If the file is unreadable or missing on disk, the service may try to return a + /// backed-up copy from its internal journal/backup database. + /// + /// The file path to read. Must not be null or empty. + /// + /// When true the service prefers the internal backup database and returns the backed-up contents + /// if available. When false the service tries the filesystem first and falls back to the backup only + /// on error or when the file is missing. + /// + /// The raw bytes stored in the file. + /// Thrown when is null or empty. + /// Thrown when the file does not exist on disk and no backup copy is available. + Task ReadAllBytesAsync(string path, bool forceBackup = false); +} diff --git a/Dalamud/Storage/ReliableFileStoragePluginScoped.cs b/Dalamud/Storage/ReliableFileStoragePluginScoped.cs new file mode 100644 index 000000000..1f1992da6 --- /dev/null +++ b/Dalamud/Storage/ReliableFileStoragePluginScoped.cs @@ -0,0 +1,120 @@ +using System.Threading.Tasks; +using System.Text; + +using Dalamud.IoC; +using Dalamud.IoC.Internal; +using Dalamud.Plugin.Internal.Types; +using Dalamud.Plugin.Services; + +namespace Dalamud.Storage; + +[PluginInterface] +[ServiceManager.ScopedService] +#pragma warning disable SA1015 +[ResolveVia] +#pragma warning restore SA1015 +public class ReliableFileStoragePluginScoped : IReliableFileStorage, IServiceType +{ + // TODO: Make sure pending writes are finalized on plugin unload? + + private readonly LocalPlugin plugin; + + [ServiceManager.ServiceDependency] + private readonly ReliableFileStorage storage = Service.Get(); + + [ServiceManager.ServiceConstructor] + internal ReliableFileStoragePluginScoped(LocalPlugin plugin) + { + this.plugin = plugin; + } + + /// + public long MaxFileSizeBytes => 64 * 1024 * 1024; + + /// + public bool Exists(string path) + { + return this.storage.Exists(path, this.plugin.EffectiveWorkingPluginId); + } + + /// + public Task WriteAllTextAsync(string path, string? contents) + { + ArgumentException.ThrowIfNullOrEmpty(path); + + var bytes = Encoding.UTF8.GetBytes(contents ?? string.Empty); + if (bytes.LongLength > this.MaxFileSizeBytes) + throw new ArgumentException($"The provided data exceeds the maximum allowed size of {this.MaxFileSizeBytes} bytes.", nameof(contents)); + + return this.storage.WriteAllBytesAsync(path, bytes, this.plugin.EffectiveWorkingPluginId); + } + + /// + public Task WriteAllTextAsync(string path, string? contents, Encoding encoding) + { + ArgumentException.ThrowIfNullOrEmpty(path); + ArgumentNullException.ThrowIfNull(encoding); + + var bytes = encoding.GetBytes(contents ?? string.Empty); + if (bytes.LongLength > this.MaxFileSizeBytes) + throw new ArgumentException($"The provided data exceeds the maximum allowed size of {this.MaxFileSizeBytes} bytes.", nameof(contents)); + + return this.storage.WriteAllBytesAsync(path, bytes, this.plugin.EffectiveWorkingPluginId); + } + + /// + public Task WriteAllBytesAsync(string path, byte[] bytes) + { + ArgumentException.ThrowIfNullOrEmpty(path); + ArgumentNullException.ThrowIfNull(bytes); + + if (bytes.LongLength > this.MaxFileSizeBytes) + throw new ArgumentException($"The provided data exceeds the maximum allowed size of {this.MaxFileSizeBytes} bytes.", nameof(bytes)); + + return this.storage.WriteAllBytesAsync(path, bytes, this.plugin.EffectiveWorkingPluginId); + } + + /// + public Task ReadAllTextAsync(string path, bool forceBackup = false) + { + ArgumentException.ThrowIfNullOrEmpty(path); + + return this.storage.ReadAllTextAsync(path, forceBackup, this.plugin.EffectiveWorkingPluginId); + } + + /// + public Task ReadAllTextAsync(string path, Encoding encoding, bool forceBackup = false) + { + ArgumentException.ThrowIfNullOrEmpty(path); + ArgumentNullException.ThrowIfNull(encoding); + + return this.storage.ReadAllTextAsync(path, encoding, forceBackup, this.plugin.EffectiveWorkingPluginId); + } + + /// + public Task ReadAllTextAsync(string path, Action reader) + { + ArgumentException.ThrowIfNullOrEmpty(path); + ArgumentNullException.ThrowIfNull(reader); + + return this.storage.ReadAllTextAsync(path, reader, this.plugin.EffectiveWorkingPluginId); + } + + /// + public Task ReadAllTextAsync(string path, Encoding encoding, Action reader) + { + ArgumentException.ThrowIfNullOrEmpty(path); + ArgumentNullException.ThrowIfNull(encoding); + ArgumentNullException.ThrowIfNull(reader); + + return this.storage.ReadAllTextAsync(path, encoding, reader, this.plugin.EffectiveWorkingPluginId); + } + + /// + public Task ReadAllBytesAsync(string path, bool forceBackup = false) + { + ArgumentException.ThrowIfNullOrEmpty(path); + + return this.storage.ReadAllBytesAsync(path, forceBackup, this.plugin.EffectiveWorkingPluginId); + } +} From f831a7c010240661969062e457ca4d59379b0a99 Mon Sep 17 00:00:00 2001 From: goaaats Date: Tue, 18 Nov 2025 21:39:47 +0100 Subject: [PATCH 03/12] Wait for pending writes when disposing service --- .../Plugin/Services/IReliableFileStorage.cs | 3 + .../ReliableFileStoragePluginScoped.cs | 101 +++++++++++++++--- 2 files changed, 92 insertions(+), 12 deletions(-) diff --git a/Dalamud/Plugin/Services/IReliableFileStorage.cs b/Dalamud/Plugin/Services/IReliableFileStorage.cs index 3a3c070f0..93c2df243 100644 --- a/Dalamud/Plugin/Services/IReliableFileStorage.cs +++ b/Dalamud/Plugin/Services/IReliableFileStorage.cs @@ -2,6 +2,7 @@ using System.IO; using System.Text; using System.Threading.Tasks; +using Dalamud.Configuration; using Dalamud.Storage; namespace Dalamud.Plugin.Services; @@ -16,6 +17,8 @@ namespace Dalamud.Plugin.Services; /// However, this also means that operations using this service duplicate data on disk, so we don't recommend /// performing large file operations. The service will not permit files larger than /// (64MB) to be written. +/// +/// Saved configuration data using the class uses this functionality implicitly. /// public interface IReliableFileStorage : IDalamudService { diff --git a/Dalamud/Storage/ReliableFileStoragePluginScoped.cs b/Dalamud/Storage/ReliableFileStoragePluginScoped.cs index 1f1992da6..f6598a087 100644 --- a/Dalamud/Storage/ReliableFileStoragePluginScoped.cs +++ b/Dalamud/Storage/ReliableFileStoragePluginScoped.cs @@ -1,5 +1,8 @@ -using System.Threading.Tasks; +using System.Collections.Generic; +using System.Linq; using System.Text; +using System.Threading; +using System.Threading.Tasks; using Dalamud.IoC; using Dalamud.IoC.Internal; @@ -8,20 +11,31 @@ using Dalamud.Plugin.Services; namespace Dalamud.Storage; +/// +/// Plugin-scoped VFS wrapper. +/// [PluginInterface] [ServiceManager.ScopedService] #pragma warning disable SA1015 [ResolveVia] #pragma warning restore SA1015 -public class ReliableFileStoragePluginScoped : IReliableFileStorage, IServiceType +public class ReliableFileStoragePluginScoped : IReliableFileStorage, IInternalDisposableService { - // TODO: Make sure pending writes are finalized on plugin unload? + private readonly Lock pendingLock = new(); + private readonly HashSet pendingWrites = []; private readonly LocalPlugin plugin; [ServiceManager.ServiceDependency] private readonly ReliableFileStorage storage = Service.Get(); + // When true, the scope is disposing and new write requests are rejected. + private volatile bool isDisposing = false; + + /// + /// Initializes a new instance of the class. + /// + /// The owner plugin. [ServiceManager.ServiceConstructor] internal ReliableFileStoragePluginScoped(LocalPlugin plugin) { @@ -34,32 +48,31 @@ public class ReliableFileStoragePluginScoped : IReliableFileStorage, IServiceTyp /// public bool Exists(string path) { + if (this.isDisposing) + throw new ObjectDisposedException(nameof(ReliableFileStoragePluginScoped)); + return this.storage.Exists(path, this.plugin.EffectiveWorkingPluginId); } /// public Task WriteAllTextAsync(string path, string? contents) { + // Route through WriteAllBytesAsync so all write tracking and size checks are centralized. ArgumentException.ThrowIfNullOrEmpty(path); var bytes = Encoding.UTF8.GetBytes(contents ?? string.Empty); - if (bytes.LongLength > this.MaxFileSizeBytes) - throw new ArgumentException($"The provided data exceeds the maximum allowed size of {this.MaxFileSizeBytes} bytes.", nameof(contents)); - - return this.storage.WriteAllBytesAsync(path, bytes, this.plugin.EffectiveWorkingPluginId); + return this.WriteAllBytesAsync(path, bytes); } /// public Task WriteAllTextAsync(string path, string? contents, Encoding encoding) { + // Route through WriteAllBytesAsync so all write tracking and size checks are centralized. ArgumentException.ThrowIfNullOrEmpty(path); ArgumentNullException.ThrowIfNull(encoding); var bytes = encoding.GetBytes(contents ?? string.Empty); - if (bytes.LongLength > this.MaxFileSizeBytes) - throw new ArgumentException($"The provided data exceeds the maximum allowed size of {this.MaxFileSizeBytes} bytes.", nameof(contents)); - - return this.storage.WriteAllBytesAsync(path, bytes, this.plugin.EffectiveWorkingPluginId); + return this.WriteAllBytesAsync(path, bytes); } /// @@ -71,12 +84,36 @@ public class ReliableFileStoragePluginScoped : IReliableFileStorage, IServiceTyp if (bytes.LongLength > this.MaxFileSizeBytes) throw new ArgumentException($"The provided data exceeds the maximum allowed size of {this.MaxFileSizeBytes} bytes.", nameof(bytes)); - return this.storage.WriteAllBytesAsync(path, bytes, this.plugin.EffectiveWorkingPluginId); + // Start the underlying write task + var task = Task.Run(() => this.storage.WriteAllBytesAsync(path, bytes, this.plugin.EffectiveWorkingPluginId)); + + // Track the task so we can wait for it on dispose + lock (this.pendingLock) + { + if (this.isDisposing) + throw new ObjectDisposedException(nameof(ReliableFileStoragePluginScoped)); + + this.pendingWrites.Add(task); + } + + // Remove when done, if the task is already done this runs synchronously here and removes immediately + _ = task.ContinueWith(t => + { + lock (this.pendingLock) + { + this.pendingWrites.Remove(t); + } + }, TaskContinuationOptions.ExecuteSynchronously); + + return task; } /// public Task ReadAllTextAsync(string path, bool forceBackup = false) { + if (this.isDisposing) + throw new ObjectDisposedException(nameof(ReliableFileStoragePluginScoped)); + ArgumentException.ThrowIfNullOrEmpty(path); return this.storage.ReadAllTextAsync(path, forceBackup, this.plugin.EffectiveWorkingPluginId); @@ -85,6 +122,9 @@ public class ReliableFileStoragePluginScoped : IReliableFileStorage, IServiceTyp /// public Task ReadAllTextAsync(string path, Encoding encoding, bool forceBackup = false) { + if (this.isDisposing) + throw new ObjectDisposedException(nameof(ReliableFileStoragePluginScoped)); + ArgumentException.ThrowIfNullOrEmpty(path); ArgumentNullException.ThrowIfNull(encoding); @@ -94,6 +134,9 @@ public class ReliableFileStoragePluginScoped : IReliableFileStorage, IServiceTyp /// public Task ReadAllTextAsync(string path, Action reader) { + if (this.isDisposing) + throw new ObjectDisposedException(nameof(ReliableFileStoragePluginScoped)); + ArgumentException.ThrowIfNullOrEmpty(path); ArgumentNullException.ThrowIfNull(reader); @@ -103,6 +146,9 @@ public class ReliableFileStoragePluginScoped : IReliableFileStorage, IServiceTyp /// public Task ReadAllTextAsync(string path, Encoding encoding, Action reader) { + if (this.isDisposing) + throw new ObjectDisposedException(nameof(ReliableFileStoragePluginScoped)); + ArgumentException.ThrowIfNullOrEmpty(path); ArgumentNullException.ThrowIfNull(encoding); ArgumentNullException.ThrowIfNull(reader); @@ -113,8 +159,39 @@ public class ReliableFileStoragePluginScoped : IReliableFileStorage, IServiceTyp /// public Task ReadAllBytesAsync(string path, bool forceBackup = false) { + if (this.isDisposing) + throw new ObjectDisposedException(nameof(ReliableFileStoragePluginScoped)); + ArgumentException.ThrowIfNullOrEmpty(path); return this.storage.ReadAllBytesAsync(path, forceBackup, this.plugin.EffectiveWorkingPluginId); } + + /// + public void DisposeService() + { + Task[] tasksSnapshot; + lock (this.pendingLock) + { + // Mark disposing to reject new writes. + this.isDisposing = true; + + if (this.pendingWrites.Count == 0) + return; + + tasksSnapshot = this.pendingWrites.ToArray(); + } + + try + { + // Wait for all pending writes to complete. If some complete while we're waiting they will be in tasksSnapshot + // and are observed here; newly started writes are rejected due to isDisposing. + Task.WaitAll(tasksSnapshot); + } + catch (AggregateException) + { + // Swallow exceptions here: the underlying write failures will have been surfaced earlier to callers. + // We don't want dispose to throw and crash unload sequences. + } + } } From 28941cb69e71b8d229e5a332ba7edb8c97dcd6d1 Mon Sep 17 00:00:00 2001 From: goaaats Date: Tue, 18 Nov 2025 21:58:13 +0100 Subject: [PATCH 04/12] SelfTestRegistryPluginScoped should inherit from IDalamudService --- .../Plugin/SelfTest/Internal/SelfTestRegistryPluginScoped.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Dalamud/Plugin/SelfTest/Internal/SelfTestRegistryPluginScoped.cs b/Dalamud/Plugin/SelfTest/Internal/SelfTestRegistryPluginScoped.cs index 18c518879..e3835ddbc 100644 --- a/Dalamud/Plugin/SelfTest/Internal/SelfTestRegistryPluginScoped.cs +++ b/Dalamud/Plugin/SelfTest/Internal/SelfTestRegistryPluginScoped.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using Dalamud.IoC; using Dalamud.IoC.Internal; using Dalamud.Plugin.Internal.Types; +using Dalamud.Plugin.Services; namespace Dalamud.Plugin.SelfTest.Internal; @@ -12,7 +13,7 @@ namespace Dalamud.Plugin.SelfTest.Internal; [PluginInterface] [ServiceManager.ScopedService] [ResolveVia] -internal class SelfTestRegistryPluginScoped : ISelfTestRegistry, IInternalDisposableService +internal class SelfTestRegistryPluginScoped : ISelfTestRegistry, IInternalDisposableService, IDalamudService { [ServiceManager.ServiceDependency] private readonly SelfTestRegistry selfTestRegistry = Service.Get(); From 0daca30203920fb733c2ab7efc3849216e28694c Mon Sep 17 00:00:00 2001 From: goaaats Date: Tue, 18 Nov 2025 23:17:21 +0100 Subject: [PATCH 05/12] Make IReliableFileStorage experimental, add legal diagnostic IDs --- Dalamud/Plugin/Services/IReliableFileStorage.cs | 2 ++ Dalamud/Plugin/Services/IUnlockState.cs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Dalamud/Plugin/Services/IReliableFileStorage.cs b/Dalamud/Plugin/Services/IReliableFileStorage.cs index 93c2df243..757493a20 100644 --- a/Dalamud/Plugin/Services/IReliableFileStorage.cs +++ b/Dalamud/Plugin/Services/IReliableFileStorage.cs @@ -1,3 +1,4 @@ +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Text; using System.Threading.Tasks; @@ -20,6 +21,7 @@ namespace Dalamud.Plugin.Services; /// /// Saved configuration data using the class uses this functionality implicitly. /// +[Experimental("Dalamud001")] public interface IReliableFileStorage : IDalamudService { /// diff --git a/Dalamud/Plugin/Services/IUnlockState.cs b/Dalamud/Plugin/Services/IUnlockState.cs index a0d733f55..0409843c4 100644 --- a/Dalamud/Plugin/Services/IUnlockState.cs +++ b/Dalamud/Plugin/Services/IUnlockState.cs @@ -10,7 +10,7 @@ namespace Dalamud.Plugin.Services; /// /// Interface for determining unlock state of various content in the game. /// -[Experimental("UnlockState")] +[Experimental("Dalamud001")] public interface IUnlockState : IDalamudService { /// From 0656bff1f9bd582fbaaef89e81f761580b1bec55 Mon Sep 17 00:00:00 2001 From: goaaats Date: Thu, 20 Nov 2025 19:31:50 +0100 Subject: [PATCH 06/12] Fix experimental diagnostic IDs --- Dalamud/Game/UnlockState/UnlockState.cs | 2 +- Dalamud/Storage/ReliableFileStoragePluginScoped.cs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Dalamud/Game/UnlockState/UnlockState.cs b/Dalamud/Game/UnlockState/UnlockState.cs index a4b9381cc..cd896ffb6 100644 --- a/Dalamud/Game/UnlockState/UnlockState.cs +++ b/Dalamud/Game/UnlockState/UnlockState.cs @@ -22,7 +22,7 @@ using PublicContentSheet = Lumina.Excel.Sheets.PublicContent; namespace Dalamud.Game.UnlockState; -#pragma warning disable UnlockState +#pragma warning disable Dalamud001 /// /// This class provides unlock state of various content in the game. diff --git a/Dalamud/Storage/ReliableFileStoragePluginScoped.cs b/Dalamud/Storage/ReliableFileStoragePluginScoped.cs index f6598a087..59d6bccc4 100644 --- a/Dalamud/Storage/ReliableFileStoragePluginScoped.cs +++ b/Dalamud/Storage/ReliableFileStoragePluginScoped.cs @@ -11,6 +11,8 @@ using Dalamud.Plugin.Services; namespace Dalamud.Storage; +#pragma warning disable Dalamud001 + /// /// Plugin-scoped VFS wrapper. /// From ea07f41ab14dffb93b9ef9cac00f4d8a9f8d8e03 Mon Sep 17 00:00:00 2001 From: bleatbot <106497096+bleatbot@users.noreply.github.com> Date: Sun, 23 Nov 2025 23:47:28 +0100 Subject: [PATCH 07/12] Update ClientStructs (#2465) Co-authored-by: github-actions[bot] --- lib/FFXIVClientStructs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/FFXIVClientStructs b/lib/FFXIVClientStructs index 0afa6b672..0769d1f18 160000 --- a/lib/FFXIVClientStructs +++ b/lib/FFXIVClientStructs @@ -1 +1 @@ -Subproject commit 0afa6b67288e5e667da74c1d3ad582e6c964644c +Subproject commit 0769d1f180f859688f47a7a99610e9ce10da946c From 1f30ce4c396287bf504fc2228aaf8f14eefa150d Mon Sep 17 00:00:00 2001 From: Haselnussbomber Date: Tue, 25 Nov 2025 18:41:21 +0100 Subject: [PATCH 08/12] Require TestingDalamudApiLevel to be set for testing --- Dalamud/Plugin/Internal/PluginManager.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Dalamud/Plugin/Internal/PluginManager.cs b/Dalamud/Plugin/Internal/PluginManager.cs index fd5c048c7..e2eded57c 100644 --- a/Dalamud/Plugin/Internal/PluginManager.cs +++ b/Dalamud/Plugin/Internal/PluginManager.cs @@ -362,6 +362,9 @@ internal class PluginManager : IInternalDisposableService if (!this.configuration.DoPluginTest) return false; + if (!manifest.TestingDalamudApiLevel.HasValue) + return false; + return manifest.IsTestingExclusive || manifest.IsAvailableForTesting; } From d56c7a19630070be274897f3a0ec6819986fdd2a Mon Sep 17 00:00:00 2001 From: goaaats Date: Tue, 25 Nov 2025 20:31:46 +0100 Subject: [PATCH 09/12] Add auto-generated changelogs through CI --- .github/generate_changelog.py | 211 +++++++++++++++++++++++ .github/workflows/generate-changelog.yml | 46 +++++ 2 files changed, 257 insertions(+) create mode 100644 .github/generate_changelog.py create mode 100644 .github/workflows/generate-changelog.yml diff --git a/.github/generate_changelog.py b/.github/generate_changelog.py new file mode 100644 index 000000000..5e921fd6e --- /dev/null +++ b/.github/generate_changelog.py @@ -0,0 +1,211 @@ +#!/usr/bin/env python3 +""" +Generate a changelog from git commits between the last two tags and post to Discord webhook. +""" + +import subprocess +import re +import sys +import json +import argparse +from typing import List, Tuple, Optional + + +def run_git_command(args: List[str]) -> str: + """Run a git command and return its output.""" + try: + result = subprocess.run( + ["git"] + args, + capture_output=True, + text=True, + check=True + ) + return result.stdout.strip() + except subprocess.CalledProcessError as e: + print(f"Git command failed: {e}", file=sys.stderr) + sys.exit(1) + + +def get_last_two_tags() -> Tuple[str, str]: + """Get the latest two git tags.""" + tags = run_git_command(["tag", "--sort=-version:refname"]) + tag_list = [t for t in tags.split("\n") if t] + + # Filter out old tags that start with 'v' (old versioning scheme) + tag_list = [t for t in tag_list if not t.startswith('v')] + + if len(tag_list) < 2: + print("Error: Need at least 2 tags in the repository", file=sys.stderr) + sys.exit(1) + + return tag_list[0], tag_list[1] + + +def get_submodule_commit(submodule_path: str, tag: str) -> Optional[str]: + """Get the commit hash of a submodule at a specific tag.""" + try: + # Get the submodule commit at the specified tag + result = run_git_command(["ls-tree", tag, submodule_path]) + # Format is: " commit \t" + parts = result.split() + if len(parts) >= 3 and parts[1] == "commit": + return parts[2] + return None + except: + return None + + +def get_commits_between_tags(tag1: str, tag2: str) -> List[Tuple[str, str]]: + """Get commits between two tags. Returns list of (message, author) tuples.""" + log_output = run_git_command([ + "log", + f"{tag2}..{tag1}", + "--format=%s|%an|%h" + ]) + + commits = [] + for line in log_output.split("\n"): + if "|" in line: + message, author, sha = line.split("|", 2) + commits.append((message.strip(), author.strip(), sha.strip())) + + return commits + + +def filter_commits(commits: List[Tuple[str, str]], ignore_patterns: List[str]) -> List[Tuple[str, str]]: + """Filter out commits matching any of the ignore patterns.""" + compiled_patterns = [re.compile(pattern) for pattern in ignore_patterns] + + filtered = [] + for message, author, sha in commits: + if not any(pattern.search(message) for pattern in compiled_patterns): + filtered.append((message, author, sha)) + + return filtered + + +def generate_changelog(version: str, prev_version: str, commits: List[Tuple[str, str]], + cs_commit_new: Optional[str], cs_commit_old: Optional[str]) -> str: + """Generate markdown changelog.""" + # Calculate statistics + commit_count = len(commits) + unique_authors = len(set(author for _, author, _ in commits)) + + changelog = f"# Dalamud Release v{version}\n\n" + changelog += f"We just released Dalamud v{version}, which should be available to users within a few minutes. " + changelog += f"This release includes **{commit_count} commit{'s' if commit_count != 1 else ''} from {unique_authors} contributor{'s' if unique_authors != 1 else ''}**.\n" + changelog += f"[Click here]() to see all Dalamud changes.\n\n" + + if cs_commit_new and cs_commit_old and cs_commit_new != cs_commit_old: + changelog += f"It ships with an updated **FFXIVClientStructs [`{cs_commit_new[:7]}`]()**.\n" + changelog += f"[Click here]() to see all CS changes.\n" + elif cs_commit_new: + changelog += f"It ships with **FFXIVClientStructs [`{cs_commit_new[:7]}`]()**.\n" + + changelog += "## Dalamud Changes\n\n" + + for message, author, sha in commits: + changelog += f"* {message} (by **{author}** as [`{sha}`]())\n" + + return changelog + + +def post_to_discord(webhook_url: str, content: str, version: str) -> None: + """Post changelog to Discord webhook as a file attachment.""" + try: + import requests + except ImportError: + print("Error: requests library is required. Install it with: pip install requests", file=sys.stderr) + sys.exit(1) + + filename = f"changelog-v{version}.md" + + # Prepare the payload + data = { + "content": f"Dalamud v{version} has been released!", + "attachments": [ + { + "id": "0", + "filename": filename + } + ] + } + + # Prepare the files + files = { + "payload_json": (None, json.dumps(data)), + "files[0]": (filename, content.encode('utf-8'), 'text/markdown') + } + + try: + result = requests.post(webhook_url, files=files) + result.raise_for_status() + print(f"Successfully posted to Discord webhook, code {result.status_code}") + except requests.exceptions.HTTPError as err: + print(f"Failed to post to Discord: {err}", file=sys.stderr) + sys.exit(1) + except Exception as e: + print(f"Failed to post to Discord: {e}", file=sys.stderr) + sys.exit(1) + + +def main(): + parser = argparse.ArgumentParser( + description="Generate changelog from git commits and post to Discord webhook" + ) + parser.add_argument( + "--webhook-url", + required=True, + help="Discord webhook URL" + ) + parser.add_argument( + "--ignore", + action="append", + default=[], + help="Regex patterns to ignore commits (can be specified multiple times)" + ) + parser.add_argument( + "--submodule-path", + default="lib/FFXIVClientStructs", + help="Path to the FFXIVClientStructs submodule (default: lib/FFXIVClientStructs)" + ) + + args = parser.parse_args() + + # Get the last two tags + latest_tag, previous_tag = get_last_two_tags() + print(f"Generating changelog between {previous_tag} and {latest_tag}") + + # Get submodule commits at both tags + cs_commit_new = get_submodule_commit(args.submodule_path, latest_tag) + cs_commit_old = get_submodule_commit(args.submodule_path, previous_tag) + + if cs_commit_new: + print(f"FFXIVClientStructs commit (new): {cs_commit_new[:7]}") + if cs_commit_old: + print(f"FFXIVClientStructs commit (old): {cs_commit_old[:7]}") + + # Get commits between tags + commits = get_commits_between_tags(latest_tag, previous_tag) + print(f"Found {len(commits)} commits") + + # Filter commits + filtered_commits = filter_commits(commits, args.ignore) + print(f"After filtering: {len(filtered_commits)} commits") + + # Generate changelog + changelog = generate_changelog(latest_tag, previous_tag, filtered_commits, + cs_commit_new, cs_commit_old) + + print("\n" + "="*50) + print("Generated Changelog:") + print("="*50) + print(changelog) + print("="*50 + "\n") + + # Post to Discord + post_to_discord(args.webhook_url, changelog, latest_tag) + + +if __name__ == "__main__": + main() \ No newline at end of file diff --git a/.github/workflows/generate-changelog.yml b/.github/workflows/generate-changelog.yml new file mode 100644 index 000000000..5fed3b1eb --- /dev/null +++ b/.github/workflows/generate-changelog.yml @@ -0,0 +1,46 @@ +name: Generate Changelog + +on: + workflow_dispatch: + push: + tags: + - '*' + +jobs: + generate-changelog: + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 # Fetch all history and tags + submodules: true # Fetch submodules + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.14' + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install requests + + - name: Generate and post changelog + run: | + python .github/generate_changelog.py \ + --webhook-url "${{ secrets.DISCORD_CHANGELOG_WEBHOOK_URL }}" \ + --ignore "^Merge" \ + --ignore "^build:" \ + --ignore "^docs:" + env: + GIT_TERMINAL_PROMPT: 0 + + - name: Upload changelog as artifact + if: always() + uses: actions/upload-artifact@v4 + with: + name: changelog + path: changelog-*.md + if-no-files-found: ignore \ No newline at end of file From bd427d7b54d2eb410db345c284fd05bd4269caf1 Mon Sep 17 00:00:00 2001 From: goaaats Date: Tue, 25 Nov 2025 20:32:39 +0100 Subject: [PATCH 10/12] build: 13.0.0.10 --- Dalamud/Dalamud.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dalamud/Dalamud.csproj b/Dalamud/Dalamud.csproj index e45ba8a1e..b5062438a 100644 --- a/Dalamud/Dalamud.csproj +++ b/Dalamud/Dalamud.csproj @@ -6,7 +6,7 @@ XIV Launcher addon framework - 13.0.0.9 + 13.0.0.10 $(DalamudVersion) $(DalamudVersion) $(DalamudVersion) From efd66fd3f87b5b5a452cdc47b304a2a35eaae99f Mon Sep 17 00:00:00 2001 From: goaaats Date: Wed, 26 Nov 2025 01:43:01 +0100 Subject: [PATCH 11/12] Handle errors in Window draw events by displaying an error message --- Dalamud/Interface/Windowing/Window.cs | 71 ++++++++++++++++++++++++--- 1 file changed, 65 insertions(+), 6 deletions(-) diff --git a/Dalamud/Interface/Windowing/Window.cs b/Dalamud/Interface/Windowing/Window.cs index d4f0070db..44ff62199 100644 --- a/Dalamud/Interface/Windowing/Window.cs +++ b/Dalamud/Interface/Windowing/Window.cs @@ -55,6 +55,9 @@ public abstract class Window private Vector2 fadeOutSize = Vector2.Zero; private Vector2 fadeOutOrigin = Vector2.Zero; + private bool hasError = false; + private Exception? lastError; + /// /// Initializes a new instance of the class. /// @@ -458,14 +461,24 @@ public abstract class Window this.presetDirty = true; } - // Draw the actual window contents - try + if (this.hasError) { - this.Draw(); + this.DrawErrorMessage(); } - catch (Exception ex) + else { - Log.Error(ex, "Error during Draw(): {WindowName}", this.WindowName); + // Draw the actual window contents + try + { + this.Draw(); + } + catch (Exception ex) + { + Log.Error(ex, "Error during Draw(): {WindowName}", this.WindowName); + + this.hasError = true; + this.lastError = ex; + } } } @@ -793,7 +806,7 @@ public abstract class Window hovered = true; // We can't use ImGui native functions here, because they don't work with clickthrough - if ((global::Windows.Win32.PInvoke.GetKeyState((int)VirtualKey.LBUTTON) & 0x8000) != 0) + if ((Windows.Win32.PInvoke.GetKeyState((int)VirtualKey.LBUTTON) & 0x8000) != 0) { held = true; pressed = true; @@ -871,6 +884,52 @@ public abstract class Window ImGui.End(); } + private void DrawErrorMessage() + { + // TODO: Once window systems are services, offer to reload the plugin + ImGui.TextColoredWrapped(ImGuiColors.DalamudRed,Loc.Localize("WindowSystemErrorOccurred", "An error occurred while rendering this window. Please contact the developer for details.")); + + ImGuiHelpers.ScaledDummy(5); + + if (ImGui.Button(Loc.Localize("WindowSystemErrorRecoverButton", "Attempt to retry"))) + { + this.hasError = false; + this.lastError = null; + } + + ImGui.SameLine(); + + if (ImGui.Button(Loc.Localize("WindowSystemErrorClose", "Close Window"))) + { + this.IsOpen = false; + this.hasError = false; + this.lastError = null; + } + + ImGuiHelpers.ScaledDummy(10); + + if (this.lastError != null) + { + using var child = ImRaii.Child("##ErrorDetails", new Vector2(0, 200 * ImGuiHelpers.GlobalScale), true); + using (ImRaii.PushColor(ImGuiCol.Text, ImGuiColors.DalamudGrey)) + { + ImGui.TextWrapped(Loc.Localize("WindowSystemErrorDetails", "Error Details:")); + ImGui.Separator(); + ImGui.TextWrapped(this.lastError.ToString()); + } + + var childWindowSize = ImGui.GetWindowSize(); + var copyText = Loc.Localize("WindowSystemErrorCopy", "Copy"); + var buttonWidth = ImGuiComponents.GetIconButtonWithTextWidth(FontAwesomeIcon.Copy, copyText); + ImGui.SetCursorPos(new Vector2(childWindowSize.X - buttonWidth - ImGui.GetStyle().FramePadding.X, + ImGui.GetStyle().FramePadding.Y)); + if (ImGuiComponents.IconButtonWithText(FontAwesomeIcon.Copy, copyText)) + { + ImGui.SetClipboardText(this.lastError.ToString()); + } + } + } + /// /// Structure detailing the size constraints of a window. /// From 9e5195492eb34171b6751c017ee67557ec181a1d Mon Sep 17 00:00:00 2001 From: goaaats Date: Wed, 26 Nov 2025 21:07:49 +0100 Subject: [PATCH 12/12] Get active track from env var, instead of git branch --- Dalamud/Interface/Internal/DalamudInterface.cs | 2 +- .../Internal/Windows/BranchSwitcherWindow.cs | 10 ++++++---- Dalamud/Utility/Util.cs | 16 +++++++++++++--- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/Dalamud/Interface/Internal/DalamudInterface.cs b/Dalamud/Interface/Internal/DalamudInterface.cs index f2ffc7a4c..202334580 100644 --- a/Dalamud/Interface/Internal/DalamudInterface.cs +++ b/Dalamud/Interface/Internal/DalamudInterface.cs @@ -1060,7 +1060,7 @@ internal class DalamudInterface : IInternalDisposableService { ImGui.PushFont(InterfaceManager.MonoFont); - ImGui.BeginMenu(Util.GetBranch() ?? "???", false); + ImGui.BeginMenu($"{Util.GetActiveTrack() ?? "???"} on {Util.GetGitBranch() ?? "???"}", false); ImGui.BeginMenu($"{Util.GetScmVersion()}", false); ImGui.BeginMenu(this.FrameCount.ToString("000000"), false); ImGui.BeginMenu(ImGui.GetIO().Framerate.ToString("000"), false); diff --git a/Dalamud/Interface/Internal/Windows/BranchSwitcherWindow.cs b/Dalamud/Interface/Internal/Windows/BranchSwitcherWindow.cs index da6217aca..51ff3bdcd 100644 --- a/Dalamud/Interface/Internal/Windows/BranchSwitcherWindow.cs +++ b/Dalamud/Interface/Internal/Windows/BranchSwitcherWindow.cs @@ -46,10 +46,12 @@ public class BranchSwitcherWindow : Window this.branches = await client.GetFromJsonAsync>(BranchInfoUrl); Debug.Assert(this.branches != null, "this.branches != null"); - var branch = Util.GetBranch(); - this.selectedBranchIndex = this.branches!.Any(x => x.Value.Track == branch) ? - this.branches.TakeWhile(x => x.Value.Track != branch).Count() - : 0; + var trackName = Util.GetActiveTrack(); + this.selectedBranchIndex = this.branches.IndexOf(x => x.Value.Track != trackName); + if (this.selectedBranchIndex == -1) + { + this.selectedBranchIndex = 0; + } }); base.OnOpen(); diff --git a/Dalamud/Utility/Util.cs b/Dalamud/Utility/Util.cs index 2a3733303..19610ef64 100644 --- a/Dalamud/Utility/Util.cs +++ b/Dalamud/Utility/Util.cs @@ -140,10 +140,10 @@ public static partial class Util } /// - /// Gets the Dalamud branch name this version of Dalamud was built from, or null, if this is a Debug build. + /// Gets the Git branch name this version of Dalamud was built from, or null, if this is a Debug build. /// /// The branch name. - public static string? GetBranch() + public static string? GetGitBranch() { if (branchInternal != null) return branchInternal; @@ -155,7 +155,17 @@ public static partial class Util if (gitBranch == null) return null; - return branchInternal = gitBranch == "master" ? "release" : gitBranch; + return branchInternal = gitBranch; + } + + /// + /// Gets the active Dalamud track, if this instance was launched through XIVLauncher and used a version + /// downloaded from webservices. + /// + /// The name of the track, or null. + internal static string? GetActiveTrack() + { + return Environment.GetEnvironmentVariable("DALAMUD_BRANCH"); } ///