Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: Separate ComponentSet from the Component #1266

Merged
merged 9 commits into from
Jan 14, 2022
31 changes: 25 additions & 6 deletions packages/flame/lib/src/components/component.dart
Original file line number Diff line number Diff line change
Expand Up @@ -238,21 +238,39 @@ class Component with Loadable {
/// only be called after the game already has its layout set, this can be
/// verified by the [Game.hasLayout] property, to add components upon game
/// initialization, the [onLoad] method can be used instead.
Future<void> add(Component component) {
return children.addChild(component);
Future<void> add(Component component) async {
component.prepare(/*parent=*/ this);
if (component.isPrepared) {
// [Component.onLoad] (if it is defined) should only run the first time
// the component is added to a parent.
if (!component.isLoaded) {
final onLoad = component.onLoadCache;
if (onLoad != null) {
await onLoad;
}
component.isLoaded = true;
}
// Should run every time the component gets a new parent, including its
// first parent.
component.onMount();
if (component.hasChildren) {
await component._reAddChildren();
}
}
children.addChild(component);
}

/// Adds multiple children.
///
/// See [add] for details.
Future<void> addAll(Iterable<Component> components) {
return children.addChildren(components);
return Future.wait(components.map(add));
}

/// The children are added again to the component set so that [prepare],
/// [onLoad] and [onMount] runs again. Used when a parent is changed
/// further up the tree.
Future<void> reAddChildren() async {
Future<void> _reAddChildren() async {
if (_children != null) {
await Future.wait(_children!.map(add));
await Future.wait(_children!.addLater.map(add));
Expand Down Expand Up @@ -335,6 +353,7 @@ class Component with Loadable {
/// If there are no parents that are a [Game] false will be returned and this
/// will run again once an ancestor or the component itself is added to a
/// [Game].
@protected
@mustCallSuper
void prepare(Component parent) {
_parent = parent;
Expand Down Expand Up @@ -364,7 +383,7 @@ class Component with Loadable {
/// This method creates the children container for the current component.
/// Override this method if you need to have a custom [ComponentSet] within
/// a particular class.
ComponentSet createComponentSet() => childrenFactory(this);
ComponentSet createComponentSet() => childrenFactory();
}

typedef ComponentSetFactory = ComponentSet Function(Component owner);
typedef ComponentSetFactory = ComponentSet Function();
135 changes: 39 additions & 96 deletions packages/flame/lib/src/components/component_set.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'dart:collection';

import 'package:meta/meta.dart';
import 'package:ordered_set/comparing.dart';
import 'package:ordered_set/queryable_ordered_set.dart';

Expand All @@ -18,6 +19,19 @@ import '../../game.dart';
/// This wrapper also guaranteed that [Component.prepare], [Loadable.onLoad]
/// and all the lifecycle methods are called properly.
class ComponentSet extends QueryableOrderedSet<Component> {
/// With default settings, creates a [ComponentSet] with the compare function
/// that uses the Component's priority for sorting.
ComponentSet({
int Function(Component e1, Component e2)? comparator,
bool? strictMode,
}) : super(
comparator: comparator ?? Comparing.on<Component>((c) => c.priority),
strictMode: strictMode ?? defaultStrictMode,
);

// When we switch to Dart 2.15 this can be replaced with constructor tear-off
static ComponentSet createDefault() => ComponentSet();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed/used anywhere? The reason to have it before was bc it was actually configuring the created set. Now you could just call ComponentSet() directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is used by Component.childrenFactory. Somehow using a simple function comprehension () => ComponentSet() produces an analyzer warning there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently, they fixed the warning just 8 days ago: dart-lang/linter#3118, but it will probably take time until we can actually use that.


/// Components to be added on the next update.
///
/// The component list is only changed at the start of each update to avoid
Expand All @@ -36,96 +50,41 @@ class ComponentSet extends QueryableOrderedSet<Component> {
/// we can only do that after each update to avoid concurrency issues.
final Set<Component> _changedPriorities = {};

/// This is the "prepare" function that will be called *before* the
/// component is added to the component list by the add/addAll methods.
/// It is also called when the component changes parent.
final Component parent;

static bool defaultStrictMode = false;

ComponentSet(
int Function(Component e1, Component e2)? comparator,
this.parent, {
bool? strictMode,
}) : super(
comparator: comparator,
strictMode: strictMode ?? defaultStrictMode,
);
/// Registers the component to be added on the next call to
/// `updateComponentList()`.
@internal
void addChild(Component component) {
_addLater.add(component);
}

/// Prepares and registers one component to be added on the next game tick.
///
/// This is the interface compliant version; if you want to provide an
/// explicit gameRef or await for the [Loadable.onLoad], use [addChild].
///
/// Note: the component is only added on the next tick. This method always
/// returns true.
/// Prohibit method `add()` 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
bool add(Component c) {
addChild(c);
return true;
throw UnsupportedError(
'Adding elements directly to a ComponentSet is prohibited; use '
'Component.add() instead',
);
}

/// Prepares and registers a list of components to be added on the next game
/// tick.
///
/// This is the interface compliant version; if you want to provide an
/// explicit gameRef or await for the [Loadable.onLoad], use [addChild].
///
/// Note: the components are only added on the next tick. This method always
/// returns the total length of the provided list.
/// 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.
@Deprecated('Do not use')
@override
int addAll(Iterable<Component> components) {
addChildren(components);
return components.length;
}

/// Prepares and registers one component to be added on the next game tick.
///
/// This allows you to provide a specific gameRef if this component is being
/// added from within another component that is already on a FlameGame.
/// You can await for the onLoad function, if present.
/// This method can be considered sync for all intents and purposes if no
/// onLoad is provided by the component.
Future<void> addChild(Component component) async {
component.prepare(parent);
if (!component.isPrepared) {
// Since the components won't be added until a proper game is added
// further up in the tree we can add them to the _addLater list and
// then re-add them once there is a proper root.
_addLater.add(component);
return;
}
// [Component.onLoad] (if it is defined) should only run the first time that
// a component is added to a parent.
if (!component.isLoaded) {
final onLoad = component.onLoadCache;
if (onLoad != null) {
await onLoad;
}
component.isLoaded = true;
}

// Should run every time the component gets a new parent, including its
// first parent.
component.onMount();
if (component.hasChildren) {
await component.reAddChildren();
}

_addLater.add(component);
}

/// Prepares and registers a list of component to be added on the next game
/// tick.
///
/// See [addChild] for more details.
Future<void> addChildren(Iterable<Component> components) async {
final ps = components.map(addChild);
await Future.wait(ps);
int addAll(Iterable<Component> c) {
throw UnsupportedError(
'Adding elements directly to a ComponentSet is prohibited; use '
'Component.addAll() instead',
);
}

/// Marks a component to be removed from the components list on the next game
/// tick.
/// Marks the component to be removed on the next call to
/// `updateComponentList()`.
@override
bool remove(Component c) {
_removeLater.add(c);
Expand Down Expand Up @@ -242,20 +201,4 @@ class ComponentSet extends QueryableOrderedSet<Component> {
parents.forEach((parent) => parent.reorderChildren());
_changedPriorities.clear();
}

/// Creates a [ComponentSet] with a default value for the compare function,
/// using the Component's priority for sorting.
///
/// You must provide the parent so that it can be handed to the children that
/// will be added.
static ComponentSet createDefault(
Component parent, {
bool? strictMode,
}) {
return ComponentSet(
Comparing.on<Component>((c) => c.priority),
parent,
strictMode: strictMode ?? defaultStrictMode,
);
}
}
10 changes: 5 additions & 5 deletions packages/flame/test/components/component_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -157,16 +157,16 @@ void main() {
);

test('childrenFactory', () {
Component.childrenFactory = (Component owner) {
return ComponentSet.createDefault(owner, strictMode: false);
};
final component0 = Component();
expect(component0.children.strictMode, false);

Component.childrenFactory = () => ComponentSet(strictMode: true);
final component1 = Component();
final component2 = Component();
component1.add(component2);
component2.add(Component());
expect(component1.children.strictMode, false);
expect(component2.children.strictMode, false);
expect(component1.children.strictMode, true);
expect(component2.children.strictMode, true);
});
});
}