fix!: Game.mouseCursor and Game.overlays can now be safely set during onLoad (#1498)

examples/lib/stories/input/mouse_cursor_example.dart

@@ -46,12 +46,12 @@ class MouseCursorExample extends FlameGame with MouseMovementDetector {
       if (hovering) {
         if (!onTarget) {
           //Entered
-          mouseCursor.value = SystemMouseCursors.grab;
+          mouseCursor = SystemMouseCursors.grab;
         }
       } else {
         if (onTarget) {
           // Exited
-          mouseCursor.value = SystemMouseCursors.move;
+          mouseCursor = SystemMouseCursors.move;
         }
       }
       onTarget = hovering;

packages/flame/lib/src/game/game_widget/game_widget.dart

@@ -1,3 +1,4 @@
+import 'dart:async';
 import 'dart:developer';
 
 import 'package:flame/extensions.dart';
@@ -52,15 +53,6 @@ class GameWidget<T extends Game> extends StatefulWidget {
   /// - [Game.overlays]
   final Map<String, OverlayWidgetBuilder<T>>? overlayBuilderMap;
 
-  /// A List of the initially active overlays, this is used only on the first
-  /// build of the widget.
-  /// To control the overlays that are active use [Game.overlays].
-  ///
-  /// See also:
-  /// - [GameWidget]
-  /// - [Game.overlays]
-  final List<String>? initialActiveOverlays;
-
   /// The [FocusNode] to control the games focus to receive event inputs.
   /// If omitted, defaults to an internally controlled focus node.
   final FocusNode? focusNode;
@@ -69,10 +61,6 @@ class GameWidget<T extends Game> extends StatefulWidget {
   /// Defaults to true.
   final bool autofocus;
 
-  /// Initial mouse cursor for this [GameWidget]
-  /// mouse cursor can be changed in runtime using [Game.mouseCursor]
-  final MouseCursor? mouseCursor;
-
   /// Renders a [game] in a flutter widget tree.
   ///
   /// Ex:
@@ -111,7 +99,7 @@ class GameWidget<T extends Game> extends StatefulWidget {
   /// ...
   /// game.overlays.add('PauseMenu');
   /// ```
-  const GameWidget({
+  GameWidget({
     Key? key,
     required this.game,
     this.textDirection,
@@ -119,11 +107,18 @@ class GameWidget<T extends Game> extends StatefulWidget {
     this.errorBuilder,
     this.backgroundBuilder,
     this.overlayBuilderMap,
-    this.initialActiveOverlays,
+    List<String>? initialActiveOverlays,
     this.focusNode,
     this.autofocus = true,
-    this.mouseCursor,
+    MouseCursor? mouseCursor,
-  }) : super(key: key);
+  }) : super(key: key) {
+    if (mouseCursor != null) {
+      game.mouseCursor = mouseCursor;
+    }
+    if (initialActiveOverlays != null) {
+      initialActiveOverlays.forEach(game.overlays.add);
+    }
+  }
 
   /// Renders a [game] in a flutter widget tree alongside widgets overlays.
   ///
@@ -133,10 +128,6 @@ class GameWidget<T extends Game> extends StatefulWidget {
 }
 
 class _GameWidgetState<T extends Game> extends State<GameWidget<T>> {
-  Set<String> initialActiveOverlays = {};
-
-  MouseCursor? _mouseCursor;
-
   Future<void> get loaderFuture => _loaderFuture ??= (() async {
         assert(widget.game.hasLayout);
         final onLoad = widget.game.onLoadFuture;
@@ -150,55 +141,61 @@ class _GameWidgetState<T extends Game> extends State<GameWidget<T>> {
 
   late FocusNode _focusNode;
 
+  /// The number of `build()` functions currently executing.
+  int _buildDepth = 0;
+
+  /// If true, then a fresh build will be scheduled after the current one
+  /// completes. This should only be set to true when the [_buildDepth] is
+  /// non-zero.
+  bool _requiresRebuild = false;
+
+  /// Helper method that arranges to have `_buildDepth > 0` while the [build] is
+  /// executing, and then schedules a re-build if [_requiresRebuild] flag was
+  /// raised during the build.
+  ///
+  /// This is needed because our build function invokes user code, which in turn
+  /// may change some of the [Game]'s properties which would require the
+  /// [GameWidget] to be rebuilt. However, Flutter doesn't allow widgets to be
+  /// marked dirty while they are building. So, this method is needed to avoid
+  /// such a limitation and ensure that the user code can set [Game]'s
+  /// properties freely, and that they will be propagated to the [GameWidget]
+  /// at the earliest opportunity.
+  Widget _protectedBuild(Widget Function() build) {
+    late final Widget result;
+    try {
+      _buildDepth++;
+      result = build();
+    } finally {
+      _buildDepth--;
+    }
+    if (_requiresRebuild && _buildDepth == 0) {
+      Future.microtask(_onGameStateChange);
+    }
+    return result;
+  }
+
+  void _onGameStateChange() {
+    if (_buildDepth > 0) {
+      _requiresRebuild = true;
+    } else {
+      setState(() => _requiresRebuild = false);
+    }
+  }
+
   @override
   void initState() {
     super.initState();
-
+    widget.game.addGameStateListener(_onGameStateChange);
-    // Add the initial overlays
-    _initActiveOverlays();
-    addOverlaysListener();
-
-    // Add the initial mouse cursor
-    _initMouseCursor();
-    addMouseCursorListener();
-
     _focusNode = widget.focusNode ?? FocusNode();
     if (widget.autofocus) {
       _focusNode.requestFocus();
     }
   }
 
-  void _initMouseCursor() {
-    if (widget.mouseCursor != null) {
-      widget.game.mouseCursor.value = widget.mouseCursor;
-      _mouseCursor = widget.game.mouseCursor.value;
-    }
-  }
-
-  void _initActiveOverlays() {
-    if (widget.initialActiveOverlays == null) {
-      return;
-    }
-    _checkOverlays(widget.initialActiveOverlays!.toSet());
-    widget.initialActiveOverlays!.forEach((key) {
-      widget.game.overlays.add(key);
-    });
-  }
-
   @override
   void didUpdateWidget(GameWidget<T> oldWidget) {
     super.didUpdateWidget(oldWidget);
     if (oldWidget.game != widget.game) {
-      removeOverlaysListener(oldWidget.game);
-
-      // Reset the overlays
-      _initActiveOverlays();
-      addOverlaysListener();
-
-      // Reset mouse cursor
-      _initMouseCursor();
-      addMouseCursorListener();
-
       // Reset the loaderFuture so that onMount will run again
       // (onLoad is still cached).
       oldWidget.game.onRemove();
@@ -209,8 +206,8 @@ class _GameWidgetState<T extends Game> extends State<GameWidget<T>> {
   @override
   void dispose() {
     super.dispose();
+    widget.game.removeGameStateListener(_onGameStateChange);
     widget.game.onRemove();
-    removeOverlaysListener(widget.game);
     // If we received a focus node from the user, they are responsible
     // for disposing it
     if (widget.focusNode == null) {
@@ -218,27 +215,6 @@ class _GameWidgetState<T extends Game> extends State<GameWidget<T>> {
     }
   }
 
-  void addMouseCursorListener() {
-    widget.game.mouseCursor.addListener(onChangeMouseCursor);
-  }
-
-  void onChangeMouseCursor() {
-    setState(() {
-      _mouseCursor = widget.game.mouseCursor.value;
-    });
-  }
-
-  //#region Widget overlay methods
-
-  void addOverlaysListener() {
-    widget.game.overlays.addListener(onChangeActiveOverlays);
-    initialActiveOverlays = widget.game.overlays.value;
-  }
-
-  void removeOverlaysListener(T game) {
-    game.overlays.removeListener(onChangeActiveOverlays);
-  }
-
   void _checkOverlays(Set<String> overlays) {
     overlays.forEach((overlayKey) {
       assert(
@@ -248,15 +224,6 @@ class _GameWidgetState<T extends Game> extends State<GameWidget<T>> {
     });
   }
 
-  void onChangeActiveOverlays() {
-    _checkOverlays(widget.game.overlays.value);
-    setState(() {
-      initialActiveOverlays = widget.game.overlays.value;
-    });
-  }
-
-  //#endregion
-
   KeyEventResult _handleKeyEvent(FocusNode focusNode, RawKeyEvent event) {
     final game = widget.game;
     if (game is KeyboardEvents) {
@@ -267,9 +234,11 @@ class _GameWidgetState<T extends Game> extends State<GameWidget<T>> {
 
   @override
   Widget build(BuildContext context) {
+    return _protectedBuild(() {
       final game = widget.game;
       Widget internalGameWidget = _GameRenderObjectWidget(game);
 
+      _checkOverlays(widget.game.overlays.value);
       assert(
         !(game is MultiTouchDragDetector && game is PanDetector),
         'WARNING: Both MultiTouchDragDetector and a PanDetector detected. '
@@ -310,16 +279,18 @@ class _GameWidgetState<T extends Game> extends State<GameWidget<T>> {
         autofocus: widget.autofocus,
         onKey: _handleKeyEvent,
         child: MouseRegion(
-        cursor: _mouseCursor ?? MouseCursor.defer,
+          cursor: widget.game.mouseCursor,
           child: Directionality(
             textDirection: textDir,
             child: Container(
               color: game.backgroundColor(),
               child: LayoutBuilder(
                 builder: (_, BoxConstraints constraints) {
+                  return _protectedBuild(() {
                     final size = constraints.biggest.toVector2();
                     if (size.isZero()) {
-                  return widget.loadingBuilder?.call(context) ?? Container();
+                      return widget.loadingBuilder?.call(context) ??
+                          Container();
                     }
                     game.onGameResize(size);
                     return FutureBuilder(
@@ -346,15 +317,18 @@ class _GameWidgetState<T extends Game> extends State<GameWidget<T>> {
                         if (snapshot.connectionState == ConnectionState.done) {
                           return Stack(children: stackedWidgets);
                         }
-                    return widget.loadingBuilder?.call(context) ?? Container();
+                        return widget.loadingBuilder?.call(context) ??
+                            Container();
                       },
                     );
+                  });
                 },
               ),
             ),
           ),
         ),
       );
+    });
   }
 
   List<Widget> _addBackground(BuildContext context, List<Widget> stackWidgets) {
@@ -373,7 +347,7 @@ class _GameWidgetState<T extends Game> extends State<GameWidget<T>> {
     if (widget.overlayBuilderMap == null) {
       return stackWidgets;
     }
-    final widgets = initialActiveOverlays.map((String overlayKey) {
+    final widgets = widget.game.overlays.value.map((String overlayKey) {
       final builder = widget.overlayBuilderMap![overlayKey]!;
       return KeyedSubtree(
         key: ValueKey(overlayKey),

packages/flame/lib/src/game/mixins/game.dart

@@ -142,6 +142,7 @@ mixin Game {
       );
     }
     _gameRenderBox = gameRenderBox;
+    overlays._game = this;
 
     onAttach();
   }
@@ -262,72 +263,78 @@ mixin Game {
   VoidCallback? pauseEngineFn;
   VoidCallback? resumeEngineFn;
 
-  /// A property that stores an [ActiveOverlaysNotifier]
+  /// A property that stores an [_ActiveOverlays]
   ///
-  /// This is useful to render widgets above a game, like a pause menu for
+  /// This is useful to render widgets on top of a game, such as a pause menu.
-  /// example.
+  /// Overlays can be made visible via [overlays].add or hidden via
-  /// Overlays visible or hidden via [overlays].add or [overlays].remove,
+  /// [overlays].remove.
-  /// respectively.
   ///
-  /// Ex:
+  /// For example:
   /// ```
   /// final pauseOverlayIdentifier = 'PauseMenu';
   /// overlays.add(pauseOverlayIdentifier); // marks 'PauseMenu' to be rendered.
-  /// overlays.remove(pauseOverlayIdentifier); // marks 'PauseMenu' to not be rendered.
+  /// overlays.remove(pauseOverlayIdentifier); // hides 'PauseMenu'.
   /// ```
-  ///
+  final overlays = _ActiveOverlays();
-  /// See also:
-  /// - GameWidget
-  /// - [Game.overlays]
-  final overlays = ActiveOverlaysNotifier();
 
   /// Used to change the mouse cursor of the GameWidget running this game.
   /// Setting the value to null will make the GameWidget defer the choice
   /// of the cursor to the closest region available on the tree.
-  final mouseCursor = ValueNotifier<MouseCursor?>(null);
+  MouseCursor get mouseCursor => _mouseCursor;
+  MouseCursor _mouseCursor = MouseCursor.defer;
+  set mouseCursor(MouseCursor value) {
+    _mouseCursor = value;
+    _refreshWidget();
   }
 
-/// A [ChangeNotifier] used to control the visibility of overlays on a [Game]
+  final List<VoidCallback> _gameStateListeners = [];
-/// instance.
+  void addGameStateListener(VoidCallback callback) {
-///
+    _gameStateListeners.add(callback);
-/// To learn more, see:
+  }
-/// - [Game.overlays]
+
-class ActiveOverlaysNotifier extends ChangeNotifier {
+  void removeGameStateListener(VoidCallback callback) {
+    _gameStateListeners.remove(callback);
+  }
+
+  /// When a Game is attached to a `GameWidget`, this method will force that
+  /// widget to be rebuilt. This can be used when updating any property which is
+  /// implemented within the Flutter tree.
+  void _refreshWidget() {
+    _gameStateListeners.forEach((callback) => callback());
+  }
+}
+
+/// A helper class used to control the visibility of overlays on a [Game]
+/// instance. See [Game.overlays].
+class _ActiveOverlays {
+  Game? _game;
   final Set<String> _activeOverlays = {};
 
   /// Clear all active overlays.
   void clear() {
-    value.clear();
+    _activeOverlays.clear();
-    notifyListeners();
+    _game?._refreshWidget();
   }
 
-  /// Mark a, overlay to be rendered.
+  /// Marks the [overlayName] to be rendered.
-  ///
-  /// See also:
-  /// - GameWidget
-  /// - [Game.overlays]
   bool add(String overlayName) {
     final setChanged = _activeOverlays.add(overlayName);
     if (setChanged) {
-      notifyListeners();
+      _game?._refreshWidget();
     }
     return setChanged;
   }
 
-  /// Mark a, overlay to not be rendered.
+  /// Hides the [overlayName].
-  ///
-  /// See also:
-  /// - GameWidget
-  /// - [Game.overlays]
   bool remove(String overlayName) {
     final hasRemoved = _activeOverlays.remove(overlayName);
     if (hasRemoved) {
-      notifyListeners();
+      _game?._refreshWidget();
     }
     return hasRemoved;
   }
 
-  /// A [Set] of the active overlay names.
+  /// The names of all currently active overlays.
   Set<String> get value => _activeOverlays;
 
   /// Returns if the given [overlayName] is active

packages/flame/test/game/active_overlays_notifier_test.dart

@@ -1,72 +0,0 @@
-import 'package:flame/game.dart';
-import 'package:flutter_test/flutter_test.dart';
-
-void main() {
-  group('ActiveOverlaysNotifier', () {
-    test('can be constructed', () {
-      expect(ActiveOverlaysNotifier(), isNotNull);
-    });
-
-    late ActiveOverlaysNotifier notifier;
-
-    setUp(() {
-      notifier = ActiveOverlaysNotifier();
-    });
-
-    group('add', () {
-      test('can add an overlay', () {
-        final result = notifier.add('test');
-
-        expect(result, true);
-        expect(notifier.isActive('test'), true);
-      });
-
-      test('wont add same overlay', () {
-        notifier.add('test');
-        final result = notifier.add('test');
-
-        expect(result, false);
-      });
-    });
-
-    group('remove', () {
-      test('can remove an overlay', () {
-        notifier.add('test');
-
-        final result = notifier.remove('test');
-
-        expect(result, true);
-        expect(notifier.isActive('test'), false);
-      });
-
-      test('wont result in removal if there is nothing to remove', () {
-        final result = notifier.remove('test');
-
-        expect(result, false);
-      });
-    });
-
-    group('isActive', () {
-      test('is true when overlay is active', () {
-        notifier.add('test');
-        expect(notifier.isActive('test'), true);
-      });
-
-      test('is false when overlay is active', () {
-        expect(notifier.isActive('test'), false);
-      });
-    });
-
-    group('clear', () {
-      test('clears all overlays', () {
-        notifier.add('test1');
-        notifier.add('test2');
-
-        notifier.clear();
-
-        expect(notifier.isActive('test1'), false);
-        expect(notifier.isActive('test2'), false);
-      });
-    });
-  });
-}

packages/flame/test/game/active_overlays_test.dart

@@ -0,0 +1,65 @@
+import 'package:flame/game.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+void main() {
+  group('_ActiveOverlays', () {
+    group('add', () {
+      test('can add an overlay', () {
+        final overlays = FlameGame().overlays;
+        final added = overlays.add('test');
+        expect(added, true);
+        expect(overlays.isActive('test'), true);
+      });
+
+      test('wont add same overlay', () {
+        final overlays = FlameGame().overlays;
+        overlays.add('test');
+        final added = overlays.add('test');
+        expect(added, false);
+      });
+    });
+
+    group('remove', () {
+      test('can remove an overlay', () {
+        final overlays = FlameGame().overlays;
+        overlays.add('test');
+
+        final removed = overlays.remove('test');
+        expect(removed, true);
+        expect(overlays.isActive('test'), false);
+      });
+
+      test('will not result in removal if there is nothing to remove', () {
+        final overlays = FlameGame().overlays;
+        final removed = overlays.remove('test');
+        expect(removed, false);
+      });
+    });
+
+    group('isActive', () {
+      test('is true when overlay is active', () {
+        final overlays = FlameGame().overlays;
+        overlays.add('test');
+        expect(overlays.isActive('test'), true);
+      });
+
+      test('is false when overlay is active', () {
+        final overlays = FlameGame().overlays;
+        expect(overlays.isActive('test'), false);
+      });
+    });
+
+    group('clear', () {
+      test('clears all overlays', () {
+        final overlays = FlameGame().overlays;
+        overlays.add('test1');
+        overlays.add('test2');
+
+        overlays.clear();
+
+        expect(overlays.isActive('test1'), false);
+        expect(overlays.isActive('test2'), false);
+      });
+    });
+  });
+}

packages/flame/test/game/game_widget/game_widget_mouse_cursor_test.dart

@@ -37,11 +37,13 @@ void main() {
           ),
         ),
       );
+      await tester.pump();
+      expect(game.isAttached, true);
 
       // Making sure this cursor isn't showing yet
       expect(byMouseCursor(SystemMouseCursors.copy), findsNothing);
 
-      game.mouseCursor.value = SystemMouseCursors.copy;
+      game.mouseCursor = SystemMouseCursors.copy;
       await tester.pump();
 
       expect(
@@ -49,5 +51,28 @@ void main() {
         findsOneWidget,
       );
     });
+
+    testWidgets(
+      'can set mouseCursor during onLoad',
+      (tester) async {
+        final game = GameWithMouseCursorSetDuringOnLoad();
+        await tester.pumpWidget(
+          GameWidget(game: game),
+        );
+        await tester.pump();
+        expect(
+          byMouseCursor(SystemMouseCursors.alias),
+          findsOneWidget,
+        );
+      },
+    );
   });
 }
+
+class GameWithMouseCursorSetDuringOnLoad extends FlameGame {
+  @override
+  Future<void>? onLoad() {
+    mouseCursor = SystemMouseCursors.alias;
+    return null;
+  }
+}