diff --git a/packages/flame/lib/src/components/core/component.dart b/packages/flame/lib/src/components/core/component.dart index 9959550c1fe..c8198d496d5 100644 --- a/packages/flame/lib/src/components/core/component.dart +++ b/packages/flame/lib/src/components/core/component.dart @@ -704,7 +704,7 @@ class Component { } else if (child._parent != null) { if (child.isRemoving) { game.dequeueRemove(child); - _clearRemovingBit(); + child._clearRemovingBit(); } game.enqueueMove(child, this); } else if (isMounted && !child.isMounted) { 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 6af18495705..f5b0b44e913 100644 --- a/packages/flame/lib/src/components/core/component_tree_root.dart +++ b/packages/flame/lib/src/components/core/component_tree_root.dart @@ -15,11 +15,15 @@ class ComponentTreeRoot extends Component { super.children, super.key, }) : queue = RecycledQueue(LifecycleEvent.new), - _blocked = {}; + _blocked = {}; @internal final RecycledQueue queue; - final Set _blocked; + + /// Components whose events are blocked during the current + /// [processLifecycleEvents] pass. Compared by identity, since [Component] + /// does not override equality. + final Set _blocked; late final Map _index = {}; Completer? _lifecycleEventsCompleter; @@ -147,8 +151,7 @@ class ComponentTreeRoot extends Component { for (final event in queue) { final child = event.child!; final parent = event.parent!; - if (_blocked.contains(identityHashCode(child)) || - _blocked.contains(identityHashCode(parent))) { + if (_blocked.contains(child) || _blocked.contains(parent)) { continue; } @@ -165,8 +168,8 @@ class ComponentTreeRoot extends Component { queue.removeCurrent(); repeatLoop = true; case LifecycleEventStatus.block: - _blocked.add(identityHashCode(child)); - _blocked.add(identityHashCode(parent)); + _blocked.add(child); + _blocked.add(parent); default: } } diff --git a/packages/flame/test/components/component_test.dart b/packages/flame/test/components/component_test.dart index 561242c8d9e..619aeee3ea2 100644 --- a/packages/flame/test/components/component_test.dart +++ b/packages/flame/test/components/component_test.dart @@ -1171,6 +1171,55 @@ void main() { expect(game.world.children.toList(), [parent, child]); expect(parent.children.toList(), isEmpty); }); + + testWithFlameGame( + 'moving a component that is scheduled for removal cancels the removal', + (game) async { + final parentA = Component()..addToParent(game.world); + final parentB = Component()..addToParent(game.world); + final child = _LifecycleComponent('child')..addToParent(parentA); + await game.ready(); + + child.removeFromParent(); + expect(child.isRemoving, true); + parentB.add(child); + expect(child.isRemoving, false); + await game.ready(); + + expect(child.isMounted, true); + expect(child.parent, parentB); + expect(parentA.hasChildren, false); + expect(child.countEvents('onRemove'), 1); + expect(child.countEvents('onMount'), 2); + }, + ); + + testWithFlameGame( + 'moving a removing child does not clear the removing state of the ' + 'new parent', + (game) async { + final parentA = Component()..addToParent(game.world); + final parentB = Component()..addToParent(game.world); + final child = Component()..addToParent(parentA); + await game.ready(); + + parentB.removeFromParent(); + child.removeFromParent(); + expect(parentB.isRemoving, true); + expect(child.isRemoving, true); + + // Re-parenting the removing child must clear the child's removing + // state, not the new parent's. + parentB.add(child); + expect(child.isRemoving, false); + expect(parentB.isRemoving, true); + await game.ready(); + + expect(parentB.isRemoved, true); + expect(parentB.isMounted, false); + expect(child.parent, parentB); + }, + ); }); group('Rebalancing components', () { @@ -1553,6 +1602,38 @@ void main() { _Pair(world, [Vector2(664, 216)]), ]); }); + + testWithFlameGame( + 'later siblings are hit first and can be removed during iteration', + (game) async { + final world = game.world; + final componentA = PositionComponent(size: Vector2.all(100)) + ..addToParent(world); + final componentB = PositionComponent(size: Vector2.all(100)) + ..addToParent(world); + final componentC = PositionComponent(size: Vector2.all(100)) + ..addToParent(world); + await game.ready(); + + // Mimics an event handler that mutates the tree while the hit-test + // iterable is being consumed, like a tap handler removing the + // tapped component and one of its siblings. + final visited = []; + for (final component in world.componentsAtPoint(Vector2(50, 50))) { + if (component == componentC) { + componentC.removeFromParent(); + componentB.removeFromParent(); + } + visited.add(component); + } + + // The removals are deferred, so the iteration still sees the tree + // as it was when the event was fired, in top-most-first order. + expect(visited, [componentC, componentB, componentA, world]); + await game.ready(); + expect(world.children.toList(), [componentA]); + }, + ); }); group('findRootGame()', () { diff --git a/packages/flame/test/components/priority_test.dart b/packages/flame/test/components/priority_test.dart index bf2c50db898..0828eb16f9c 100644 --- a/packages/flame/test/components/priority_test.dart +++ b/packages/flame/test/components/priority_test.dart @@ -25,6 +25,32 @@ void main() { }, ); + testWithFlameGame( + 'components with equal priority keep insertion order', + (game) async { + final components = List.generate(5, (_) => _PriorityComponent(0)); + await game.world.ensureAddAll(components); + expect(game.world.children.toList(), components); + }, + ); + + testWithFlameGame( + 'equal-priority components keep their relative order after a sibling ' + 'changes priority', + (game) async { + final a = _PriorityComponent(0); + final b = _PriorityComponent(0); + final c = _PriorityComponent(0); + final d = _PriorityComponent(1); + await game.world.ensureAddAll([a, b, c, d]); + expect(game.world.children.toList(), [a, b, c, d]); + + d.priority = -1; + game.update(0); + expect(game.world.children.toList(), [d, a, b, c]); + }, + ); + testWithFlameGame( 'changing priority with the priority setter should reorder the list', (game) async {