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

feat: Add collision completed listener #2896

Merged
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4b8bdd5
First attempt at creating onCollisionsCompleted
Nov 11, 2023
5d42eed
Bug fix which gets things working.
Nov 12, 2023
f4ab7b9
First attempt at adding tests
Nov 12, 2023
76d2970
Revert "First attempt at adding tests"
Nov 23, 2023
1216440
Revert "First attempt at creating onCollisionsCompleted"
Nov 23, 2023
e9dba44
Revert "Bug fix which gets things working."
Nov 23, 2023
d49d3da
Add ChangeNotifier to collision detection.
Nov 26, 2023
05db541
Add an override of the ChaneNotifier class and start documentation.
Nov 26, 2023
015f9bd
Add a description of new ChangeNotifier override.
Nov 26, 2023
92eb7c5
Remove changes from shape hitbox.
Nov 26, 2023
e6256db
Switch from ChangeNotifier to implementing Listenable.
Nov 27, 2023
09fec9c
Merge branch 'main' into add_collision_completed
Nov 29, 2023
21e8725
Update comment for Listener class
Dec 2, 2023
362ad63
Fix markdown lint failure
Dec 2, 2023
82b6745
Merge branch 'main' into add_collision_completed
christian-mindfulness Dec 3, 2023
9d74231
Update packages/flame/lib/src/collisions/collision_detection.dart
christian-mindfulness Dec 4, 2023
834c950
Change name of notifier, as requested in review
Dec 4, 2023
2b9c946
Notify class inherits from ChangeNotifier, rather than Listenable.
Dec 4, 2023
66879ca
Add first version of notifier test.
Dec 4, 2023
8e16121
Bug fix on flame test.
Dec 4, 2023
6b779c2
Fixes following @spydon's review.
Dec 6, 2023
13b3905
Fix change notifier.
Dec 6, 2023
2f9fc20
Update formatting in docs
Dec 6, 2023
a9308c9
Merge branch 'main' into add_collision_completed
spydon Dec 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 35 additions & 0 deletions doc/flame/collision_detection.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,41 @@ other way around. If there are no intersections with the edges on a solid hitbox
position is instead returned.


### Collision order

If a `PositionComponent` collides with more than one other object within a given time step, then
christian-mindfulness marked this conversation as resolved.
Show resolved Hide resolved
the `onCollision` callbacks will be called in an essentially random order. In some cases this can
be a problem, such as in a bouncing ball game where the trajectory of the ball can differ depending
on which other object was hit first. To help resolve this the `collisionsCompleted` listener can be
used - this triggers at the end of the collision detection process.

An example of how this might be used is to add a local variable in your `PositionComponent` to save
the other components with which this one is colliding:
christian-mindfulness marked this conversation as resolved.
Show resolved Hide resolved
`List<PositionComponent> collisionComponents = [];`. The `onCollision` callback is then used to
save all the other `PositionComponent`s to this list:

```dart
@override
void onCollision(Set<Vector2> intersectionPoints, PositionComponent other) {
collisionComponents.add(other);
super.onCollision(intersectionPoints, other);
}

```

Finally, one adds a listener to the `onLoad` method of the `PositionComponent` to call a routine
christian-mindfulness marked this conversation as resolved.
Show resolved Hide resolved
which will resolve how the collisions should be dealt with:

```dart
(game as HasCollisionDetection).collisionDetection.collisionsCompleted.
addListener(() {
resolveCollisions();
});
```

The list `collisionComponents` would need to be cleared in each call to `update`.


## ShapeHitbox

The `ShapeHitbox`s are normal components, so you add them to the component that you want to add
Expand Down
26 changes: 26 additions & 0 deletions packages/flame/lib/src/collisions/collision_detection.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'package:flame/collisions.dart';
import 'package:flame/components.dart';
import 'package:flame/geometry.dart';
import 'package:flutter/material.dart';

/// [CollisionDetection] is the foundation of the collision detection system in
/// Flame.
Expand All @@ -13,6 +14,7 @@ abstract class CollisionDetection<T extends Hitbox<T>,

List<T> get items => broadphase.items;
final _lastPotentials = <CollisionProspect<T>>[];
final collisionsCompleted = CollisionDetectionCompletionNotifier();
christian-mindfulness marked this conversation as resolved.
Show resolved Hide resolved

CollisionDetection({required this.broadphase});

Expand Down Expand Up @@ -62,6 +64,9 @@ abstract class CollisionDetection<T extends Hitbox<T>,
}
}
_updateLastPotentials(potentials);

// Let any listeners know that the collision detection step has completed
christian-mindfulness marked this conversation as resolved.
Show resolved Hide resolved
collisionsCompleted.notifyListeners();
}

final _lastPotentialsPool = <CollisionProspect<T>>[];
Expand Down Expand Up @@ -161,3 +166,24 @@ abstract class CollisionDetection<T extends Hitbox<T>,
List<RaycastResult<T>>? out,
});
}

/// Add a basic class which implements a Listener.
christian-mindfulness marked this conversation as resolved.
Show resolved Hide resolved
class CollisionDetectionCompletionNotifier implements Listenable {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
class CollisionDetectionCompletionNotifier implements Listenable {
class CollisionDetectionCompletionNotifier with ChangeNotifier {

Copy link
Member

Choose a reason for hiding this comment

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

No need to manually implement Listenable, check https://api.flutter.dev/flutter/foundation/ChangeNotifier-class.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the suggestion to implement Listenable came from you, to avoid the warning about unnecessary overrides. Happy to do whichever you think best.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, now I remember. 🤦‍♂️
I think it is probably better to go with with ChangeNotifier anyways, so that we don't have to do the re-implementation, and then we can just put an ignore lint lint above the overridden method.

final List<VoidCallback> _listenFunctions = [];

@override
void addListener(VoidCallback listener) {
_listenFunctions.add(listener);
}

@override
void removeListener(VoidCallback listener) {
_listenFunctions.remove(listener);
}

void notifyListeners() {
for (final callback in _listenFunctions) {
callback();
}
}
}