Skip to content

Commit

Permalink
refactor: Parent change and component removal logic (#1385)
Browse files Browse the repository at this point in the history 
* ProcessQueues() method

* Added deadQueue in LifecycleManager

* refactor removeFromParent()

* make shouldRemove non-overridable

* Added lifecycle state "removing"

* prevent double-removal

* shouldRemove can only be set to true

* eliminate _shouldRemove

* rename _dead -> _dying

* added adoption queue

* removed nextParent

* deprecate ComponentSet.clear and .removeAll

* ComponentSet no longer handles removal

* remove usages of shouldRemove

* doc-comments

* onRemove refactor

* cleanup

* remove obsolete paragraph in docs

* rename queue dying -> removals

* feat: update examples dashbook (#1398)

* docs: Added tutorial for creating a bare Flame project (#1376)

* feat: Add missing optional priority to SpriteBodyComponent (#1404)

* Add missing optional priority to SpriteBodyComponent

Gives you the option to set the priority directly when creating the component.

* Add optional parameter priority

Adds priority as an optional parameter

* removed wrong trailing comma

Added a comma at the wrong position.

* feat:  Added getImageLayer to flame_tiled (#1405)

* feat: Create sphinx extension for integrating Flutter apps into the documentation site (#1393)

* feat: adding FlameBloc mixin to allow its usage with enhanced FlameGame classes (#1399)

* feat: adding FlameBloc mixin to allow its usage with enhanced FlameGame classes

* fixing tests

* Apply suggestions from code review

Co-authored-by: Lukas Klingsbo <me@lukas.fyi>
Co-authored-by: Pasha Stetsenko <stpasha@google.com>
Co-authored-by: Luan Nico <luanpotter27@gmail.com>

Co-authored-by: Lukas Klingsbo <me@lukas.fyi>
Co-authored-by: Pasha Stetsenko <stpasha@google.com>
Co-authored-by: Luan Nico <luanpotter27@gmail.com>

* refactor: Organize tests in the game/ folder (#1403)

* position_type_test

* detectors_test

* reformat projections_test

* Reorganize tests in projector_test

* review flame_game_test

* move some cameratests

* created viewport_test file

* reformat camera tests

* feat: Add missing optional priority to SpriteBodyComponent (#1404)

* Add missing optional priority to SpriteBodyComponent

Gives you the option to set the priority directly when creating the component.

* Add optional parameter priority

Adds priority as an optional parameter

* removed wrong trailing comma

Added a comma at the wrong position.

* feat:  Added getImageLayer to flame_tiled (#1405)

* feat: Create sphinx extension for integrating Flutter apps into the documentation site (#1393)

Co-authored-by: KurtLa <KurtLa@users.noreply.github.com>
Co-authored-by: Munsterlander <munsterlander@users.noreply.github.com>
Co-authored-by: Erick <erickzanardoo@gmail.com>

* feat: improving generics on position body component (#1397)

* chore(release): publish packages (#1407)

- flame@1.1.0-releasecandidate.1
 - flame_bloc@1.2.0-releasecandidate.1
 - flame_rive@1.1.0-releasecandidate.1
 - flame_test@1.2.0-releasecandidate.1
 - flame_tiled@1.3.0-releasecandidate.1

* chore: fixing pub deps to allow publish (#1408)

* chore(release): publish packages

 - flame@1.1.0-releasecandidate.1
 - flame_bloc@1.2.0-releasecandidate.1
 - flame_rive@1.1.0-releasecandidate.1
 - flame_test@1.2.0-releasecandidate.1
 - flame_tiled@1.3.0-releasecandidate.1

* chore: fixing deps to enable pub publish

* fixing vector math version

* chore(flame_forge2d): export all files in barrel file (#1409)

* chore(release): publish packages (#1410)

- flame_forge2d@0.9.0-releasecandidate.1

* chore: commented out PR template sections (#1412)

* feat: Make ContactCallback begin end methods optional overrides (#1415)

* feat: made begin and end optional overrides

* chore: removed unecessary end override

* feat: Camera as a component (#1355)

* feat(collision detection)!: Use a broadphase to make collision detection more efficient (#1252)

* fix: PositionBodyComponent had an async onMount, without needing (#1424)

* fix: Fix collision detection comments and typo (#1422)

* Fix collision detection comments and typo

* Update packages/flame/lib/src/collisions/collision_callbacks.dart

Co-authored-by: Pasha Stetsenko <stpasha@google.com>

* Update doc/flame/collision_detection.md

Co-authored-by: Pasha Stetsenko <stpasha@google.com>

Co-authored-by: Pasha Stetsenko <stpasha@google.com>

* feat: adding has mounted to component (#1418)

* feat: adding has mounted to component

* feat: pr suggestions

* feat: improving hasMounted

* feat: renaming hasMounted to mounted

* feat: pr suggestion

* chore(release): publish packages (#1427)

- flame_svg@1.1.0-releasecandidate.1
 - flame@1.1.0-releasecandidate.2
 - flame_bloc@1.2.0-releasecandidate.2
 - flame_forge2d@0.9.0-releasecandidate.2
 - flame_rive@1.1.0-releasecandidate.2
 - flame_test@1.2.0-releasecandidate.2
 - flame_tiled@1.3.0-releasecandidate.2

* fix a test

* fix broken merge

* rename parent->owner in LifecycleManager

* update doc-comment

Co-authored-by: Erick <erickzanardoo@gmail.com>
Co-authored-by: KurtLa <KurtLa@users.noreply.github.com>
Co-authored-by: Munsterlander <munsterlander@users.noreply.github.com>
Co-authored-by: Lukas Klingsbo <me@lukas.fyi>
Co-authored-by: Luan Nico <luanpotter27@gmail.com>
Co-authored-by: Allison Ryan <77211884+allisonryan0002@users.noreply.github.com>
Co-authored-by: Alejandro Santiago <dev@alestiago.com>
  • Loading branch information
@st-pasha @erickzanardo
@KurtLa @munsterlander @spydon @luanpotter @allisonryan0002 @alestiago
8 people committed Mar 9, 2022
1 parent e30bf52 commit 8b9fa35
Show file tree
Hide file tree
Showing 14 changed files with 155 additions and 129 deletions.
10 changes: 0 additions & 10 deletions doc/flame/components.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,6 @@ if you wish.
The `onGameResize` method is called whenever the screen is resized, and once in the beginning when
the component is added to the game via the `add` method.

The `shouldRemove` variable can be overridden or set to true and `FlameGame` will remove the
component before the next update loop. It will then no longer be rendered or updated. Note that
`game.remove(Component c)` and `component.removeFromParent()` also can be used to remove components
from its parent.

The `respectCamera` variable can be overridden or set to `false` (defaults to `true`) to make the
`FlameGame` ignore the `camera` for this component, making it static in relation to the viewport
that is.
Do note that this currently only works if the component is added directly to the root `FlameGame`.

The `onRemove` method can be overridden to run code before the component is removed from the game,
it is only run once even if the component is removed both by using the parents remove method and
the `Component` remove method.
Expand Down
141 changes: 103 additions & 38 deletions packages/flame/lib/src/components/component.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,15 @@ class Component {
}

/// Whether this component is currently added to a component tree.
bool get isMounted => _state == LifecycleState.mounted;
bool get isMounted {
return (_state == LifecycleState.mounted) ||
(_state == LifecycleState.removing);
}

/// The current parent of the component, or null if there is none.
Component? get parent => _parent;
Component? _parent;

/// If the component should be added to another parent once it has been
/// removed from its current parent.
Component? nextParent;

/// The children of the current component.
///
/// This getter will automatically create the [ComponentSet] container within
Expand All @@ -57,7 +56,10 @@ class Component {
Completer<void>? _mountCompleter;

@protected
_LifecycleManager get lifecycle => _lifecycleManager ??= _LifecycleManager();
_LifecycleManager get lifecycle {
return _lifecycleManager ??= _LifecycleManager(this);
}

_LifecycleManager? _lifecycleManager;

/// Render priority of this component. This allows you to control the order in
Expand All @@ -77,7 +79,18 @@ class Component {
///
/// It will be checked once per component per tick, and if it is true,
/// FlameGame will remove it.
bool shouldRemove = false;
@nonVirtual
bool get shouldRemove => _state == LifecycleState.removing;

/// Setting [shouldRemove] to true will schedule the component to be removed
/// from the game tree before the next game cycle.
///
/// This property is equivalent to using the method [removeFromParent].
@nonVirtual
set shouldRemove(bool value) {
assert(value, '"Resurrecting" a component is not allowed');
removeFromParent();
}

/// Returns whether this [Component] is in debug mode or not.
/// When a child is added to the [Component] it gets the same [debugMode] as
Expand Down Expand Up @@ -206,6 +219,9 @@ class Component {
/// ```
void onMount() {}

/// Called right before the component is removed from the game.
void onRemove() {}

/// A future that will complete once the component is mounted on its parent
Future<void> get mounted {
if (isMounted) {
Expand Down Expand Up @@ -233,7 +249,7 @@ class Component {
/// priority of the direct siblings, not the children or the ancestors.
void updateTree(double dt) {
_children?.updateComponentList();
_lifecycleManager?.processChildrenQueue();
_lifecycleManager?.processQueues();
update(dt);
_children?.forEach((c) => c.updateTree(dt));
}
Expand Down Expand Up @@ -267,14 +283,15 @@ class Component {
}

/// Remove the component from its parent in the next tick.
void removeFromParent() => shouldRemove = true;
void removeFromParent() {
_parent?.remove(this);
}

/// Changes the current parent for another parent and prepares the tree under
/// the new root.
void changeParent(Component component) {
void changeParent(Component newParent) {
newParent.lifecycle._adoption.add(this);
_mountCompleter = null;
parent?.remove(this);
nextParent = component;
}

/// An iterator producing this component's parent, then its parent's parent,
Expand All @@ -290,16 +307,6 @@ class Component {
}
}

/// Called right before the component is removed from the game.
@mustCallSuper
void onRemove() {
_children?.forEach((child) => child.onRemove());
_state = LifecycleState.removed;
_parent = null;
nextParent?.add(this);
nextParent = null;
}

//#region Add/remove components

/// Schedules [component] to be added as a child to this component.
Expand Down Expand Up @@ -362,9 +369,9 @@ class Component {
debugMode |= parent.debugMode;
parent.lifecycle._children.add(this);

final root = parent.findGame();
if (root != null) {
if (!isLoaded) {
if (!isLoaded) {
final root = parent.findGame();
if (root != null) {
assert(
root.hasLayout,
'add() called before the game has a layout. Did you try to add '
Expand All @@ -378,14 +385,17 @@ class Component {

/// Removes a component from the component tree, calling [onRemove] for it and
/// its children.
void remove(Component c) {
_children?.remove(c);
void remove(Component component) {
if (component._state != LifecycleState.removing) {
lifecycle._removals.add(component);
component._state = LifecycleState.removing;
}
}

/// Removes all the children in the list and calls [onRemove] for all of them
/// and their children.
void removeAll(Iterable<Component> cs) {
_children?.removeAll(cs);
void removeAll(Iterable<Component> components) {
components.forEach(remove);
}

Future<void>? _load() {
Expand Down Expand Up @@ -430,13 +440,26 @@ class Component {
(child) => child._mount(parent: this, existingChild: true),
);
}
_lifecycleManager?.processChildrenQueue();
_lifecycleManager?.processQueues();
}

// TODO(st-pasha): remove this after #1351 is done
@internal
void setMounted() => _state = LifecycleState.mounted;

void _remove() {
_parent!.children.remove(this);
propagateToChildren(
(Component component) {
component.onRemove();
component._state = LifecycleState.removed;
component._parent = null;
return true;
},
includeSelf: true,
);
}

//#endregion

bool get hasPendingLifecycleEvents {
Expand All @@ -446,7 +469,7 @@ class Component {
/// Attempt to resolve any pending lifecycle events on this component.
void processPendingLifecycleEvents() {
if (_lifecycleManager != null) {
_lifecycleManager!.processChildrenQueue();
_lifecycleManager!.processQueues();
if (!_lifecycleManager!.hasPendingEvents) {
_lifecycleManager = null;
}
Expand Down Expand Up @@ -542,8 +565,8 @@ typedef ComponentSetFactory = ComponentSet Function();
///
/// The progression between states happens as follows:
/// ```
/// uninitialized -> loading -> loaded -> mounted -> removed -,
/// ^-----------------'
/// uninitialized -> loading -> loaded -> mounted -> removing -> removed -.
/// ^-----------------------------'
/// ```
///
/// Publicly visible flags `isLoaded` and `isMounted` are derived from this
Expand All @@ -565,6 +588,9 @@ enum LifecycleState {
/// its parent's `children` list.
mounted,

/// The component is scheduled to be removed on the next game tick.
removing,

/// The component which was mounted before, is now removed from its parent.
removed,
}
Expand All @@ -577,12 +603,17 @@ enum LifecycleState {
/// queues. Which is why these queues are placed into a separate class, so that
/// they can be easily disposed of at the end.
class _LifecycleManager {
/// Queues for adding children to a component.
_LifecycleManager(this.owner);

/// The component which is the owner of this [_LifecycleManager].
final Component owner;

/// Queue for adding children to a component.
///
/// When the user `add()`s a child to a component, we immediately place it
/// into the component's queue, and only after that do the standard lifecycle
/// into that component's queue, and only after that do the standard lifecycle
/// processing: resizing, loading, mounting, etc. After all that is finished,
/// the component is retrieved from the queue and placed into the parent's
/// the child component is retrieved from the queue and placed into the
/// children list.
///
/// Since the components are processed in the FIFO order, this ensures that
Expand All @@ -591,16 +622,31 @@ class _LifecycleManager {
/// finish loading in arbitrary order.
final Queue<Component> _children = Queue();

/// Queue for removing children from a component.
///
/// Components that were placed into this queue will be removed from [owner]
/// when the pending events are resolved.
final Queue<Component> _removals = Queue();

/// Queue for moving components from another parent to this one.
final Queue<Component> _adoption = Queue();

bool get hasPendingEvents {
return _children.isNotEmpty;
return !(_children.isEmpty && _removals.isEmpty && _adoption.isEmpty);
}

void processQueues() {
_processChildrenQueue();
_processRemovalQueue();
_processAdoptionQueue();
}

/// Attempt to resolve pending events in all lifecycle event queues.
///
/// This method must be periodically invoked by the game engine, in order to
/// ensure that the components get properly added/removed from the component
/// tree.
void processChildrenQueue() {
void _processChildrenQueue() {
while (_children.isNotEmpty) {
final child = _children.first;
assert(child.parent!.isMounted);
Expand All @@ -614,4 +660,23 @@ class _LifecycleManager {
}
}
}

void _processRemovalQueue() {
while (_removals.isNotEmpty) {
final component = _removals.removeFirst();
if (component.isMounted) {
component._remove();
}
assert(component._state == LifecycleState.removed);
}
}

void _processAdoptionQueue() {
while (_adoption.isNotEmpty) {
final child = _adoption.removeFirst();
child._remove();
child._parent = owner;
child._mount();
}
}
}
46 changes: 16 additions & 30 deletions packages/flame/lib/src/components/component_set.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ class ComponentSet extends QueryableOrderedSet<Component> {
// When we switch to Dart 2.15 this can be replaced with constructor tear-off
static ComponentSet createDefault() => ComponentSet();

/// Components to be removed on the next update.
///
/// The component list is only changed at the start of each update to avoid
/// concurrency issues.
final Set<Component> _removeLater = {};

/// Components whose priority changed since the last update.
///
/// When priorities change we need to re-balance the component set, but
Expand All @@ -46,6 +40,12 @@ class ComponentSet extends QueryableOrderedSet<Component> {
@override
bool add(Component component) => super.add(component);

/// Marked as internal, because the users shouldn't be able to remove elements
/// from the [ComponentSet] directly, bypassing the normal lifecycle handling.
@internal
@override
bool remove(Component component) => super.remove(component);

/// Prohibit method `addAll()` inherited from the [QueryableOrderedSet]. If
/// this was allowed, then the user would be able to bypass standard lifecycle
/// methods of the [Component] class.
Expand All @@ -58,27 +58,24 @@ class ComponentSet extends QueryableOrderedSet<Component> {
);
}

/// Marks the component to be removed on the next call to
/// `updateComponentList()`.
@override
bool remove(Component c) {
_removeLater.add(c);
return true;
}

/// Marks a list of components to be removed from the components list on the
/// next game tick. This will return the same list as sent in.
/// Prohibit method `removeAll()` inherited from the [QueryableOrderedSet]. If
/// this was allowed, then the user would be able to bypass standard lifecycle
/// methods of the [Component] class.
@Deprecated('Do not use')
@override
Iterable<Component> removeAll(Iterable<Component> components) {
_removeLater.addAll(components);
return components;
throw UnsupportedError(
'Removing elements directly from a ComponentSet is prohibited; use '
'Component.removeAll() instead',
);
}

/// Marks all existing components to be removed from the components list on
/// the next game tick.
@Deprecated('This method will be removed in 1.1.0')
@override
void clear() {
_removeLater.addAll(this);
forEach((element) => element.removeFromParent());
}

/// Whether the component set contains components or that there are components
Expand All @@ -93,17 +90,6 @@ class ComponentSet extends QueryableOrderedSet<Component> {
/// Note: do not call this while iterating the set.
void updateComponentList() {
_actuallyUpdatePriorities();
_actuallyRemove();
}

void _actuallyRemove() {
_removeLater.addAll(where((c) => c.shouldRemove));
_removeLater.forEach((c) {
c.onRemove();
super.remove(c);
c.shouldRemove = false;
});
_removeLater.clear();
}

@override
Expand Down
7 changes: 3 additions & 4 deletions packages/flame/lib/src/components/particle_component.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ class ParticleComponent extends Component {

ParticleComponent(this.particle);

/// This [ParticleComponent] will be removed by the FlameGame.
@override
bool get shouldRemove => particle.shouldRemove;

/// Returns progress of the child [Particle].
///
/// Could be used by external code if needed.
Expand All @@ -32,5 +28,8 @@ class ParticleComponent extends Component {
@override
void update(double dt) {
particle.update(dt);
if (particle.shouldRemove) {
removeFromParent();
}
}
}
Loading

0 comments on commit 8b9fa35

Please sign in to comment.