The onRemove of game is be called when game is running.

[MiaoShichang]

Current bug behavior

The onRemove of game is be called when game is running.

Expected behavior

Don't be called;

Steps to reproduce

this is test code

import 'dart:async';

import 'package:flame/components.dart';
import 'package:flame/events.dart';
import 'package:flame/game.dart';
import 'package:flutter/material.dart';

///  flame:^1.8.2

void main() {
  runApp(const GameWidget.controlled(gameFactory: TestWidthGame.new));
}

// with TapCallbacks
class TestFirstZone extends PositionComponent with TapCallbacks {
  TestFirstZone({super.position, super.size}) : super(anchor: Anchor.center);
  @override
  void render(Canvas canvas) {
    super.render(canvas);
    canvas.drawRect(Rect.fromLTWH(0, 0, width, height), Paint()..color = const Color(0xff9999cc));
  }
}

// with null
class TestSecondZone extends PositionComponent {
  TestSecondZone({super.position, super.size}) : super(anchor: Anchor.center);
  @override
  void render(Canvas canvas) {
    super.render(canvas);
    canvas.drawRect(Rect.fromLTWH(0, 0, width, height), Paint()..color = const Color(0xffff66cc));
  }
}

class TestWidthGame extends FlameGame {
  @override
  Color backgroundColor() {
    return const Color(0xff9966cc);
  }

  @override
  FutureOr<void> onLoad() {
    // first
    add(TestFirstZone(position: size / 2, size: Vector2(160, 160)));

    // second
    // add(TestSecondZone(position: size / 2, size: Vector2(160, 160)));

    super.onLoad();
  }

  @override
  void onMount() {
    super.onMount();
    print("onMount");
  }

  @override
  void onRemove() {
    super.onRemove();
    // why this method is be called when only add TestFirstZone in onLoad method,
    // but not when only add TestSecondZone ?
    print("onRemove");
  }
}

this is output

--
Launching lib/main.dart on POT AL00a in debug mode...
Running Gradle task 'assembleDebug'...
✓ Built build/app/outputs/flutter-apk/app-debug.apk.
Debug service listening on ws://127.0.0.1:59555/2h5VAvqcp8c=/ws
Syncing files to device POT AL00a...
I/flutter ( 9136): onMount
W/Gralloc3( 9136): mapper 3.x is not supported
I/HwViewRootImpl( 9136): removeInvalidNode jank list is null
I/flutter ( 9136): onRemove
D/AwareBitmapCacher( 9136): handleInit switch not opened pid=9136

[spydon]

@munsterlander this seems like it is due to detach being called, haven't had any time to investigate why detach is being called though, could you have a look?

[munsterlander]

Will do today

[munsterlander]

Ok, sorry, I could have answered this if I had looked at the code closer, but anyway, I got it all mocked up, running, then immediately realized why and it is technically the correct call and not a bug. When you add a gesture callback (tap in this case), when the game first loads, if (game.firstChild<MultiTapDispatcher>() == null) { then it adds that gesture detector which forces the game to briefly unload and reload as it requires a different parent structure for the gesture detectors.

Now, @spydon we could continue that conversation about what constitutes a true onRemove, but as is, this is correct behavior because the game is being unloaded and then reloaded with the GestureDetector being the first child.

The code that matters - game.dart lines 356+:

  /// When a Game is attached to a `GameWidget`, this method will force that
  /// widget to be rebuilt. This can be used when updating any property which is
  /// implemented within the Flutter tree.
  @internal
  void refreshWidget() {
    gameStateListeners.forEach((callback) => callback());
  }

Finally, the GestureDetectorBuilder in game.dart that calls the refresh which is called by has_tappable_components.dart which is called by tap_callbacks.dart:

  /// Used internally by various mixins that need to use gesture detection
/// functionality in Flutter.
late final GestureDetectorBuilder gestureDetectors =
    GestureDetectorBuilder(refreshWidget)..initializeGestures(this);

So in closing, basically when it initially gets added, it causes the widget to be rebuilt, which is why you see onRemove being called, because it is actually being removed and reloaded. Now, this happens only when you add the component that uses a gesture, BUT in my opinion, this should not change BECAUSE if you needed to know something about state in your game prior to adding a gesture, this gives you that opportunity to do that.

I would apologize up front for the long ramble, but hopefully Spydon can clean up my rambles, lol.

[spydon]

Ok, sorry, I could have answered this if I had looked at the code closer, but anyway, I got it all mocked up, running, then immediately realized why and it is technically the correct call and not a bug. When you add a gesture callback (tap in this case), when the game first loads, if (game.firstChild<MultiTapDispatcher>() == null) { then it adds that gesture detector which forces the game to briefly unload and reload as it requires a different parent structure for the gesture detectors.

Now, @spydon we could continue that conversation about what constitutes a true onRemove, but as is, this is correct behavior because the game is being unloaded and then reloaded with the GestureDetector being the first child.

The code that matters - game.dart lines 356+:

  /// When a Game is attached to a `GameWidget`, this method will force that
  /// widget to be rebuilt. This can be used when updating any property which is
  /// implemented within the Flutter tree.
  @internal
  void refreshWidget() {
    gameStateListeners.forEach((callback) => callback());
  }

Finally, the GestureDetectorBuilder in game.dart that calls the refresh which is called by has_tappable_components.dart which is called by tap_callbacks.dart:

  /// Used internally by various mixins that need to use gesture detection
/// functionality in Flutter.
late final GestureDetectorBuilder gestureDetectors =
    GestureDetectorBuilder(refreshWidget)..initializeGestures(this);

So in closing, basically when it initially gets added, it causes the widget to be rebuilt, which is why you see onRemove being called, because it is actually being removed and reloaded. Now, this happens only when you add the component that uses a gesture, BUT in my opinion, this should not change BECAUSE if you needed to know something about state in your game prior to adding a gesture, this gives you that opportunity to do that.

I would apologize up front for the long ramble, but hopefully Spydon can clean up my rambles, lol.

Aaah of course, then it all makes sense and it should not be marked as a bug. We should document this better in the lifecycle section of the docs.

[munsterlander]

I am about half way through with the tile tutorial. I can knock out a doc update with another quick PR. I will be working on Flame all day tomorrow.

[MiaoShichang]

Ok, sorry, I could have answered this if I had looked at the code closer, but anyway, I got it all mocked up, running, then immediately realized why and it is technically the correct call and not a bug. When you add a gesture callback (tap in this case), when the game first loads, if (game.firstChild<MultiTapDispatcher>() == null) { then it adds that gesture detector which forces the game to briefly unload and reload as it requires a different parent structure for the gesture detectors.

Now, @spydon we could continue that conversation about what constitutes a true onRemove, but as is, this is correct behavior because the game is being unloaded and then reloaded with the GestureDetector being the first child.

The code that matters - game.dart lines 356+:

  /// When a Game is attached to a `GameWidget`, this method will force that
  /// widget to be rebuilt. This can be used when updating any property which is
  /// implemented within the Flutter tree.
  @internal
  void refreshWidget() {
    gameStateListeners.forEach((callback) => callback());
  }

Finally, the GestureDetectorBuilder in game.dart that calls the refresh which is called by has_tappable_components.dart which is called by tap_callbacks.dart:

  /// Used internally by various mixins that need to use gesture detection
/// functionality in Flutter.
late final GestureDetectorBuilder gestureDetectors =
    GestureDetectorBuilder(refreshWidget)..initializeGestures(this);

So in closing, basically when it initially gets added, it causes the widget to be rebuilt, which is why you see onRemove being called, because it is actually being removed and reloaded. Now, this happens only when you add the component that uses a gesture, BUT in my opinion, this should not change BECAUSE if you needed to know something about state in your game prior to adding a gesture, this gives you that opportunity to do that.

I would apologize up front for the long ramble, but hopefully Spydon can clean up my rambles, lol.

---

i think is not good action, i have a mixin that do something ,but now it is not correct for me that game with mixin (HasEventMessageHandler)

here is code

mixin HasEventMessageHandler on Component {

//
//

@override
void onMount() {
  super.onMount();
  List<int> messageList = [];
  registerEventMessage(messageList);
  if (messageList.isNotEmpty) {
    final game = findGame()!;
    if (game is HasEventMessageCenter) {
      for (var msgType in messageList) {
        game.mMessageManager.addListener(msgType, onEventMessage);
      }
    }
  }
}

@override
void onRemove() {
  super.onRemove();
  List<int> messageList = [];
  registerEventMessage(messageList);
  if (messageList.isNotEmpty) {
    final game = findGame()!;
    if (game is HasEventMessageCenter) {
      for (var msgType in messageList) {
        game.mMessageManager.remove(onEventMessage, msgType: msgType);
      }
    }
  }
}
}

[munsterlander]

As I was saying, there is a conversation to be had about this, but technically, onRemove as is, is correct, because the widget that contains the FlameGame is being removed and replaced. I can think of a couple of ways to do this, but the easiest would likely be to set a flag like this:

import 'dart:async';

import 'package:flame/components.dart';
import 'package:flame/events.dart';
import 'package:flame/game.dart';
import 'package:flutter/material.dart';

///  flame:^1.8.2

void main() {
  runApp(const GameWidget.controlled(gameFactory: TestWidthGame.new));
}

// with TapCallbacks
class TestFirstZone extends PositionComponent with TapCallbacks {
  TestFirstZone({super.position, super.size}) : super(anchor: Anchor.center);
  @override
  void render(Canvas canvas) {
    super.render(canvas);
    canvas.drawRect(Rect.fromLTWH(0, 0, width, height),
        Paint()..color = const Color(0xff9999cc));
  }
}

class TestWidthGame extends FlameGame {
  bool gestureComponentJustAdded = false;

  @override
  Color backgroundColor() {
    return const Color(0xff9966cc);
  }

  @override
  FutureOr<void> onLoad() {
    gestureComponentJustAdded = true;
    add(TestFirstZone(position: size / 2, size: Vector2(160, 160)));
    super.onLoad();
  }

  @override
  void onRemove() {
    super.onRemove();
    if (!gestureComponentJustAdded) {
      print("onRemove");
    } else {
      gestureComponentJustAdded = false;
    }
  }
}

So just basically wrap that block in your onRemove. Another way would be to check if the first child is not the multi-gesture detector and a flag or such.

Regardless though, you should know that your game is being removed to only be added back but with the gesture detector and it is this way specifically for your use case. As your demo code had two components and if you needed to know what your game was doing before the gesture detector was actually added, then now you have an event hook to do that.

[MiaoShichang]

As I was saying, there is a conversation to be had about this, but technically, onRemove as is, is correct, because the widget that contains the FlameGame is being removed and replaced. I can think of a couple of ways to do this, but the easiest would likely be to set a flag like this:

import 'dart:async';

import 'package:flame/components.dart';
import 'package:flame/events.dart';
import 'package:flame/game.dart';
import 'package:flutter/material.dart';

///  flame:^1.8.2

void main() {
  runApp(const GameWidget.controlled(gameFactory: TestWidthGame.new));
}

// with TapCallbacks
class TestFirstZone extends PositionComponent with TapCallbacks {
  TestFirstZone({super.position, super.size}) : super(anchor: Anchor.center);
  @override
  void render(Canvas canvas) {
    super.render(canvas);
    canvas.drawRect(Rect.fromLTWH(0, 0, width, height),
        Paint()..color = const Color(0xff9999cc));
  }
}

class TestWidthGame extends FlameGame {
  bool gestureComponentJustAdded = false;

  @override
  Color backgroundColor() {
    return const Color(0xff9966cc);
  }

  @override
  FutureOr<void> onLoad() {
    gestureComponentJustAdded = true;
    add(TestFirstZone(position: size / 2, size: Vector2(160, 160)));
    super.onLoad();
  }

  @override
  void onRemove() {
    super.onRemove();
    if (!gestureComponentJustAdded) {
      print("onRemove");
    } else {
      gestureComponentJustAdded = false;
    }
  }
}

So just basically wrap that block in your onRemove. Another way would be to check if the first child is not the multi-gesture detector and a flag or such.

Regardless though, you should know that your game is being removed to only be added back but with the gesture detector and it is this way specifically for your use case. As your demo code had two components and if you needed to know what your game was doing before the gesture detector was actually added, then now you have an event hook to do that.

---

i think is not good action, i have a mixin that do something ,but now it is not correct for me that game with mixin (HasEventMessageHandler)

here is code

mixin HasEventMessageHandler on Component {

//
//

@override
void onMount() {
  super.onMount();
  List<int> messageList = [];
  registerEventMessage(messageList);
  if (messageList.isNotEmpty) {
    final game = findGame()!;
    if (game is HasEventMessageCenter) {
      for (var msgType in messageList) {
        game.mMessageManager.addListener(msgType, onEventMessage);
      }
    }
  }
}

@override
void onRemove() {
  super.onRemove();
  List<int> messageList = [];
  registerEventMessage(messageList);
  if (messageList.isNotEmpty) {
    final game = findGame()!;
    if (game is HasEventMessageCenter) {
      for (var msgType in messageList) {
        game.mMessageManager.remove(onEventMessage, msgType: msgType);
      }
    }
  }
}
}

---

how can handle this if i have multiple mixin for game ,

doc says :
/// See also:
/// - [onRemove] that is called every time the component is removed from the
/// game tree
void onMount() {}

/// Called right before the component is removed from the game.
///
/// This method will only run for a component that was previously mounted into
/// a component tree. If a component was never mounted (for example, when it
/// is removed before it had a chance to mount), then this callback will not
/// trigger. Thus, [onRemove] runs if and only if there was a corresponding
/// [onMount] call before.
void onRemove() {}

[munsterlander]

What are your wanting to achieve? That may be easier to guide you than discussing the event lifecycle as all those components are getting mounted and being removed, so the calls are right.

[MiaoShichang]

What are your wanting to achieve? That may be easier to guide you than discussing the event lifecycle as all those components are getting mounted and being removed, so the calls are right.

---

i want that don't call onRmove after calling onMount in flamegame,
or if flamegaem need to reload, you should call onMount after calling onRemove again.

Now i have a mixin on Component, the mixin overright onMount and onRemove, i add some data in onMount mixin and remove it in onRemove.
if flamegame with mixin, the onRemove is being called after calling onMOunt when game is running, the data i adding data will be removed.

[munsterlander]

As I said, just add a boolean check with a check of the first child as a multi gesture detector is null, the your on remove code isn't called.

Please understand the lifecycle is correct. When you add a gesture detector, if it hasn't been loaded before, the game has to unload and reload with the gesture detector component as the parent, so on remove and on mount are the correct calls.

You just need to figure out a flag that works for your specific code needs.

Edit: To be clear, onRemove is not being called after onMount. It's being called when you add a tappable component for the first time, as the game needs to reload and instantiate the gesture detector.

[MiaoShichang]

sorry , i think lifecycle is wrong now.

if game need to unload and reload the component, ok, you can do unload and reload, why you call onRemove when call unload and not call onMount when reload again?

the game lefecycle is: onLoad -> onGameResize -> onMount ->Update -> onRemove .
but now the onRemove being called and onMount not when the game unload and reload.

The result is : onRemove will be called twice and onMount once for game like my code.

why don't call onMount when reload the game?

and, i think it will affects the mixin that override onMount and onRemove and the game with this mixin.

for example：

mixin ListenerHandler on Component {
  @override
  void onMount() {
    super.onMount();
    // add listener
  }

  @override
  void onRemove() {
    super.onRemove();
    // remove listener
  }
}

class MyGame extends FlameGame with ListenerHandler {
  // now the listener can't correctly work after unloaded and reloaded
  // because the onRemove will be called twice.
}

if i have on one mixin i can handle it ,but if i have a lot of mixin like this, i will do more work and possiblly make something wrong for some person don't know this rule.

[munsterlander]

Well, the issue comes down to lifecycle event names. The reason onMount is not called again, is because it was already mounted and it was never unMounted to the widget "representative" parent. It was removed though as it was physically removed from the tree. Thus when it comes back it needs to be attached back to the Flutter widget tree.

So I have tried explaining this multiple times and I never really do a good job with it, but understand that the stateful Game Widget is actually a child to a stateless Flutter widget; because it is stateless, there are only two events - attach and detach. You have to use the detach event to then fake a remove event. That is the inherent problem with Flutter if you do not add Gesture Detectors immediately in your game. You have to destroy the initial widget to then recreate a widget that has the Flutter (not Flame) detectors associated with it. Your FlameGame still exists in memory, so nothing should change in it, but we need to pop it from the widget, store it temporarily, destroy the widget, add a new widget with the gesture detectors attached to it, and then restore your FlameGame. The games state is still identical, BUT its important to pass that remove event into the FlameGame in case any user would need a hook into that event lifecycle exactly at that moment.

I hope that makes sense and as I was saying, its not wrong and its not a bug. It is just the way Flutter and Flame work.

Edit: Again, all you need is a flag to capture when you first add a tappable component. After that, you don't need to worry about it unless you are removing gesture detectors.

[spydon]

The games state is still identical, BUT its important to pass that remove event into the FlameGame in case any user would need a hook into that event lifecycle exactly at that moment.

I know you have explained this earlier, but what was the reason that we didn't call just onDetach instead of onRemove? Because I can agree with OP that the lifecycle now feels inconsistent and you can't use it properly without having to do checks on the state in there.

[MiaoShichang]

Well, the issue comes down to lifecycle event names. The reason onMount is not called again, is because it was already mounted and it was never unMounted to the widget "representative" parent. It was removed though as it was physically removed from the tree. Thus when it comes back it needs to be attached back to the Flutter widget tree.

So I have tried explaining this multiple times and I never really do a good job with it, but understand that the stateful Game Widget is actually a child to a stateless Flutter widget; because it is stateless, there are only two events - attach and detach. You have to use the detach event to then fake a remove event. That is the inherent problem with Flutter if you do not add Gesture Detectors immediately in your game. You have to destroy the initial widget to then recreate a widget that has the Flutter (not Flame) detectors associated with it. Your FlameGame still exists in memory, so nothing should change in it, but we need to pop it from the widget, store it temporarily, destroy the widget, add a new widget with the gesture detectors attached to it, and then restore your FlameGame. The games state is still identical, BUT its important to pass that remove event into the FlameGame in case any user would need a hook into that event lifecycle exactly at that moment.

I hope that makes sense and as I was saying, its not wrong and its not a bug. It is just the way Flutter and Flame work.

Edit: Again, all you need is a flag to capture when you first add a tappable component. After that, you don't need to worry about it unless you are removing gesture detectors.

i know what you said.
may i ask a question? why don't add the Gesture Detectors immediately when creating the Game Widget?
i guess every game needs Gesture Detectors. is it right?

[spydon]

i know what you said. may i ask a question? why don't add the Gesture Detectors immediately when creating the Game Widget? i guess every game needs Gesture Detectors. is it right?

They do not, there are different types of input systems that can be used, like keyboard and gamepads for example.
So with this system Flame dynamically adds only the detectors that it needs.

[MiaoShichang]

i think that the falmegame is a component and it is should do FOLLOW the lifecycle of component.
Otherwise, it will make people feel confused, like me. That's what I want to express.

every response is good, thanks !

[munsterlander]

The games state is still identical, BUT its important to pass that remove event into the FlameGame in case any user would need a hook into that event lifecycle exactly at that moment.

I know you have explained this earlier, but what was the reason that we didn't call just onDetach instead of onRemove? Because I can agree with OP that the lifecycle now feels inconsistent and you can't use it properly without having to do checks on the state in there.

Mainly because that event did not exist but I would say additionally because it's being removed but not disposed. This goes back to the stateful vs stateless widget event names and the intermixing that was done previously.

For v2, I agree, there should be more events and it should be clearer, otherwise if it was done now, it would be a pretty huge breaking change. I can do it now though and we can do some testing around it.

Edit: For me though, onDetach feels slightly off as well, because in this instance, the game is being removed. I don't know, I would need to think like what all events would you have because the intermixing is confusing. Stateful / stateless and Flutter widget tree / Flame component tree.

[munsterlander]

i think that the falmegame is a component and it is should do FOLLOW the lifecycle of component. Otherwise, it will make people feel confused, like me. That's what I want to express.

every response is good, thanks !

I like to call FlameGame a quasi-component because yes it extends a component, but not really, as it's also with Game class and the two have some overlap that clash imo.

[spydon]

Mainly because that event did not exist but I would say additionally because it's being removed but not disposed.

Detach does not mean dispose though, detach is exactly what it does here and then re-attaches again.

For v2, I agree, there should be more events and it should be clearer, otherwise if it was done now, it would be a pretty huge breaking change. I can do it now though and we can do some testing around it.

To do a onDetach call instead of onRemove wouldn't be that huge breaking change, since the onRemove call was just added in the last version and I don't think people have started using it yet. onDetach is also better to use since it isn't related to the Component lifecycle, so it doesn't interfere with that, but onAttach also has to be called when it comes back, similarly to how onMount should need to be called after onRemove has been called like OP mentioned, otherwise the state machine is broken.

Edit: For me though, onDetach feels slightly off as well, because in this instance, the game is being removed. I don't know, I would need to think like what all events would you have because the intermixing is confusing. Stateful / stateless and Flutter widget tree / Flame component tree.

I don't think the user needs to know when the widget is being re-attached when it is for changes like adding detectors, that should be an internal implementation detail for us to solve.

[spydon]

I like to call FlameGame a quasi-component because yes it extends a component, but not really, as it's also with Game class and the two have some overlap that clash imo.

The FlameGame is actually a full blown component, it can be added anywhere in the component tree, it doesn't have to be in the root. When it is in the root it has more responsibilities though.

[munsterlander]

The FlameGame is actually a full blown component, it can be added anywhere in the component tree, it doesn't have to be in the root. When it is in the root it has more responsibilities though.

Agreed, but that last sentence is why I say a quasi-component, because when it is root, it has a different behavior set. A lot of the internal methods just act differently then if it was a regular component in the list of children. Even embedded games within games, the FlameGame instance acts differently because child components can be attached to it, thus it then gets extended by Game, which then means many other things, but I digress.

FlameGame is a component (just with quirks) - :o)

[MiaoShichang]

I like to call FlameGame a quasi-component because yes it extends a component, but not really, as it's also with Game class and the two have some overlap that clash imo.

The FlameGame is actually a full blown component, it can be added anywhere in the component tree, it doesn't have to be in the root. When it is in the root it has more responsibilities though.

agreed, so the FlameGame should follow the lifecycle rule of component , onMount once and onRemove once.

[munsterlander]

We'll have to agree to disagree on this one lol. Yes it's a component, but it's also special and is extended from Game as well.

I will say that the event names could be clearer and would still say that onRemove is correct because the game is being removed from the widget. The widget is what gets detached from the Flutter widget tree, but I digress.

@spydon I will ping you about this externally to get your thoughts on possible names. I also have a diagram in my head that will map out the events.

[spydon]

We'll have to agree to disagree on this one lol. Yes it's a component, but it's also special and is extended from Game as well.

These events come from Component though and not from Game. It would be great to get some input from @st-pasha here since he wrote the dartdocs about that onRemove should be called from dispatch.

I will say that the event names could be clearer and would still say that onRemove is correct because the game is being removed from the widget. The widget is what gets detached from the Flutter widget tree, but I digress.

Then what would onDetach mean, since we already have that event to hook into in the game? I think we should probably keep all of the component lifecycle events to only the component tree as far as it is possible if we can't get it to work in a nice way for our users. The state machine shouldn't be able to go from onRemove to then keep on updating without going through onMount for example. Another idea is that we could filter out the calls to onRemove when the widget is reloaded due to our internal changes, like adding detectors.
I will have a chat with the rest of the team in our meeting today and see what they think!

[munsterlander]

So I dropped this in our chat but for @MiaoShichang, I hope this shows how the events (today) are impacted by the Flutter event lifecycle. There are definitely events missing and you can say that onRemove should come from GameWidget dispose (it was originally there).

The main problem is there are two widgets that house a game and depending on what action a user is doing, it triggers different cascading events that don't necessarily cross paths. The last PR I did was addressing memory leaks where onRemove was not called if you just killed the GameRenderBox because onRemove never got called, just onDetach.

Now obviously, you are seeing onRemove called because the game gets detached but not disposed. This is where the discussion should come in about what should the event names really be, because the game was definitely removed but not disposed. Disposed being permanent and removed not. So even if the event was onDetach, the game is still removed, so should it call that event OR should it pass something to say the source of the removal, etc.

I am open to any discussion about this, I just hope I am making sense and this provides some clarity on how it works today.

[Skyost]

Hey,

Just saying that the doc page about a game instance lifecycle (https://docs.flame-engine.org/latest/flame/game.html#lifecycle) is pretty irrelevant with the current onRemove behavior :

  @override
  void onRemove() {
    removeAll(children);
    processLifecycleEvents();
    Flame.images.clearCache();
    Flame.assets.clearCache();
    // Any other code that you want to run when the game is removed.
  }

This code leads to errors and bugs. I was also disposing FlameAudio.bgm in this hook, and my game kept auto closing, I've spent the last two hours trying to figure out what's happened. Looks like this was the cause of the problem.

[munsterlander]

You just need a boolean to check the reason onRemove is being called. Regardless, I have a potential solution proposed.

[MiaoShichang]

The last PR I did was addressing memory leaks where onRemove was not called if you just killed the GameRenderBox because onRemove never got called, just onDetach.

@munsterlander , I suggest

Edit: to be clear that the OnRemove being lifecycle of the game should not be called by the flutter widget event ，such as the GameRenderBox removed or readded, unloaded or reloaded, attach or detach.

we can see that other lifecycle of the game, such as the onLoad ,the onMount, onGameResize ,is be called by the GameWidget and doesn't be connected with the attach or detach of GameRenderBox.

[munsterlander]

I have split this out and now made Flame aware if it is a Gesture Detector being the cause of the reload. If so, then onRemove is no longer called. This should fix your issue and @Skyost's issue. So apologies up front for causing this bug with the last PR. I knew there was some risk with it, but due to the two paths into Game, I was trying to address an issue. The current PR is working for all the prior code examples that used to fail.

[MiaoShichang]

I have a question, @spydon @munsterlander , why is the lifecycle order of the game : onLoad -> onGameResize ->onMount -> Update -> onRemove instead of onMount -> onGameResize ->onLoad -> Update -> onRemove ?

I feel strange. Isn't onMount first, and then onLoad?
Logically speaking, onLoad can only continue to work after onMount.

Perhaps my idea is wrong.

[munsterlander]

OnLoad is actually a future that gets awaited, but in Flames case, it's loading things (game.dart) that need to be mounted that then get attached.

Edit: Here is the code of interest:

flame/packages/flame/lib/src/game/game_widget/game_widget.dart

Line 191 in d08284d

Future<void> get loaderFuture => _loaderFuture ??= (() async {

Edit2: in all things flame (game / component), onload can only happen once so it knows it can be mounted.

[MiaoShichang]

OnLoad is actually a future that gets awaited, but in Flames case, it's loading things that need to be mounted that then get attached.

Edit: Here is the code of interest:

flame/packages/flame/lib/src/game/game_widget/game_widget.dart

Line 191 in d08284d

Future<void> get loaderFuture => _loaderFuture ??= (() async {

That's why I have questions. If onMount first and then onLoad, the required things at that time are actually already mounted.

I have a situation now,

mixin ListenerHandler on Component {
  @override
  void onMount() {
    super.onMount();
    // add listener
  }

  @override
  void onRemove() {
    super.onRemove();
    // remove listener
  }
}

class MyComponent extends Component with ListenerHandler {
  @override
  FutureOr<void> onLoad() {
    // here, i can not call lister!
    // lister.call();

    return super.onLoad();
  }
}

Do I need to add a listener in onLoad of the mixin?

[ufrshubham]

why is the lifecycle order of the game : onLoad -> onGameResize ->onMount -> Update -> onRemove instead of onMount -> onGameResize ->onLoad -> Update -> onRemove ?

That is because once a component is mounted it starts ticking (updating + rendering). If we mark it mounted first and then start the async loading, it will be in an uninitialized state for some frame until the loading is complete. Current approach allows for fully loading heavy components much before they are mounted and start updating.

Do I need to add a listener in onLoad of the mixin?

Yes, or your can call the listener.call() in MyComponent.onMount, like so:

class MyComponent extends Component with ListenerHandler {
  @override
  void onMount() {
    super.onMount(); // makes sure listener is added first.
    listener.call();
  }
}

[MiaoShichang]

why is the lifecycle order of the game : onLoad -> onGameResize ->onMount -> Update -> onRemove instead of onMount -> onGameResize ->onLoad -> Update -> onRemove ?

That is because once a component is mounted it starts ticking (updating + rendering). If we mark it mounted first and then start the async loading, it will be in an uninitialized state for some frame until the loading is complete. Current approach allows for fully loading heavy components much before they are mounted and start updating.

Do I need to add a listener in onLoad of the mixin?

Yes, or your can call the listener.call() in MyComponent.onMount, like so:

class MyComponent extends Component with ListenerHandler {
  @override
  void onMount() {
    super.onMount(); // makes sure listener is added first.
    listener.call();
  }
}

why is the lifecycle order of the game : onLoad -> onGameResize ->onMount -> Update -> onRemove instead of onMount -> onGameResize ->onLoad -> Update -> onRemove ?

That is because once a component is mounted it starts ticking (updating + rendering). If we mark it mounted first and then start the async loading, it will be in an uninitialized state for some frame until the loading is complete. Current approach allows for fully loading heavy components much before they are mounted and start updating.

Do I need to add a listener in onLoad of the mixin?

Yes, or your can call the listener.call() in MyComponent.onMount, like so:

class MyComponent extends Component with ListenerHandler {
  @override
  void onMount() {
    super.onMount(); // makes sure listener is added first.
    listener.call();
  }
}

Although this can still meet my needs, it is not elegant enough for the code.

[ufrshubham]

Although this can still meet my needs, it is not elegant enough for the code.

It's literally the same code just being called a little later in the components lifecycle.

I don't know your full usecase, but the mixin you shared looks quite misleading. It gives the idea that it allows the component to listen to something by registering and unregistering from the listener in onMount and onRemove. But then the component implementation you shared, it itself calls the listener.call while loading. So if the emitter and listener are the same objects, is the invoke and listen even needed?

Overall I'm just saying that there is a possibility that your code needs some redesign instead of Flame's component lifecycle 😅

[ufrshubham]

@MiaoShichang feel free to open up new issues if you think something needs fixing or start a discussion here or on discord to discuss questions and potential problems about Flame. Let's limit the current issue to the original problem.

[munsterlander]

Maybe I am completely confused here, but why can't you just flip the events? Use onload then onmount? Additionally, you can check isMounted.