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

docs: Updated docs for gesture detector lifecycle #2650

Conversation

munsterlander
Copy link
Contributor

Description

As discussed in #2647, this PR just updates the docs to clarify that onRemove will get called when you first add a gesture based component.

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

Related Issues

@munsterlander munsterlander changed the title Initial Commit docs: Updated docs for gesture detector lifecycle Aug 13, 2023
when the first draggable component is added to the game, `FlameGame` must
temporarily remove itself and run the `GestureDetectorBuilder` which will add a
gesture detector as the first child to `FlameGame`, and then re-add itelf. As
such, `FlameGame`'s `onRemove` event will be triggered. A simple boolean flag
Copy link
Member

Choose a reason for hiding this comment

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

I think these probably should be two notes, or it's own section, now the next is in the middle of the previous note and reads a bit strange.

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 thought about that, but thought two notes at the top looked weird. I was wanting to ride on the note about the new API, but I can put it down below.

@munsterlander munsterlander deleted the UpdateDocsForGestureLifecycle branch August 19, 2023 03:20
spydon pushed a commit that referenced this pull request Aug 20, 2023
…etector addition (#2653)

This fixes: #2647
Reverts: #2602
OBE: #2650

To be clear, the memory leak issue is not addressed because it is not a
Flame issue. Flame properly calls the `onRemove` event and if it is
needed to pass that on to children, users should do as documented:
https://docs.flame-engine.org/latest/flame/game.html#lifecycle.

So if you are using `GameWidget.controlled`, add a tappable component
and then remove the game, the lifecycle looks like:

```
flutter: onLoad
flutter: mount
flutter: onMount
flutter: attach
flutter: add has_tappable_components
flutter: refreshWidget 
// Remove Game
flutter: detach
flutter: removeGameStateListener
flutter: onRemove
flutter: remove
```

If you do not use `GameWidget.controlled`, add a tappable component and
then remove the game, the lifecycle looks like:

```
flutter: onLoad
flutter: mount
flutter: onMount
flutter: attach
flutter: add has_tappable_components
flutter: refreshWidget
// Remove Game
flutter: detach
flutter: removeGameStateListener
flutter: onRemove
flutter: remove
```

If you do not use `GameWidget.controlled`, do not add a tappable
component and then remove the game, the lifecycle looks like:

```
flutter: onLoadFuture
flutter: mount
flutter: onMount
flutter: attach
// Remove Game
flutter: detach
flutter: removeGameStateListener
flutter: onRemove
flutter: remove
```

Previously, the lifecycle would look like:

```
flutter: onLoad
flutter: mount
flutter: onMount
flutter: attach
flutter: add has_tappable_components
flutter: refreshWidget 
flutter: detach // These unnecessary calls have been eliminated
flutter: removeGameStateListener // These unnecessary calls have been eliminated
flutter: onRemove // These unnecessary calls have been eliminated
flutter: attach
// Remove Game
flutter: detach
flutter: removeGameStateListener
flutter: onRemove
flutter: remove
```

I have updated the below diagram for those who may need it in the
future:

![Flame and Flutter Events
Fixed](https://github.com/flame-engine/flame/assets/17042581/1d98bdaf-db13-4e2c-b0a6-74da2ada89f3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants