Skip to content

Commit

Permalink
feat: Add onDispose to game.dart called from game_widget.dart (#…
Browse files Browse the repository at this point in the history
…2659)

This adds, imo, the missing piece in the event lifecycle based on all the other lifecycle work I have been doing.

Currently, when dispose is called in game_widget.dart, it calls disposeCurrentGame which has the call for onRemove, at issue though is, disposeCurrentGame is also called by didUpdateWidget if the oldWidget.game != widget.game. It may not be needed, but I feel like because FlameGame is inherently a stateless set of widgets (children of the LeafRenderObjectWidget), the hook for onRemove is muddled with what is NOT actually a dispose event. So my thought is, introduce the onDispose method to game which gets called by making disposeCurrentGame({isDispose = false}) then in the actual overridden dispose method, pass that as true, so we can then call currentGame.onDispose() after the call to currentGame.onRemove().
  • Loading branch information
munsterlander committed Aug 20, 2023
1 parent f582f49 commit 2f44e48
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 2 deletions.
3 changes: 3 additions & 0 deletions packages/flame/lib/src/game/game.dart
Expand Up @@ -213,6 +213,9 @@ abstract mixin class Game {
/// do cleanups to avoid memory leaks.
void onRemove() {}

/// Called when the GameWidget is disposed by Flutter.
void onDispose() {}

/// Called after the game has left the widget tree.
/// This can be overridden to add logic that requires the game
/// not be on the flutter widget tree anymore.
Expand Down
10 changes: 8 additions & 2 deletions packages/flame/lib/src/game/game_widget/game_widget.dart
Expand Up @@ -254,8 +254,14 @@ class GameWidgetState<T extends Game> extends State<GameWidget<T>> {
_loaderFuture = null;
}

void disposeCurrentGame() {
/// [disposeCurrentGame] is called by two flutter events - `didUpdateWidget`
/// and `dispose`. When the parameter [callGameOnDispose] is true, the
/// `currentGame`'s `onDispose` method will be called; otherwise, it will not.
void disposeCurrentGame({bool callGameOnDispose = false}) {
currentGame.removeGameStateListener(_onGameStateChange);
if (callGameOnDispose) {
currentGame.onDispose();
}
}

@override
Expand All @@ -281,7 +287,7 @@ class GameWidgetState<T extends Game> extends State<GameWidget<T>> {
@override
void dispose() {
super.dispose();
disposeCurrentGame();
disposeCurrentGame(callGameOnDispose: true);
// If we received a focus node from the user, they are responsible
// for disposing it
if (widget.focusNode == null) {
Expand Down
Expand Up @@ -30,6 +30,12 @@ class _MyGame extends FlameGame {
super.onRemove();
events.add('onRemove');
}

@override
void onDispose() {
super.onDispose();
events.add('onDispose');
}
}

class _TitlePage extends StatelessWidget {
Expand Down Expand Up @@ -185,6 +191,11 @@ void main() {
true,
reason: 'onRemove was not called',
);
expect(
events.contains('onDispose'),
true,
reason: 'onDispose was not called',
);
});

testWidgets('on resize, parents are kept', (tester) async {
Expand Down Expand Up @@ -229,6 +240,7 @@ void main() {
'onLoad',
'onMount',
'onRemove',
'onDispose',
'onGameResize',
'onMount',
],
Expand Down

0 comments on commit 2f44e48

Please sign in to comment.