From c26580e706625864d5896c452f92c00cac313da4 Mon Sep 17 00:00:00 2001 From: Lukas Klingsbo Date: Wed, 26 Aug 2026 16:44:20 +0200 Subject: [PATCH] fix: Scan the lifecycle queue without its iterator in dequeueAdd/dequeueRemove RecycledQueue only supports a single iterator at a time, but dequeueAdd and dequeueRemove iterated the queue with a plain for loop. Both are reachable from inside processLifecycleEvents (through onMount/onLoad callbacks that remove a queued sibling or re-add a removing component), so the nested iteration reset the outer cursor, an already processed add event survived in the queue and was handled again, and the second mount tripped assert(!isMounted). Route dequeueAdd, dequeueRemove, handleResize and handleHotReload through RecycledQueue.forEachWhere, which walks the storage directly and is safe to call during iteration. Fixes #4018 Claude-Session: https://claude.ai/code/session_01Hqw2qU499wSThwN3EwEDR4 --- .../components/core/component_tree_root.dart | 67 ++++++++++++------- .../flame/test/components/component_test.dart | 58 ++++++++++++++++ 2 files changed, 100 insertions(+), 25 deletions(-) diff --git a/packages/flame/lib/src/components/core/component_tree_root.dart b/packages/flame/lib/src/components/core/component_tree_root.dart index 9627a335075..fb4e21745af 100644 --- a/packages/flame/lib/src/components/core/component_tree_root.dart +++ b/packages/flame/lib/src/components/core/component_tree_root.dart @@ -35,19 +35,30 @@ class ComponentTreeRoot extends Component { ..parent = parent; } + /// Cancels the pending ADD event for [child] into [parent]. + /// + /// Scans the queue without using its iterator, so this is safe to call while + /// [processLifecycleEvents] is iterating over the queue (for example from a + /// component's [Component.onMount]). @internal void dequeueAdd(Component child, Component parent) { - for (final event in queue) { - if (event.kind == LifecycleEventKind.add && + var found = false; + queue.forEachWhere( + (event) => + !found && + event.kind == LifecycleEventKind.add && event.child == child && - event.parent == parent) { + event.parent == parent, + (event) { event.kind = LifecycleEventKind.unknown; - return; - } - } - throw AssertionError( - 'Cannot find a lifecycle event Add(child=$child, parent=$parent)', + found = true; + }, ); + if (!found) { + throw AssertionError( + 'Cannot find a lifecycle event Add(child=$child, parent=$parent)', + ); + } } @internal @@ -58,13 +69,18 @@ class ComponentTreeRoot extends Component { ..parent = parent; } + /// Cancels all pending REMOVE events for [child]. + /// + /// Scans the queue without using its iterator, so this is safe to call while + /// [processLifecycleEvents] is iterating over the queue (for example from a + /// component's [Component.onMount]). @internal void dequeueRemove(Component child) { - for (final event in queue) { - if (event.kind == LifecycleEventKind.remove && event.child == child) { - event.kind = LifecycleEventKind.unknown; - } - } + queue.forEachWhere( + (event) => + event.kind == LifecycleEventKind.remove && event.child == child, + (event) => event.kind = LifecycleEventKind.unknown, + ); } /// Finds all children in [candidates] that have a pending REMOVE event, @@ -205,12 +221,10 @@ class ComponentTreeRoot extends Component { @internal void handleResize(Vector2 size) { super.handleResize(size); - for (final event in queue) { - if ((event.kind == LifecycleEventKind.add) && - (event.child!.isLoading || event.child!.isLoaded)) { - event.child!.onGameResize(size); - } - } + queue.forEachWhere( + _isPendingAddOfLoadingOrLoadedChild, + (event) => event.child!.onGameResize(size), + ); } @mustCallSuper @@ -218,12 +232,15 @@ class ComponentTreeRoot extends Component { @internal void handleHotReload() { super.handleHotReload(); - for (final event in queue) { - if ((event.kind == LifecycleEventKind.add) && - (event.child!.isLoading || event.child!.isLoaded)) { - event.child!.onHotReload(); - } - } + queue.forEachWhere( + _isPendingAddOfLoadingOrLoadedChild, + (event) => event.child!.onHotReload(), + ); + } + + static bool _isPendingAddOfLoadingOrLoadedChild(LifecycleEvent event) { + return event.kind == LifecycleEventKind.add && + (event.child!.isLoading || event.child!.isLoaded); } @mustCallSuper diff --git a/packages/flame/test/components/component_test.dart b/packages/flame/test/components/component_test.dart index 47287774f5a..53141b94a2b 100644 --- a/packages/flame/test/components/component_test.dart +++ b/packages/flame/test/components/component_test.dart @@ -1205,6 +1205,41 @@ void main() { expect(parent.parent, isNull); }, ); + + testWithFlameGame( + 'removing a queued sibling from onMount does not mount twice', + (game) async { + final sibling = _LifecycleComponent('sibling'); + final component = _SiblingRemovingOnMountComponent(sibling); + game.world.add(component); + game.world.add(sibling); + await game.ready(); + + expect(component.isMounted, true); + expect(component.countEvents('onMount'), 1); + expect(sibling.isMounted, false); + expect(sibling.parent, isNull); + expect(game.world.children, [component]); + }, + ); + + testWithFlameGame( + 're-adding a removing component from onMount keeps it in the tree', + (game) async { + final existing = _LifecycleComponent('existing'); + await game.world.ensureAdd(existing); + final component = _ReAddingOnMountComponent(existing); + game.world.add(component); + game.world.remove(existing); + await game.ready(); + + expect(component.isMounted, true); + expect(existing.isMounted, true); + expect(existing.isRemoving, false); + expect(existing.parent, game.world); + expect(game.world.children, containsAll([component, existing])); + }, + ); }); group('Moving components', () { @@ -2174,6 +2209,29 @@ class _SelfRemovingOnMountComponent extends Component { } } +class _SiblingRemovingOnMountComponent extends _LifecycleComponent { + _SiblingRemovingOnMountComponent(this.sibling) : super('remover'); + + final Component sibling; + + @override + void onMount() { + super.onMount(); + parent!.remove(sibling); + } +} + +class _ReAddingOnMountComponent extends Component { + _ReAddingOnMountComponent(this.component); + + final Component component; + + @override + void onMount() { + parent!.add(component); + } +} + class _Pair { _Pair(this.component, this.points); final Component component;