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

fix: Correct FlameGame lifecycle diagram and description #2707

Merged
merged 1 commit into from
Sep 16, 2023

Conversation

jlyonsmith
Copy link
Contributor

Description

The FlameGame lifecycle diagram is incorrect. The lifecycle for a FlameGame is as follows:

onGameResize
onLoad
onMount
render
update
onRemove
onGameResize
onLoad
onMount
render
update

Note in particular the onGameResize is before onLoad and the render and update are reversed. Because they are different, this PR creates a new lifecycle diagram that is not shared with Component. It also fixes the Component lifecycle diagram to be consistent with the FlameGame lifecycle diagram formatting and terms.

Checklist

  • [x ] 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.
  • [x ] No, this PR is not a breaking change.

Related Issues

Closes #2674

@spydon
Copy link
Member

spydon commented Sep 5, 2023

I think this indicates on at least one bug, that should be fixed instead. update should always run before render. I think onLoad should only happen once too, but I'll investigate more on that when Flutter & Friends conference is over. :)

@spydon
Copy link
Member

spydon commented Sep 14, 2023

@jlyonsmith now update and render is performed in the correct order after #2714, can you update this PR accordingly and we can merge it? :)

@jlyonsmith
Copy link
Contributor Author

Will do. Are the onLoad and onGameResize in the correct order? What's the reasoning behind them being reversed just for FlameGame? I made a local branch in my fork that reversed them if you want to take a look at jlyonsmith/flame/reorder1

@spydon
Copy link
Member

spydon commented Sep 14, 2023

Will do. Are the onLoad and onGameResize in the correct order? What's the reasoning behind them being reversed just for FlameGame? I made a local branch in my fork that reversed them if you want to take a look at jlyonsmith/flame/reorder1

Since FlameGame is the root we want to give the users access to the size of the game before onLoad runs so that they can use FlameGame.size to set up components in onLoad, I don't remember why onGameResize is called after onLoad in normal components, but it is not as important there since they can still access the size from the game.

I'll have a look at your branch in a bit!

@spydon
Copy link
Member

spydon commented Sep 15, 2023

Will do. Are the onLoad and onGameResize in the correct order? What's the reasoning behind them being reversed just for FlameGame? I made a local branch in my fork that reversed them if you want to take a look at jlyonsmith/flame/reorder1

Had a look now, and like I said above, onGameResize needs to be called before onLoad for FlameGame.

@jlyonsmith
Copy link
Contributor Author

OK, I updated the diagram to flip the the update and render to the expected order, and also corrected the text in the note.

@spydon spydon changed the title fix: FlameGame lifecycle diagram and description incorrect fix: Correct FlameGame lifecycle diagram and description Sep 16, 2023
@spydon spydon merged commit 3c550f8 into flame-engine:main Sep 16, 2023
8 checks passed
@spydon
Copy link
Member

spydon commented Sep 16, 2023

@jlyonsmith thanks for your contribution!

@jlyonsmith
Copy link
Contributor Author

No problem @spydon ! Thanks for maintaining this awesome package. I'm looking forward to finishing the port of my game to it. I'll be sure to share a link when it launches.

@jlyonsmith jlyonsmith deleted the bug_2674 branch September 16, 2023 17:15
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.

Lifecycle diagram incorrect - onGameResize called before onLoad
2 participants