Skip to content

Commit 27e60d0

Browse files
authored
Merge pull request #96 from vchelaru/fix/95-mouse-input-regression
Fix #95: mouse/gamepad input dies after restart (narrow Window event cleanup)
2 parents fa39846 + 3d95f4a commit 27e60d0

2 files changed

Lines changed: 75 additions & 13 deletions

File tree

XnaFiddle.Core/Plugins/GameWindowPlugin.cs

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,22 @@ public class GameWindowPlugin : ILibraryPlugin
1111
public string[] RequiredAssemblies => [];
1212
public string[] VersionAssemblies => [];
1313

14+
// The nkast.Wasm.Dom.Window event-delegate fields that BlazorGameWindow's ctor re-subscribes
15+
// on EVERY game (BlazorGameWindow.cs: OnTouch*, OnKeyDown/Up, OnFocus/OnBlur). CleanUp() nulls
16+
// exactly these between runs to stop per-run subscription accumulation (issue #90).
17+
//
18+
// It must NEVER include mouse/gamepad/resize: those are subscribed ONCE — ConcreteMouse
19+
// .PlatformSetWindowHandle and ConcreteGamePad wire them, gated on Mouse.WindowHandle, which
20+
// BlazorGameWindow sets only on the FIRST game. Nulling OnMouseMove/Down/Up/Wheel or
21+
// OnGamepad* orphans input permanently after the first restart (mouse/Gum/cursor-trail go
22+
// dead) — that was issue #95. Exposed (and pinned by GameWindowPluginTests) so this contract
23+
// can't silently regress.
24+
public static readonly string[] ClearedWindowEventFields =
25+
{
26+
"OnTouchStart", "OnTouchMove", "OnTouchEnd", "OnTouchCancel",
27+
"OnKeyDown", "OnKeyUp", "OnFocus", "OnBlur",
28+
};
29+
1430
public void CleanUp()
1531
{
1632
try
@@ -63,16 +79,18 @@ public void CleanUp()
6379

6480
try
6581
{
66-
// Each game's BlazorGameWindow ctor subscribes new closures to Window.Current's input-event
67-
// delegates (OnTouchStart/Move/End/Cancel, OnKeyDown/Up, OnMouse*, OnResize, etc.).
68-
// Window.Current is a page-lifetime singleton and old games are dropped without Dispose(),
69-
// so those closures accumulate one set per Run and are never removed. On touch devices a
70-
// single tap fans the event out to stale closures from dead games (reaching into a torn-down
71-
// TouchPanel.Current strategy), tripping a Mono runtime assertion (class-accessors.c) -> abort()
72-
// -> dead app on the 2nd-3rd restart. Desktop uses mouse paths with far more headroom and
73-
// tolerates it. Nulling these fields here (CleanUp runs before the next game's ctor
74-
// re-subscribes) makes each Run start from a single subscriber. Resolved by name because
75-
// nkast.Wasm.Dom is browser-only. See issue #90.
82+
// Null only the Window.Current event delegates that BlazorGameWindow's ctor
83+
// re-subscribes every game (ClearedWindowEventFields). These leak otherwise:
84+
// Window.Current is a page-lifetime singleton, old games are dropped without
85+
// Dispose(), so each Run's closures pile up; a single touch then fans out to stale
86+
// closures from dead games and trips a Mono runtime assertion (class-accessors.c) ->
87+
// abort() on the 2nd-3rd restart (issue #90). CleanUp runs before the next game's
88+
// ctor re-adds them, so each Run starts from a single subscriber.
89+
//
90+
// We must NOT blanket-clear every delegate field: mouse/gamepad are subscribed once
91+
// and never re-subscribed, so clearing them kills input after the first restart
92+
// (issue #95). See ClearedWindowEventFields. Resolved by name (nkast.Wasm.Dom is
93+
// browser-only).
7694
Type windowDomType = AppDomain.CurrentDomain.GetAssemblies()
7795
.Select(a => a.GetType("nkast.Wasm.Dom.Window"))
7896
.FirstOrDefault(t => t != null);
@@ -81,9 +99,9 @@ public void CleanUp()
8199
object window = currentProp?.GetValue(null);
82100
if (window != null)
83101
{
84-
foreach (var f in windowDomType.GetFields(BindingFlags.Instance | BindingFlags.Public))
85-
if (typeof(Delegate).IsAssignableFrom(f.FieldType))
86-
f.SetValue(window, null);
102+
foreach (string name in ClearedWindowEventFields)
103+
windowDomType.GetField(name, BindingFlags.Instance | BindingFlags.Public)
104+
?.SetValue(window, null);
87105
}
88106
}
89107
catch
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
using XnaFiddle.Plugins;
2+
3+
namespace XnaFiddle.Tests;
4+
5+
// These guard the contract for GameWindowPlugin.ClearedWindowEventFields — the set of
6+
// nkast.Wasm.Dom.Window event delegates CleanUp() nulls between runs.
7+
//
8+
// Testability note: the *behavior* (does mouse still work after a 2nd game?) can't be unit-tested
9+
// here — nkast.Wasm.Dom.Window and KNI's BlazorGameWindow/ConcreteMouse are browser-only and don't
10+
// load in net8.0, so CleanUp() no-ops in this process. (That's exactly why the issue #95 regression
11+
// shipped green.) These tests instead pin the *contract* so the cleared set can't silently drift
12+
// back into clearing handlers that aren't re-subscribed per game.
13+
public class GameWindowPluginTests
14+
{
15+
[Fact]
16+
public void ClearedWindowEventFields_match_the_per_game_resubscribed_set()
17+
{
18+
// Exactly the delegates BlazorGameWindow's ctor re-subscribes on every game.
19+
Assert.Equal(
20+
new[]
21+
{
22+
"OnTouchStart", "OnTouchMove", "OnTouchEnd", "OnTouchCancel",
23+
"OnKeyDown", "OnKeyUp", "OnFocus", "OnBlur",
24+
},
25+
GameWindowPlugin.ClearedWindowEventFields);
26+
}
27+
28+
// Issue #95 regression guard: mouse and gamepad are subscribed ONCE (ConcreteMouse
29+
// .PlatformSetWindowHandle / ConcreteGamePad, gated on the first game's Mouse.WindowHandle) and
30+
// never re-subscribed. Nulling them orphans input permanently after the first restart. They must
31+
// never appear in the cleared set.
32+
[Theory]
33+
[InlineData("OnMouseMove")]
34+
[InlineData("OnMouseDown")]
35+
[InlineData("OnMouseUp")]
36+
[InlineData("OnMouseWheel")]
37+
[InlineData("OnGamepadConnected")]
38+
[InlineData("OnGamepadDisconnected")]
39+
[InlineData("OnResize")]
40+
public void ClearedWindowEventFields_never_include_subscribe_once_handlers(string field)
41+
{
42+
Assert.DoesNotContain(field, GameWindowPlugin.ClearedWindowEventFields);
43+
}
44+
}

0 commit comments

Comments
 (0)