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

refactor!: The method onLoad() now returns FutureOr<void> #2228

Merged
merged 8 commits into from
Dec 23, 2022

Conversation

st-pasha
Copy link
Contributor

Description

Before this PR, the return type of onLoad() was Future<void>?, after this PR the return type is FutureOr<void> -- both for classes Component and Game.

Reasons:

  • The use of FutureOr is more idiomatic in Dart, this class was specifically created in order to be used in situations like ours.
  • This makes learning Flame easier for beginners, since you no longer need to explain what asynchronous programming is from the start (and onLoad() is one of the first methods the user encounters in Flame).
  • The code can be cleaner in case when onLoad doesn't need to be async.

With new approach, the onLoad() method can be overridden as either

@override
Future<void> onLoad() async { ... }

or

@override
void onLoad() { ... }

Of course, it can also be overridden as

@override
FutureOr<void> onLoad() { ... }

but this is rare, only for components that are designed to be further subclassed, or for mixins.

The documentation was updated to show the new recommended usage.

Checklist

  • The title of my PR starts with a Conventional Commit prefix (fix:, feat:, docs: etc).
  • 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 (minor).

Migration instructions

Most of the existing code is expected to work as-is. In particular, the following patterns will continue to work:

  • @override Future<void> onLoad() async { ... };
  • @override Future<void> onLoad() async { await super.onLoad(); ... };
  • @override Future<void> onLoad() { ...; return super.onLoad(); };
  • @override Future<void>? onLoad() { ...; return null; };

However, the following pattern will no longer work:

  • @override Future<void>? onLoad() { ...; return super.onLoad(); }

Instead, it will now produce a compile error, which can be fixed by changing the signature to one of

  • @override void onLoad() { ...; } (in most cases the call to super.onLoad() is not needed);
  • @override void onLoad() { ...; super.onLoad(); };
  • @override FutureOr<void> onLoad() { ...; return super.onLoad(); };

Related Issues

@erickzanardo
Copy link
Member

Is this really a breaking change? I don't think this will break any existing code right?

@st-pasha
Copy link
Contributor Author

Is this really a breaking change? I don't think this will break any existing code right?

It's barely breaking. Like, the following example would no longer work, because FutureOr<void> cannot be returned from a method with return type Future<void>?:

  @override
  Future<void>? onLoad() {
    width = 200;
    height = 70;
    return super.onLoad();
  }

But then, these kind of examples are rare (there is no point in calling super.onLoad() since it's a noop), and are easily fixable.

@spydon
Copy link
Member

spydon commented Dec 21, 2022

Good idea!

@spydon spydon requested a review from a team December 23, 2022 17:44
@spydon spydon enabled auto-merge (squash) December 23, 2022 20:24
@spydon spydon merged commit d898b53 into main Dec 23, 2022
@spydon spydon deleted the ps.future-or-void branch December 23, 2022 20:30
@pdong15dth
Copy link

i can't find onLoad function in flame 1.6.0 version

@spydon
Copy link
Member

spydon commented Jan 25, 2023

i can't find onLoad function in flame 1.6.0 version

Please put questions in either on Discord, in issues or on StackOverflow.
This PR is unrelated to your question. :)

onLoad still exists on the Component, so you can override it in any Component (including the FlameGame).

@pdong15dth
Copy link

i can't find onLoad function in flame 1.6.0 version

Please put questions in either on Discord, in issues or on StackOverflow. This PR is unrelated to your question. :)

onLoad still exists on the Component, so you can override it in any Component (including the FlameGame).

Thanks. I found it, because android studio doesn't show recommended for onload function i thought it was removed from flame.

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.

None yet

5 participants