Fix engine-wide code review findings
CI / build-test (push) Failing after 1m7s

Collisions: init bucket heads to -1 (QueryAabb hung before the first
rebuild), reset query stamps on truncated QueryAabb (later queries
silently dropped entities), inside-origin raycasts hit at fraction 0 for
circles too, exactly-touching boxes now pair like touching circles.

Graphics: render into the letterbox viewport so the picture matches
ScreenToWorld/WorldToScreen instead of stretching; Y-sort by the
transform pivot rather than the quad center; lock-free snapshot
LayerRegistry (parallel submit read it unsynchronized); validate
InitialCapacity; warn when UseRenderer2D drops options of a later scene.

Core: scenes are explicitly single-use (re-loading threw silently
duplicated systems/entities before — now it throws), Scene.RegisterUnload
for per-scene resources, a switch requested during the reveal phase
covers again instead of hard-swapping, borderless fullscreen
(HardwareModeSwitch off), InputCapture service for input-suppressing
overlays, host disposes the transition renderer and IDisposable services
on shutdown.

Input: game input reads as released while InputCapture is held; mouse
position and wheel freeze so deltas stay zero.

DevConsole: holds InputCapture while open (typing no longer drives the
camera), Revision increments only under the lock, quoted command
arguments, history capped at 256.

UI: scene Desktop skips Myra input processing while the console is open
(clicks no longer fall through), is disposed on scene unload, and Myra
init no longer depends on a process-static flag.

Audio: validate channel count/sample rate before stopping the previous
track, empty looped oggs no longer hang FillBuffers, the instance stops
when a non-looping track drains (IsPlaying was stuck true).

Atlases: metadata v2 stores per-source size+mtime snapshots, so
timestamp-preserving copies and renames invalidate correctly; loader
checks the version and disposes pages on partial load failure; shared
pages never exceed a non-POT MaxPageSize; oversized items pack first
onto exact-size pages instead of splitting an open shared page; the CLI
validates numeric options.

Assets.Generator: file names are escaped in XML docs and string
literals, members no longer collide with the enclosing class (CS0542),
and the Assets root is resolved against build_property.projectdir so
nested "Assets" directories do not shift region paths.

Pathfinding: queries throw when the grid was resized after construction;
generation stamps survive int overflow.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Leonid Pershin
2026-06-11 21:18:08 +03:00
co-authored by Claude Fable 5
parent 56f2a85478
commit 501d81e19f
35 changed files with 926 additions and 98 deletions
+12
View File
@@ -36,4 +36,16 @@ public sealed class EngineContext
}
internal void AttachGraphicsDevice(GraphicsDevice device) => _graphicsDevice = device;
/// <summary>
/// Disposes everything the context owns: registered <see cref="IDisposable"/> services
/// and the transition renderer. <paramref name="except"/> (the host itself, also a
/// registered service) is skipped — it is being disposed by the caller already.
/// Called by <see cref="GameHost.Dispose(bool)"/>.
/// </summary>
internal void DisposeOwnedResources(object except)
{
Scenes.DisposeRenderer();
Services.DisposeServices(except);
}
}
+14
View File
@@ -29,6 +29,9 @@ public class GameHost : Game
PreferredBackBufferWidth = options.Width,
PreferredBackBufferHeight = options.Height,
IsFullScreen = options.Fullscreen,
// Borderless, как и обещает GameHostOptions.Fullscreen; по умолчанию MonoGame
// делает эксклюзивное переключение видеорежима монитора.
HardwareModeSwitch = false,
SynchronizeWithVerticalRetrace = options.VSync,
};
@@ -75,4 +78,15 @@ public class GameHost : Game
Context.Scenes.ApplyPending();
base.OnExiting(sender, args);
}
/// <inheritdoc />
protected override void Dispose(bool disposing)
{
if (disposing)
{
Context.DisposeOwnedResources(except: this);
}
base.Dispose(disposing);
}
}
+13
View File
@@ -0,0 +1,13 @@
namespace MrGameEng.Core;
/// <summary>
/// Shared flag service: true while a UI overlay (the developer console, a modal dialog)
/// captures input. Producers (e.g. the DevConsole module) set <see cref="Captured"/> while
/// open; consumers (the Input module, scene UI rendering) suppress game-facing input while
/// it is set. Lives in Core so modules can cooperate without referencing each other.
/// </summary>
public sealed class InputCapture
{
/// <summary>True while game-facing input should be suppressed.</summary>
public bool Captured { get; set; }
}
+4
View File
@@ -9,4 +9,8 @@
<PackageReference Include="Friflo.Engine.ECS" />
</ItemGroup>
<ItemGroup>
<InternalsVisibleTo Include="MrGameEng.Core.Tests" />
</ItemGroup>
</Project>
+27
View File
@@ -7,6 +7,9 @@ namespace MrGameEng.Core;
/// A scene owns its ECS world (<see cref="EntityStore"/>) and two system roots:
/// <see cref="UpdateSystems"/> for game logic and <see cref="DrawSystems"/> for rendering.
/// Override <see cref="OnLoad"/> to create entities and register systems.
/// Scene instances are single-use: <see cref="OnLoad"/> populates the store and the system
/// roots, and nothing resets them on unload — switch to a <b>new</b> instance instead of
/// reloading an old one (re-loading throws).
/// </summary>
public abstract class Scene
{
@@ -26,6 +29,8 @@ public abstract class Scene
public bool IsLoaded => _context is not null;
private EngineContext? _context;
private bool _loadedOnce;
private readonly List<Action> _unloadActions = [];
/// <summary>Initializes the scene's ECS world and system roots.</summary>
protected Scene()
@@ -40,6 +45,12 @@ public abstract class Scene
/// <summary>Called once when the scene is replaced or the game exits. Release scene resources here.</summary>
protected virtual void OnUnload() { }
/// <summary>
/// Registers a callback run once when the scene unloads (after <see cref="OnUnload"/>),
/// in reverse registration order. Engine modules use this to release per-scene resources.
/// </summary>
public void RegisterUnload(Action action) => _unloadActions.Add(action);
/// <summary>Runs the update phase. Called by <see cref="SceneManager"/>.</summary>
public virtual void Update(GameClock clock) =>
UpdateSystems.Update(new UpdateTick(clock.DeltaTime, (float)clock.TotalTime));
@@ -50,6 +61,16 @@ public abstract class Scene
internal void Load(EngineContext context)
{
// Без guard'а повторная загрузка молча дублирует системы и сущности
// (OnLoad добавляет в те же SystemRoot/EntityStore поверх старого содержимого).
if (_loadedOnce)
{
throw new InvalidOperationException(
$"Scene '{GetType().Name}' was already loaded once. Scene instances are " +
"single-use: create a new instance instead of switching back to an old one.");
}
_loadedOnce = true;
_context = context;
OnLoad();
}
@@ -57,6 +78,12 @@ public abstract class Scene
internal void Unload()
{
OnUnload();
for (var i = _unloadActions.Count - 1; i >= 0; i--)
{
_unloadActions[i]();
}
_unloadActions.Clear();
_context = null;
}
}
+16 -3
View File
@@ -36,18 +36,24 @@ public sealed class SceneManager
/// Requests a switch to <paramref name="scene"/>. Without a transition the swap happens at
/// the start of the next update tick; with one, the old scene is first covered.
/// Passing null unloads the current scene. Calling during an active transition replaces
/// the pending target scene.
/// the pending target scene; a switch requested while a transition is revealing starts
/// covering again from the current coverage.
/// </summary>
public void Switch(Scene? scene, Transition? transition = null)
{
_pending = scene;
_hasPending = true;
if (_state == State.Idle && transition is not null)
if (transition is not null && _state != State.CoveringOut)
{
_transition = transition;
if (_state == State.Idle)
{
_coverage = 0f;
}
// Из RevealingIn закрытие продолжается с текущего coverage — без скачка.
_state = State.CoveringOut;
_coverage = 0f;
}
}
@@ -99,6 +105,13 @@ public sealed class SceneManager
_transition!.Draw(_renderer, Math.Clamp(_coverage, 0f, 1f), phase);
}
/// <summary>Disposes the lazily created transition renderer. Called on host shutdown.</summary>
internal void DisposeRenderer()
{
_renderer?.Dispose();
_renderer = null;
}
internal void ApplyPending()
{
if (!_hasPending)
+19
View File
@@ -30,4 +30,23 @@ public sealed class ServiceRegistry
{
return _services.TryGetValue(typeof(T), out var service) ? (T)service : null;
}
/// <summary>
/// Disposes every registered <see cref="IDisposable"/> service (each instance once, even
/// when registered under several types) and clears the registry. <paramref name="except"/>
/// is skipped. Called on host shutdown.
/// </summary>
internal void DisposeServices(object? except = null)
{
var disposed = new HashSet<object>(ReferenceEqualityComparer.Instance);
foreach (var service in _services.Values)
{
if (!ReferenceEquals(service, except) && service is IDisposable disposable && disposed.Add(service))
{
disposable.Dispose();
}
}
_services.Clear();
}
}
+4 -1
View File
@@ -7,12 +7,15 @@ namespace MrGameEng.Core;
/// Minimal overlay renderer handed to <see cref="Transition.Draw"/>: fills rectangles in
/// normalized screen coordinates (0..1 on both axes) over the rendered scene.
/// </summary>
public sealed class TransitionRenderer
public sealed class TransitionRenderer : IDisposable
{
private readonly GraphicsDevice _device;
private readonly BasicEffect _effect;
private readonly VertexPositionColor[] _vertices = new VertexPositionColor[6];
/// <summary>Disposes the GPU effect. Called by <see cref="SceneManager"/> on shutdown.</summary>
public void Dispose() => _effect.Dispose();
internal TransitionRenderer(GraphicsDevice device)
{
_device = device;