diff --git a/GFramework.Godot.Tests/Logging/GodotLogTemplateTests.cs b/GFramework.Godot.Tests/Logging/GodotLogTemplateTests.cs index 8e9f53e2..a1a34e3b 100644 --- a/GFramework.Godot.Tests/Logging/GodotLogTemplateTests.cs +++ b/GFramework.Godot.Tests/Logging/GodotLogTemplateTests.cs @@ -129,4 +129,14 @@ public sealed class GodotLogTemplateTests Assert.That(result, Is.EqualTo("red")); } + + [Test] + public void Options_Should_Use_White_Color_When_Level_Is_Not_Defined() + { + var options = new GodotLoggerOptions(); + + var result = options.GetColor((LogLevel)999); + + Assert.That(result, Is.EqualTo("white")); + } } diff --git a/GFramework.Godot.Tests/Logging/GodotLoggerSettingsLoaderTests.cs b/GFramework.Godot.Tests/Logging/GodotLoggerSettingsLoaderTests.cs index caebf7e3..defd50f8 100644 --- a/GFramework.Godot.Tests/Logging/GodotLoggerSettingsLoaderTests.cs +++ b/GFramework.Godot.Tests/Logging/GodotLoggerSettingsLoaderTests.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Text.Json; using GFramework.Core.Abstractions.Logging; using GFramework.Godot.Logging; @@ -88,6 +89,50 @@ public sealed class GodotLoggerSettingsLoaderTests }); } + [Test] + public void LoadFromJsonString_Should_Normalize_Null_GodotLogger_Options() + { + const string json = """ + { + "Logging": { + "GodotLogger": { + "DebugOutputTemplate": null, + "ReleaseOutputTemplate": null, + "Colors": null + } + } + } + """; + + var settings = GodotLoggerSettingsLoader.LoadFromJsonString(json); + + Assert.Multiple(() => + { + Assert.That(settings.Options.DebugOutputTemplate, Is.Not.Null.And.Not.Empty); + Assert.That(settings.Options.ReleaseOutputTemplate, Is.Not.Null.And.Not.Empty); + Assert.That(settings.Options.GetColor(LogLevel.Info), Is.EqualTo("white")); + Assert.That(settings.Options.GetColor(LogLevel.Error), Is.EqualTo("red")); + }); + } + + [Test] + public void LoadFromJsonString_Should_Reject_Invalid_Numeric_LogLevel() + { + const string json = """ + { + "Logging": { + "LogLevel": { + "Default": 999 + } + } + } + """; + + var error = Assert.Throws(() => GodotLoggerSettingsLoader.LoadFromJsonString(json)); + + Assert.That(error?.Message, Does.Contain("Unsupported numeric LogLevel value '999'")); + } + [Test] public void Provider_Should_Apply_Updated_Settings_To_Existing_Loggers() { diff --git a/GFramework.Godot/Logging/DeferredLogger.cs b/GFramework.Godot/Logging/DeferredLogger.cs index 5507fc75..d37d4ccb 100644 --- a/GFramework.Godot/Logging/DeferredLogger.cs +++ b/GFramework.Godot/Logging/DeferredLogger.cs @@ -1,14 +1,39 @@ using System; using System.Linq; +using System.Threading; using GFramework.Core.Abstractions.Logging; namespace GFramework.Godot.Logging; +/// +/// Defers resolving the real Godot logger until the first logging operation needs it. +/// +/// +/// This wrapper allows static logger fields to be created before or +/// runs. The resolved inner logger is published with an atomic compare +/// exchange so concurrent first-use calls converge on one cached instance without relying on the non-atomic +/// null-coalescing assignment pattern. +/// internal sealed class DeferredLogger(string category, Func providerAccessor) : IStructuredLogger { private ILogger? _inner; - private ILogger Inner => _inner ??= ResolveLogger(); + private ILogger Inner + { + get + { + var current = Volatile.Read(ref _inner); + if (current != null) + { + return current; + } + + var createdLogger = ResolveLogger(); + + // Multiple callers can resolve concurrently; only one publishes the cached reference. + return Interlocked.CompareExchange(ref _inner, createdLogger, null) ?? createdLogger; + } + } public string Name() { diff --git a/GFramework.Godot/Logging/GodotLog.cs b/GFramework.Godot/Logging/GodotLog.cs index 7659e547..f305168a 100644 --- a/GFramework.Godot/Logging/GodotLog.cs +++ b/GFramework.Godot/Logging/GodotLog.cs @@ -34,10 +34,10 @@ public static class GodotLog lock (ConfigureLock) { - if (LazyProvider.IsValueCreated) + if (LazyProvider.IsValueCreated || LazyConfigurationSource.IsValueCreated) { throw new InvalidOperationException( - "GodotLog.Configure must be called before any GodotLog logger is materialized."); + "GodotLog.Configure must be called before any GodotLog provider or configuration source is materialized."); } _configure = configure; @@ -59,9 +59,16 @@ public static class GodotLog } /// - /// Gets the discovered configuration file path, if any. + /// Gets the discovered configuration file path, if any, without materializing the global configuration source. /// - public static string? ConfigurationPath => LazyConfigurationSource.Value.ConfigurationPath; + /// + /// This property is safe for diagnostics before runs. When the source is not created + /// yet, it performs discovery directly instead of touching LazyConfigurationSource.Value, so callers do + /// not accidentally lock in the default options before configuring . + /// + public static string? ConfigurationPath => LazyConfigurationSource.IsValueCreated + ? LazyConfigurationSource.Value.ConfigurationPath + : GodotLoggerSettingsLoader.DiscoverConfigurationPath(); /// /// Creates a logger for the specified category without materializing the provider until first use. @@ -88,6 +95,23 @@ public static class GodotLog LoggerFactoryResolver.Provider = Provider; } + /// + /// Stops the file watcher owned by the materialized configuration source, if the source has been created. + /// + /// + /// Godot hosts often keep process-wide logging for the whole game lifetime. Dedicated servers and tests can call + /// this method during teardown to release the watcher handle deterministically. The static lazy source is not + /// reset; later logger usage continues with the last published settings snapshot but no longer receives reload + /// notifications from the disposed watcher. + /// + public static void Shutdown() + { + if (LazyConfigurationSource.IsValueCreated) + { + LazyConfigurationSource.Value.Dispose(); + } + } + private static GodotLogConfigurationSource CreateConfigurationSource() { lock (ConfigureLock) diff --git a/GFramework.Godot/Logging/GodotLogConfigurationSource.cs b/GFramework.Godot/Logging/GodotLogConfigurationSource.cs index 7f2a908e..7ee68d25 100644 --- a/GFramework.Godot/Logging/GodotLogConfigurationSource.cs +++ b/GFramework.Godot/Logging/GodotLogConfigurationSource.cs @@ -6,29 +6,72 @@ using GFramework.Core.Abstractions.Logging; namespace GFramework.Godot.Logging; +/// +/// Owns discovery, loading, hot reload, and publication of the current Godot logger settings snapshot. +/// +/// +/// Construction follows a fixed lifecycle: discover the configuration path, perform an initial strict load, then +/// subscribe a when a concrete file exists. is +/// published through so cached loggers can read a last-good immutable snapshot without +/// locking. Hot reload keeps the previous settings when a transient parse or file-system error occurs. +/// internal sealed class GodotLogConfigurationSource : IDisposable { private readonly Action? _configure; private readonly FileSystemWatcher? _watcher; private GodotLoggerSettings _currentSettings = GodotLoggerSettings.Default; + /// + /// Initializes the configuration source and starts watching the discovered file when one is available. + /// + /// Optional imperative option overrides applied after file settings are loaded. + /// Thrown during initial loading when the configuration file cannot be read. + /// Thrown during initial loading when the configuration file is locked. + /// + /// Initial loading uses retry/backoff and propagates the final error because startup configuration failures should + /// be visible. Watcher callbacks use the hot-reload path and preserve the previous snapshot on failure. + /// public GodotLogConfigurationSource(Action? configure) { _configure = configure; + + // Discovery is done before the first strict reload so startup reports invalid files immediately. ConfigurationPath = GodotLoggerSettingsLoader.DiscoverConfigurationPath(); Reload(throwOnError: true); _watcher = CreateWatcher(ConfigurationPath); } + /// + /// Gets the discovered configuration file path, or null when no supported location contains a file. + /// public string? ConfigurationPath { get; } + /// + /// Gets the last successfully loaded settings snapshot. + /// + /// + /// The snapshot is read through Volatile.Read so logger instances running on other + /// threads observe settings published by reload callbacks without taking the configuration lock. + /// public GodotLoggerSettings CurrentSettings => Volatile.Read(ref _currentSettings); + /// + /// Stops the file watcher before the source is abandoned. + /// + /// + /// Disposal does not clear ; existing loggers can continue using the last published + /// snapshot after watcher notifications have been stopped. + /// public void Dispose() { _watcher?.Dispose(); } + /// + /// Creates the watcher that drives hot reload for the discovered configuration file. + /// + /// The configuration file to watch. + /// A configured watcher, or null when no stable directory and file name can be resolved. private FileSystemWatcher? CreateWatcher(string? configurationPath) { if (string.IsNullOrWhiteSpace(configurationPath)) @@ -43,6 +86,7 @@ internal sealed class GodotLogConfigurationSource : IDisposable return null; } + // FileSystemWatcher raises callbacks on thread-pool threads; callbacks keep reload work short and non-blocking. var watcher = new FileSystemWatcher(directory, fileName) { EnableRaisingEvents = true, @@ -69,11 +113,17 @@ internal sealed class GodotLogConfigurationSource : IDisposable Reload(throwOnError: false); } + /// + /// Reloads settings and publishes them when loading succeeds. + /// + /// Whether load errors should escape to the caller. private void Reload(bool throwOnError) { try { - var settings = LoadSettingsWithRetry(); + var settings = throwOnError ? LoadSettingsWithRetry() : LoadSettings(); + + // Volatile publication gives cached loggers a coherent replacement snapshot without per-log locks. Volatile.Write(ref _currentSettings, settings); } catch when (!throwOnError) @@ -82,6 +132,11 @@ internal sealed class GodotLogConfigurationSource : IDisposable } } + /// + /// Loads settings with short retry/backoff for startup races with file writers or deployment tools. + /// + /// The loaded settings snapshot. + /// Thrown when no retry produced a usable settings snapshot. private GodotLoggerSettings LoadSettingsWithRetry() { Exception? lastError = null; @@ -101,12 +156,20 @@ internal sealed class GodotLogConfigurationSource : IDisposable lastError = ex; } - Thread.Sleep(50); + if (attempt < 2) + { + // Startup can race with a writer finishing appsettings.json; keep the retry bounded and deterministic. + Thread.Sleep(50); + } } throw lastError ?? new InvalidOperationException("Failed to load Godot logging configuration."); } + /// + /// Loads settings from disk or defaults, then applies imperative overrides. + /// + /// The settings snapshot to publish. private GodotLoggerSettings LoadSettings() { var settings = string.IsNullOrWhiteSpace(ConfigurationPath) || !File.Exists(ConfigurationPath) @@ -120,9 +183,17 @@ internal sealed class GodotLogConfigurationSource : IDisposable var configuredOptions = CloneOptions(settings.Options); _configure(configuredOptions); - return new GodotLoggerSettings(configuredOptions, settings.DefaultLogLevel, CopyLoggerLevels(settings)); + return new GodotLoggerSettings( + configuredOptions.CreateNormalizedCopy(), + settings.DefaultLogLevel, + CopyLoggerLevels(settings)); } + /// + /// Creates a mutable options copy before user overrides are applied. + /// + /// The options from the file or default settings. + /// A normalized mutable copy. private static GodotLoggerOptions CloneOptions(GodotLoggerOptions options) { return new GodotLoggerOptions @@ -132,10 +203,17 @@ internal sealed class GodotLogConfigurationSource : IDisposable ReleaseMinLevel = options.ReleaseMinLevel, DebugOutputTemplate = options.DebugOutputTemplate, ReleaseOutputTemplate = options.ReleaseOutputTemplate, - Colors = new Dictionary(options.Colors) - }; + Colors = options.Colors is { } colors + ? new Dictionary(colors) + : [] + }.CreateNormalizedCopy(); } + /// + /// Copies category log level overrides into an ordinal dictionary. + /// + /// The source settings snapshot. + /// A copy that preserves exact and prefix matching semantics. private static IReadOnlyDictionary CopyLoggerLevels( GodotLoggerSettings settings) { diff --git a/GFramework.Godot/Logging/GodotLogRenderContext.cs b/GFramework.Godot/Logging/GodotLogRenderContext.cs index b7339aa8..095706cc 100644 --- a/GFramework.Godot/Logging/GodotLogRenderContext.cs +++ b/GFramework.Godot/Logging/GodotLogRenderContext.cs @@ -3,6 +3,15 @@ using GFramework.Core.Abstractions.Logging; namespace GFramework.Godot.Logging; +/// +/// Carries the already-resolved values that needs to render one log line. +/// +/// The UTC timestamp assigned to the log entry. +/// The severity level used for filtering, formatting, and Godot debug routing. +/// The source logger category name. +/// The formatted log message body. +/// The Godot BBCode color name resolved for . +/// The preformatted structured property suffix, or an empty string when none exist. internal readonly record struct GodotLogRenderContext( DateTime Timestamp, LogLevel Level, diff --git a/GFramework.Godot/Logging/GodotLogTemplate.cs b/GFramework.Godot/Logging/GodotLogTemplate.cs index 260da77f..5bc147a2 100644 --- a/GFramework.Godot/Logging/GodotLogTemplate.cs +++ b/GFramework.Godot/Logging/GodotLogTemplate.cs @@ -3,15 +3,41 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Globalization; using System.Text; +using System.Threading; using GFramework.Core.Abstractions.Logging; namespace GFramework.Godot.Logging; +/// +/// Parses and renders Godot logger output templates. +/// +/// +/// Supported placeholders include {timestamp}, {timestamp:format}, {level}, +/// {level:u3}, {level:l3}, {level:padded}, {category}, +/// {category:lN}, {category:rN}, {color}, {message}, and +/// {properties}. Unknown placeholders are rendered back as {key} so configuration mistakes stay +/// visible instead of silently deleting text. Parsed templates and category formatting results use bounded +/// concurrent caches to avoid unbounded growth across hot reloads or dynamic category names. +/// internal sealed class GodotLogTemplate { - private static readonly ConcurrentDictionary Cache = new(StringComparer.Ordinal); + /// + /// Caches parsed template instances by the raw template text. + /// + /// + /// The cache is process-wide because templates are immutable after parsing. It is bounded so repeated hot reloads + /// with unique template strings cannot grow memory without limit. + /// + private static readonly BoundedCache Cache = new(maxEntries: 256); - private readonly ConcurrentDictionary _categoryCache = new(StringComparer.Ordinal); + /// + /// Caches formatted category names for this template instance. + /// + /// + /// Category formatting depends on the template segment and category name. The per-template cache is bounded to + /// protect long-running hosts that create loggers with dynamic category names. + /// + private readonly BoundedCache _categoryCache = new(maxEntries: 1024); private readonly int _literalLength; private readonly Action[] _segments; @@ -20,12 +46,22 @@ internal sealed class GodotLogTemplate (_segments, _literalLength) = ParseCore(template); } + /// + /// Parses or retrieves a cached template. + /// + /// The template text. + /// An immutable parsed template. public static GodotLogTemplate Parse(string template) { ArgumentNullException.ThrowIfNull(template); - return Cache.GetOrAdd(template, static value => new GodotLogTemplate(value)); + return Cache.GetOrAdd(template, () => new GodotLogTemplate(template)); } + /// + /// Renders the template against a concrete log context. + /// + /// The resolved values for the log entry. + /// The rendered Godot log line. public string Render(GodotLogRenderContext context) { var builder = new StringBuilder(_literalLength + context.Category.Length + context.Message.Length + 48); @@ -37,6 +73,11 @@ internal sealed class GodotLogTemplate return builder.ToString(); } + /// + /// Converts template text into literal and placeholder render segments. + /// + /// The template text to parse. + /// The render segments and total literal length used to size the output builder. private (Action[] Segments, int LiteralLength) ParseCore(string template) { var segments = new List>(); @@ -45,6 +86,7 @@ internal sealed class GodotLogTemplate while (position < template.Length) { + // The parser is deliberately small: scan literal runs, then turn balanced placeholders into delegates. var open = template.IndexOf('{', position); if (open < 0) { @@ -79,10 +121,16 @@ internal sealed class GodotLogTemplate } literalLength += literal.Length; + // Capturing the literal string once avoids reparsing or slicing it on each rendered log entry. segments.Add((builder, _) => builder.Append(literal)); } } + /// + /// Creates the render delegate for one placeholder key. + /// + /// The placeholder name and optional format suffix. + /// A delegate that appends the placeholder value. private Action CreateSegment(string key) { return key switch @@ -98,10 +146,16 @@ internal sealed class GodotLogTemplate not null when key.StartsWith("category:", StringComparison.Ordinal) => CreateCategorySegment(key[9..]), not null when key.StartsWith("level:", StringComparison.Ordinal) => CreateLevelSegment(key[6..]), not null when key.StartsWith("timestamp:", StringComparison.Ordinal) => CreateTimestampSegment(key[10..]), + // Preserve unknown placeholders so configuration errors are visible in the rendered log line. _ => (builder, _) => builder.Append('{').Append(key).Append('}') }; } + /// + /// Creates the render delegate for a timestamp placeholder. + /// + /// The optional .NET timestamp format. + /// A delegate that appends the formatted timestamp using invariant culture. private Action CreateTimestampSegment(string format) { if (string.IsNullOrWhiteSpace(format)) @@ -114,6 +168,11 @@ internal sealed class GodotLogTemplate return (builder, context) => builder.Append(context.Timestamp.ToString(format, CultureInfo.InvariantCulture)); } + /// + /// Creates the render delegate for a level placeholder. + /// + /// The level format, such as u3, l3, or padded. + /// A delegate that appends the formatted level. private static Action CreateLevelSegment(string format) { return format switch @@ -125,6 +184,11 @@ internal sealed class GodotLogTemplate }; } + /// + /// Creates the render delegate for a category placeholder. + /// + /// The category alignment format, such as l16 or r32. + /// A delegate that appends the category with optional abbreviation and padding. private Action CreateCategorySegment(string format) { if (format.Length < 2) @@ -148,16 +212,31 @@ internal sealed class GodotLogTemplate : (builder, context) => builder.Append(GetFormattedCategory(context.Category, format, width, padLeft: true)); } + /// + /// Formats and caches one category for a category alignment segment. + /// + /// The full category name. + /// The original segment format used as part of the cache key. + /// The desired category width. + /// Whether the result is left-padded instead of right-padded. + /// The abbreviated and padded category string. private string GetFormattedCategory(string category, string format, int width, bool padLeft) { + // Include the format in the key because the same category can render differently per width and alignment. var cacheKey = string.Concat(format, "\0", category); - return _categoryCache.GetOrAdd(cacheKey, _ => + return _categoryCache.GetOrAdd(cacheKey, () => { var abbreviated = AbbreviateCategory(category, width); return padLeft ? abbreviated.PadLeft(width) : abbreviated.PadRight(width); }); } + /// + /// Abbreviates dotted category names to fit a target width. + /// + /// The category to abbreviate. + /// The maximum rendered length. + /// The category shortened by initials, dropped prefixes, or final-segment truncation. private static string AbbreviateCategory(string category, int maxLength) { if (category.Length <= maxLength) @@ -173,6 +252,7 @@ internal sealed class GodotLogTemplate for (var i = 0; i < parts.Length - 1; i++) { + // Collapse namespace-like prefixes first so the most specific final segment remains readable. if (parts[i].Length > 1) { parts[i] = parts[i][..1]; @@ -195,6 +275,12 @@ internal sealed class GodotLogTemplate return last.Length > maxLength ? last[..maxLength] : last; } + /// + /// Converts a level to its three-character form. + /// + /// The level to format. + /// Whether the result should use uppercase letters. + /// A three-character level label, or unk for undefined enum values. private static string ToShortLevel(LogLevel level, bool upper) { var value = level switch @@ -211,6 +297,11 @@ internal sealed class GodotLogTemplate return upper ? value.ToUpperInvariant() : value; } + /// + /// Converts a level to the fixed-width historical Godot logger label. + /// + /// The level to format. + /// A padded level label, or output for undefined enum values. private static string ToPaddedLevel(LogLevel level) { return level switch @@ -224,4 +315,60 @@ internal sealed class GodotLogTemplate _ => level.ToString() }; } + + private sealed class BoundedCache + { + private readonly ConcurrentDictionary> _entries = new(StringComparer.Ordinal); + private readonly int _maxEntries; + private long _sequence; + + internal BoundedCache(int maxEntries) + { + _maxEntries = maxEntries; + } + + internal TValue GetOrAdd(string key, Func valueFactory) + { + if (_entries.TryGetValue(key, out var existing)) + { + return existing.Value; + } + + var created = new CacheEntry(valueFactory(), Interlocked.Increment(ref _sequence)); + var stored = _entries.GetOrAdd(key, created); + if (stored.Sequence == created.Sequence) + { + Trim(); + } + + return stored.Value; + } + + private void Trim() + { + while (_entries.Count > _maxEntries) + { + var oldestKey = string.Empty; + var oldestSequence = long.MaxValue; + + foreach (var pair in _entries) + { + if (pair.Value.Sequence >= oldestSequence) + { + continue; + } + + oldestKey = pair.Key; + oldestSequence = pair.Value.Sequence; + } + + if (oldestSequence == long.MaxValue || !_entries.TryRemove(oldestKey, out _)) + { + break; + } + } + } + } + + private readonly record struct CacheEntry(TValue Value, long Sequence); } diff --git a/GFramework.Godot/Logging/GodotLogger.cs b/GFramework.Godot/Logging/GodotLogger.cs index 3a9ef33c..7a519b4d 100644 --- a/GFramework.Godot/Logging/GodotLogger.cs +++ b/GFramework.Godot/Logging/GodotLogger.cs @@ -13,7 +13,6 @@ namespace GFramework.Godot.Logging; /// public sealed class GodotLogger : AbstractLogger { - private readonly Func _minLevelProvider; private readonly Func _optionsProvider; /// @@ -37,9 +36,10 @@ public sealed class GodotLogger : AbstractLogger public GodotLogger(string? name, GodotLoggerOptions options) : this( name ?? RootLoggerName, - () => options ?? throw new ArgumentNullException(nameof(options)), - () => (options ?? throw new ArgumentNullException(nameof(options))).GetEffectiveMinLevel()) + () => options, + () => options.GetEffectiveMinLevel()) { + ArgumentNullException.ThrowIfNull(options); } internal GodotLogger( @@ -49,7 +49,6 @@ public sealed class GodotLogger : AbstractLogger : base(name, minLevelProvider ?? throw new ArgumentNullException(nameof(minLevelProvider))) { _optionsProvider = optionsProvider ?? throw new ArgumentNullException(nameof(optionsProvider)); - _minLevelProvider = minLevelProvider; } /// @@ -68,7 +67,7 @@ public sealed class GodotLogger : AbstractLogger /// public override void Log(LogLevel level, string message, params (string Key, object? Value)[] properties) { - if (level < _minLevelProvider()) + if (!IsEnabled(level)) { return; } @@ -85,7 +84,7 @@ public sealed class GodotLogger : AbstractLogger Exception? exception, params (string Key, object? Value)[] properties) { - if (level < _minLevelProvider()) + if (!IsEnabled(level)) { return; } diff --git a/GFramework.Godot/Logging/GodotLoggerOptions.cs b/GFramework.Godot/Logging/GodotLoggerOptions.cs index d8ac0382..25be0962 100644 --- a/GFramework.Godot/Logging/GodotLoggerOptions.cs +++ b/GFramework.Godot/Logging/GodotLoggerOptions.cs @@ -82,19 +82,39 @@ public sealed class GodotLoggerOptions /// The Godot named color. public string GetColor(LogLevel level) { - if (Colors.TryGetValue(level, out var color) && !string.IsNullOrWhiteSpace(color)) + if (Colors is { } colors && colors.TryGetValue(level, out var color) && !string.IsNullOrWhiteSpace(color)) { return color; } - return DefaultColors[level]; + return DefaultColors.TryGetValue(level, out var fallback) ? fallback : "white"; } + /// + /// Gets the active minimum level for the current . + /// + /// + /// when is ; otherwise + /// . + /// + /// + /// Factories use this value as the option-level floor before category-specific settings are applied. + /// internal LogLevel GetEffectiveMinLevel() { return Mode == GodotLoggerMode.Debug ? DebugMinLevel : ReleaseMinLevel; } + /// + /// Creates a copy whose debug and release floors are at least . + /// + /// The minimum level that both mode-specific floors must satisfy. + /// A normalized copy with stricter or equal mode-specific minimum levels. + /// + /// The operation can raise and through + /// , but it never lowers them. , + /// , and are preserved through a defensive copy. + /// internal GodotLoggerOptions WithMinimumLevelFloor(LogLevel minLevel) { return new GodotLoggerOptions @@ -104,7 +124,35 @@ public sealed class GodotLoggerOptions ReleaseMinLevel = Max(ReleaseMinLevel, minLevel), DebugOutputTemplate = DebugOutputTemplate, ReleaseOutputTemplate = ReleaseOutputTemplate, - Colors = new Dictionary(Colors) + Colors = CopyColorsWithDefaults(Colors) + }; + } + + /// + /// Creates a copy that replaces missing templates or color mappings with safe defaults. + /// + /// A normalized copy suitable for runtime rendering. + /// + /// JSON input can set , , or + /// to null even though the public API treats them as non-null. This method keeps + /// deserialization and imperative configuration from publishing values that would fail during rendering. + /// + internal GodotLoggerOptions CreateNormalizedCopy() + { + var defaults = new GodotLoggerOptions(); + + return new GodotLoggerOptions + { + Mode = Mode, + DebugMinLevel = DebugMinLevel, + ReleaseMinLevel = ReleaseMinLevel, + DebugOutputTemplate = string.IsNullOrWhiteSpace(DebugOutputTemplate) + ? defaults.DebugOutputTemplate + : DebugOutputTemplate, + ReleaseOutputTemplate = string.IsNullOrWhiteSpace(ReleaseOutputTemplate) + ? defaults.ReleaseOutputTemplate + : ReleaseOutputTemplate, + Colors = CopyColorsWithDefaults(Colors) }; } @@ -112,4 +160,23 @@ public sealed class GodotLoggerOptions { return left > right ? left : right; } + + private static Dictionary CopyColorsWithDefaults(Dictionary? colors) + { + var merged = new Dictionary(DefaultColors); + if (colors == null) + { + return merged; + } + + foreach (var pair in colors) + { + if (!string.IsNullOrWhiteSpace(pair.Value)) + { + merged[pair.Key] = pair.Value; + } + } + + return merged; + } } diff --git a/GFramework.Godot/Logging/GodotLoggerSettings.cs b/GFramework.Godot/Logging/GodotLoggerSettings.cs index df4b27e7..e95962fe 100644 --- a/GFramework.Godot/Logging/GodotLoggerSettings.cs +++ b/GFramework.Godot/Logging/GodotLoggerSettings.cs @@ -4,33 +4,82 @@ using GFramework.Core.Abstractions.Logging; namespace GFramework.Godot.Logging; +/// +/// Represents one immutable Godot logger configuration snapshot. +/// +/// +/// A snapshot combines mode-specific , an optional default log level, and category overrides. +/// Category matching is ordinal and deterministic: exact matches win first, then the longest dotted prefix such as +/// Game.Services for Game.Services.Inventory, and finally is used when +/// present. +/// internal sealed class GodotLoggerSettings { private readonly IReadOnlyDictionary _loggerLevels; + /// + /// Gets the default settings snapshot used when no configuration file is available. + /// public static GodotLoggerSettings Default { get; } = new(new GodotLoggerOptions()); + /// + /// Creates a settings snapshot from normalized options and optional category thresholds. + /// + /// The formatting and mode options for this snapshot. + /// The optional fallback level used when no category override matches. + /// Exact category names or dotted prefixes mapped to minimum levels. public GodotLoggerSettings( GodotLoggerOptions options, LogLevel? defaultLogLevel = null, IReadOnlyDictionary? loggerLevels = null) { - Options = options ?? throw new ArgumentNullException(nameof(options)); + Options = (options ?? throw new ArgumentNullException(nameof(options))).CreateNormalizedCopy(); DefaultLogLevel = defaultLogLevel; _loggerLevels = loggerLevels ?? new Dictionary(StringComparer.Ordinal); } + /// + /// Gets the optional fallback minimum level for categories without exact or prefix overrides. + /// public LogLevel? DefaultLogLevel { get; } + /// + /// Gets normalized rendering and mode options for this snapshot. + /// public GodotLoggerOptions Options { get; } + /// + /// Gets exact and dotted-prefix category level overrides. + /// + /// + /// Keys are interpreted with semantics. A key only matches a child category + /// when the category starts with the key plus a dot, which prevents Game.Service from matching + /// Game.Services accidentally. + /// public IReadOnlyDictionary LoggerLevels => _loggerLevels; + /// + /// Creates a settings snapshot from options without any category overrides. + /// + /// The options to normalize and wrap. + /// A settings snapshot that relies only on the option-level minimum level. public static GodotLoggerSettings FromOptions(GodotLoggerOptions options) { return new GodotLoggerSettings(options); } + /// + /// Calculates the effective minimum level for a category. + /// + /// The logger category name. + /// The provider-level floor captured by the logger. + /// The strictest level selected from options, provider floor, and category configuration. + /// + /// The merge starts with and + /// , then applies when it returns a + /// value. is used at each step so configuration can only make a logger + /// stricter, never more verbose than the active floor. + /// public LogLevel GetEffectiveMinLevel(string categoryName, LogLevel providerMinLevel) { ArgumentNullException.ThrowIfNull(categoryName); @@ -40,8 +89,14 @@ internal sealed class GodotLoggerSettings return configuredLevel.HasValue ? Max(effective, configuredLevel.Value) : effective; } + /// + /// Finds the configured category level using exact match, longest dotted-prefix match, then default fallback. + /// + /// The category to resolve. + /// The configured level, or null when no default or override applies. private LogLevel? GetConfiguredMinLevel(string categoryName) { + // Exact category configuration is the most specific and avoids unnecessary prefix scans. if (_loggerLevels.TryGetValue(categoryName, out var exactLevel)) { return exactLevel; @@ -52,6 +107,7 @@ internal sealed class GodotLoggerSettings foreach (var pair in _loggerLevels) { + // The dotted boundary keeps sibling categories from matching by raw string prefix alone. if (!categoryName.StartsWith(pair.Key + ".", StringComparison.Ordinal)) { continue; @@ -69,6 +125,12 @@ internal sealed class GodotLoggerSettings return bestMatchLevel; } + /// + /// Returns the stricter of two log levels. + /// + /// The first level. + /// The second level. + /// The level with the higher severity ordering. private static LogLevel Max(LogLevel left, LogLevel right) { return left > right ? left : right; diff --git a/GFramework.Godot/Logging/GodotLoggerSettingsLoader.cs b/GFramework.Godot/Logging/GodotLoggerSettingsLoader.cs index a5402a05..d460ba84 100644 --- a/GFramework.Godot/Logging/GodotLoggerSettingsLoader.cs +++ b/GFramework.Godot/Logging/GodotLoggerSettingsLoader.cs @@ -8,8 +8,18 @@ using Godot; namespace GFramework.Godot.Logging; +/// +/// Discovers and parses Godot logging configuration documents. +/// +/// +/// The loader treats JSON as external input: enum values are validated, nullable serializer output is normalized, +/// and unsupported values produce clear exceptions before a settings snapshot reaches runtime log rendering. +/// internal static class GodotLoggerSettingsLoader { + /// + /// Names the environment variable that can point to an explicit Godot logging configuration file. + /// internal const string ConfigEnvironmentVariableName = "GODOT_LOGGER_CONFIG"; private static readonly JsonSerializerOptions JsonOptions = new() @@ -24,6 +34,13 @@ internal static class GodotLoggerSettingsLoader } }; + /// + /// Finds the first supported configuration file location. + /// + /// Optional explicit path used instead of reading the environment variable. + /// Optional process path used when checking the executable directory. + /// Optional resolver for Godot res:// paths. + /// The first existing configuration path, or null when none exists. public static string? DiscoverConfigurationPath( string? environmentPath = null, string? processPath = null, @@ -59,6 +76,12 @@ internal static class GodotLoggerSettingsLoader return null; } + /// + /// Loads a settings snapshot from a JSON file. + /// + /// The configuration file path. + /// The parsed and normalized settings snapshot. + /// Thrown when does not exist. public static GodotLoggerSettings LoadFromJsonFile(string filePath) { ArgumentException.ThrowIfNullOrWhiteSpace(filePath); @@ -71,6 +94,12 @@ internal static class GodotLoggerSettingsLoader return LoadFromJsonString(File.ReadAllText(filePath)); } + /// + /// Parses a settings snapshot from a JSON string. + /// + /// The JSON configuration content. + /// The parsed and normalized settings snapshot. + /// Thrown when an unsupported log level or malformed document is encountered. public static GodotLoggerSettings LoadFromJsonString(string json) { ArgumentNullException.ThrowIfNull(json); @@ -79,7 +108,7 @@ internal static class GodotLoggerSettingsLoader ?? throw new InvalidOperationException("Failed to deserialize Godot logging configuration."); var logging = root.Logging; - var options = logging?.GodotLogger ?? new GodotLoggerOptions(); + var options = (logging?.GodotLogger ?? new GodotLoggerOptions()).CreateNormalizedCopy(); LogLevel? defaultLogLevel = null; var loggerLevels = new Dictionary(StringComparer.Ordinal); @@ -132,6 +161,12 @@ internal static class GodotLoggerSettingsLoader { if (reader.TokenType == JsonTokenType.Number && reader.TryGetInt32(out var numericValue)) { + if (!Enum.IsDefined(typeof(LogLevel), numericValue)) + { + throw new JsonException( + $"Unsupported numeric {nameof(LogLevel)} value '{numericValue}'. Expected a defined {nameof(LogLevel)} value."); + } + return (LogLevel)numericValue; } diff --git a/ai-plan/public/godot-logging-compliance-polish/todos/godot-logging-compliance-polish-tracking.md b/ai-plan/public/godot-logging-compliance-polish/todos/godot-logging-compliance-polish-tracking.md index b852408e..00865126 100644 --- a/ai-plan/public/godot-logging-compliance-polish/todos/godot-logging-compliance-polish-tracking.md +++ b/ai-plan/public/godot-logging-compliance-polish/todos/godot-logging-compliance-polish-tracking.md @@ -7,14 +7,15 @@ GFramework 自身日志抽象不分叉”的稳定宿主层，并为后续 Godot ## 当前恢复点 -- 恢复点编号：`GODOT-LOGGING-COMPLIANCE-POLISH-RP-001` -- 当前阶段：`Phase 1` +- 恢复点编号：`GODOT-LOGGING-COMPLIANCE-POLISH-RP-002` +- 当前阶段：`PR review hardening` - 当前焦点： - 已补齐 `GodotLog` 静态入口、延迟 logger 解析、配置自动发现与热重载 - 已让 `GodotLoggerFactoryProvider` 对已缓存 logger 生效动态配置，而不是只在新建 logger 时读快照 - 已让 `GodotLogger` 支持 `{properties}` 占位符，并把 `IStructuredLogger` / `LogContext` 属性落到 Godot 输出 - 已兼容 `GodotLogger` 风格配置值，如 `Information` / `Critical` - - 下一轮优先评估是否把 Godot 输出进一步并入 Core 的 appender / formatter / filter 组合管线 + - 已处理 PR #314 最新 AI review 中仍适用的生命周期、配置输入、缓存边界、注释和脚本健壮性问题 + - 下一轮优先只复核 CI 反馈是否已收敛，避免继续扩大 Godot logging API 面 ## 当前状态摘要 @@ -43,6 +44,10 @@ GFramework 自身日志抽象不分叉”的稳定宿主层，并为后续 Godot - GFramework 风格：`Info` / `Fatal` - `GodotLogger` 风格：`Information` / `Critical` - 现有设计仍保留 UTC 时间戳语义，没有为了对齐原项目而默认切回本地时间 +- `GodotLog.ConfigurationPath` 现在不会提前 materialize 全局配置源；`GodotLog.Shutdown()` 可释放已创建配置源的 watcher +- 配置 JSON 会先归一化模板和颜色字典，并拒绝未定义的数字 `LogLevel` +- `GodotLogTemplate` 的模板缓存和分类格式缓存已改为有界并发缓存，避免热重载或动态 category 长期单向增长 +- `refactor-scripts/update-namespaces.py` 已移除本机绝对路径默认值，并会把文件处理失败汇总成非零退出码 ## 当前风险 @@ -52,6 +57,8 @@ GFramework 自身日志抽象不分叉”的稳定宿主层，并为后续 Godot - 缓解措施：下一轮只评估“Godot sink / appender 化”，不再继续扩张独立的 Godot logging 面 - 配置热重载的宿主差异风险：Godot 编辑器、导出包和测试宿主的文件系统语义不完全一致 - 缓解措施：active 入口先锁定 discovery / reload 语义，后续若遇到平台差异，再用定向回归和文档补充收口 +- `GodotLog.ConfigurationPath` 的“不会 materialize”语义没有加入自动化测试 + - 缓解措施：直接调用会触碰 Godot project path resolver，在普通 test host 中可能崩溃；当前以实现和文档约束记录，后续若增加 Godot 宿主集成测试再覆盖 ## 活跃文档 @@ -69,9 +76,18 @@ GFramework 自身日志抽象不分叉”的稳定宿主层，并为后续 Godot - `dotnet test GFramework.Core.Tests/GFramework.Core.Tests.csproj -c Release --filter FullyQualifiedName~Logging -nologo` - 结果：通过 - 备注：Core logging 相关测试共 `214` 项通过，覆盖 `AbstractLogger` 动态最小级别改造回归 +- `dotnet test GFramework.Godot.Tests/GFramework.Godot.Tests.csproj -c Release --filter FullyQualifiedName~GodotLog -nologo` + - 结果：通过 + - 备注：PR review hardening 后 Godot logging 定向测试共 `14` 项通过 +- `dotnet test GFramework.Godot.Tests/GFramework.Godot.Tests.csproj -c Release -nologo` + - 结果：通过 + - 备注：PR review hardening 后 Godot 测试项目共 `72` 项通过 +- `python3 -B refactor-scripts/update-namespaces.py --help` + - 结果：通过 + - 备注：确认脚本 CLI 参数解析可用 ## 下一步 -1. 评估是否需要把 Godot 控制台输出收敛成 Core 可组合 sink / appender，而不是继续扩张独立 provider 逻辑 -2. 若继续做 Godot logger 能力，优先补真实宿主下的配置 reload / 输出行为回归，而不是再添加新的公开入口 -3. 若本轮改动进入 PR，后续 review / follow-up 继续写回本 topic，而不是另开第二份 Godot logging 追踪 +1. 刷新 PR review / CI 状态，确认最新 head 上 CodeRabbit 与 Greptile 线程是否关闭或变为 stale +2. 若 CI 仍报 MegaLinter `dotnet-format` restore 失败，优先复核 Actions restore 环境，而不是继续改本地格式 +3. 后续若继续推进能力设计，再评估 Godot 输出是否应变成 Core 可组合 sink / appender diff --git a/ai-plan/public/godot-logging-compliance-polish/traces/godot-logging-compliance-polish-trace.md b/ai-plan/public/godot-logging-compliance-polish/traces/godot-logging-compliance-polish-trace.md index de768d8f..6afb0b79 100644 --- a/ai-plan/public/godot-logging-compliance-polish/traces/godot-logging-compliance-polish-trace.md +++ b/ai-plan/public/godot-logging-compliance-polish/traces/godot-logging-compliance-polish-trace.md @@ -52,3 +52,33 @@ 1. 若继续推进本主题，优先评估 Godot 输出是否应变成 Core 可组合 appender / sink 2. 若出现后续 review 反馈，直接在本 topic 追加 RP-002，而不是重新开临时 local-plan 3. 若本主题阶段性完成，再把详细实现 history 迁入 `archive/`，active 入口只保留恢复点与风险 + +### 阶段：PR review hardening（RP-002） + +- 使用 `$gframework-pr-review` 抓取 PR #314 最新 review payload，确认当前 head 上仍有 CodeRabbit 与 Greptile + 未解决线程 +- 接受并处理仍适用的 review 结论： + - `GodotLog.ConfigurationPath` 不应提前创建全局配置源，`Configure(...)` 需要在 provider 或配置源已创建后 fail-fast + - 静态配置源需要可显式释放 watcher，因此新增 `GodotLog.Shutdown()` + - `DeferredLogger` 首次解析改为 `Interlocked.CompareExchange` 发布，避免 `_inner ??=` 并发竞态 + - `GodotLogger` 结构化 `Log(...)` 覆写改为复用 `IsEnabled(level)`，删除重复的最小级别 provider 字段 + - JSON 配置输入需要归一化模板和颜色字典，并拒绝未定义的数字 `LogLevel` + - `GodotLogTemplate` 模板缓存和分类缓存需要有界，避免热重载或动态 category 长期增长 + - `refactor-scripts/update-namespaces.py` 不能依赖本机绝对路径，也不能把文件处理异常吞成 0 次替换 +- 同步补充 Godot logging 内部类型和关键方法 XML 文档，说明热重载、快照发布、分类匹配和模板缓存语义 +- 同步更新 `docs/zh-CN/godot/logging.md`，记录 `ConfigurationPath` 的诊断语义和 `Shutdown()` teardown 用法 + +### 验证 + +- `dotnet test GFramework.Godot.Tests/GFramework.Godot.Tests.csproj -c Release --filter FullyQualifiedName~GodotLog -nologo` + - 结果：通过（14/14） +- `dotnet test GFramework.Godot.Tests/GFramework.Godot.Tests.csproj -c Release -nologo` + - 结果：通过（72/72） +- `python3 -B refactor-scripts/update-namespaces.py --help` + - 结果：通过 + +### 下一步 + +1. 提交 RP-002 review hardening 改动 +2. 刷新 PR review / CI，确认最新 head 是否关闭已处理线程 +3. 若 CI 仍只有 MegaLinter `dotnet-format` restore 失败，优先定位 Actions restore 环境 diff --git a/docs/zh-CN/godot/logging.md b/docs/zh-CN/godot/logging.md index 3811feba..0ae7f9f0 100644 --- a/docs/zh-CN/godot/logging.md +++ b/docs/zh-CN/godot/logging.md @@ -91,6 +91,10 @@ var logger = GodotLog.CreateLogger
(); - 自动按 `GODOT_LOGGER_CONFIG` -> 可执行目录 `appsettings.json` -> `res://appsettings.json` 顺序发现配置 - 返回延迟解析 logger，避免 `static readonly` 字段过早锁死配置 +`GodotLog.ConfigurationPath` 可以用于诊断当前会命中的配置文件路径；读取它不会提前创建全局配置源，也不会让后续 +`GodotLog.Configure(...)` 失效。长生命周期服务器或测试宿主如果需要在退出时主动释放配置文件 watcher，可以调用 +`GodotLog.Shutdown()`；它会停止热重载监听，已创建 logger 仍然继续使用最后一次成功发布的配置快照。 + ## 最小接入路径 ### 1. 在 `ArchitectureConfiguration` 中挂上 Godot provider diff --git a/refactor-scripts/update-namespaces.py b/refactor-scripts/update-namespaces.py index 7b69a07e..c836d35d 100644 --- a/refactor-scripts/update-namespaces.py +++ b/refactor-scripts/update-namespaces.py @@ -5,9 +5,10 @@ import os import re -from pathlib import Path +import sys +from argparse import ArgumentParser -ROOT_DIR = "/mnt/f/gewuyou/System/Documents/WorkSpace/GameDev/GFramework" +DEFAULT_ROOT_DIR = os.getcwd() # 命名空间替换规则（按优先级排序，长的先匹配） NAMESPACE_RULES = [ @@ -120,15 +121,26 @@ def update_file(file_path): return 0 except Exception as e: - print(f"错误处理文件 {file_path}: {e}") - return 0 + raise RuntimeError(f"错误处理文件 {file_path}: {e}") from e def main(): + parser = ArgumentParser(description="更新 C# 文件中的命名空间声明和 using 语句") + parser.add_argument( + "--root-dir", + default=os.getenv("ROOT_DIR", DEFAULT_ROOT_DIR), + help="要扫描的仓库根目录，默认使用 ROOT_DIR 环境变量或当前工作目录") + args = parser.parse_args() + root_dir = os.path.abspath(args.root_dir) + + if not os.path.isdir(root_dir): + print(f"根目录不存在或不是目录: {root_dir}", file=sys.stderr) + return 2 + print("开始更新命名空间...") # 查找所有 C# 文件 cs_files = [] - for root, dirs, files in os.walk(ROOT_DIR): + for root, dirs, files in os.walk(root_dir): # 跳过 bin, obj, Generated 目录 dirs[:] = [d for d in dirs if d not in ['bin', 'obj', 'Generated', '.git', 'node_modules']] @@ -140,15 +152,28 @@ def main(): updated_files = 0 total_replacements = 0 + failed_files = [] for file_path in cs_files: - replacements = update_file(file_path) + try: + replacements = update_file(file_path) + except RuntimeError as e: + failed_files.append((file_path, str(e))) + continue + if replacements > 0: updated_files += 1 total_replacements += replacements print(f"更新: {os.path.basename(file_path)} ({replacements} 处替换)") print(f"\n完成！更新了 {updated_files} 个文件，共 {total_replacements} 处替换") + if failed_files: + print(f"失败文件数: {len(failed_files)}", file=sys.stderr) + for file_path, error in failed_files: + print(f"- {file_path}: {error}", file=sys.stderr) + return 1 + + return 0 if __name__ == '__main__': - main() + sys.exit(main())