feat!: Making add,addAll sync methods

[erickzanardo]

Description

Makes add, addAll and addToParent methods sync.

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.

  Migration instructions

  Component.add, Component.addAll and Component.addToParent now return void instead of a
  future. That future only ever covered the child's loading, never its mounting, so awaiting it was
  misleading, and forgetting to await it (or to wrap it in unawaited) tripped the
  discarded_futures lint in a lot of games.

  Drop the await:

  // Beforeawaitadd(MyComponent());
  awaitaddAll([MyComponent(), MyOtherComponent()]);
  // Afteradd(MyComponent());
  addAll([MyComponent(), MyOtherComponent()]);

  If you were relying on the returned future to know when the child had finished loading, await the
  child's loaded future instead:

  // Beforeawaitadd(crate);
  // Afteradd(crate);
  await crate.loaded;

  Awaiting loaded is safe from inside the parent's own onLoad, since the child starts loading as
  soon as it is added. Awaiting mounted, removed or game.lifecycleEventsProcessed there is not:
  the parent only mounts after its onLoad completes, so those waits deadlock.

  For a batch of children, or when you need them to be present in children rather than just loaded,
  await game.lifecycleEventsProcessed once after adding them.

  Load errors are no longer reported by GameWidget.errorBuilder

  GameWidget.errorBuilder used to catch a failing child's onLoad, because await add(child)
  chained the child's error onto the game's own onLoad future. That chain is gone: a child that
  throws in onLoad no longer reaches errorBuilder.

  The component is not added to the tree, and the rest of the game keeps running. The error is
  reported through the child's loaded future, and if nothing is awaiting it, it is handed to the
  current Zone as an uncaught error. To get the old behavior for a specific child, await its
  loaded future inside the parent's onLoad:

  classMyGameextendsFlameGame {
  @overrideFuture<void> onLoad() async {
  final level =Level();
  world.add(level);
  // Throws here if Level.onLoad fails, so errorBuilder is shown.await level.loaded;
  }
  }

Related Issues

Indicate which issues this PR resolves, if any. For example:

Part of #1938

[luanpotter]

trying to think of having a less clunky alternative, just random thoughts (and these can be followups), how about a

await Component.loaded(blobParts)

shape? (just does Future.wait(components.map(it => it.loaded)))
same for the other 3.

[erickzanardo]

I kinda like that helper, I would be happy to add it to this PR if we all agree on having it and using it around our code.

@spydon you onboard of this helper?

[spydon]

Hmm... It's a bit strange, doesn't really feel Dart idiomatic, but I don't have another suggestion for now (except for just using Future.wait).

[erickzanardo]

Maybe an extension on Iterable that would allow us to wait them? Something like:

extensionComponentIterableExtensiononIterable<Component> {
Future<void> loaded =>Future.wait(map((el) => el.loaded));
}

Then the developer can just call await myList.loaded, and that would be in pair with how the user wait for a single component to be loaded

[spydon]

Yeah, I like that better!!

[luanpotter]

to be honest, looking only at the PR it is not clear to me which ones of these need the load after all, and which (many ones) didn't, and way. even wondering if we want to justify the ones that kept the await with a comment on why

[erickzanardo]

I didn't checked that tbh, I will double check for obvious places where we know it isn't needed!

[luanpotter]

thanks @erickzanardo , sorry for the delay!

LGTM, aligned on removing. my only two thoughts:

1. it is unfortunate we made waiting for a list a bit clunkier than v1. I think we can explore some possible patterns on followups unless we really want to discourage this (but then we shouldn't use it on our own examples either).
2. not super clear to me when the await is truly needed. I guess I've been just defaulting to having it over the years, I've had issues of for example an update(dt) failing to find a child w/o it.

in summary: I think we need to be even more crisp on either "no you should never need to do wait add on onLoad, and there is no un-clunked way of doing" or "yes there are some cases it is needed, here they are, here are some helpers with docs"

would love to see that explored on followups!

[spydon]

Here are some things that we should consider before merging this PR:

Blocking

1. Deadlock warning is factually wrong.add() starts the child loading synchronously (hasLayout is always true by onLoad time), so await child.loaded in the parent's onLoad works fine. Only mounted deadlocks. Rewrite the warning in both the dartdoc and components.md; the PR's own flame_test/example/lib/game.dart and game.md contradict it.
2. _failLoading hangs loaded. Reading loaded after the error fired, or a second time, hangs forever. Persist the failure (_loadError field or state bit) and have loaded return Future.error(...).
3. GameWidget.errorBuilder no longer catches child load failures (verified: true on main, false here). Document in migration instructions, which are still commented out.

Should fix

4. Dartdoc oversells recoverability: a failed load permanently blocks the lifecycle queue, so catching via loaded recovers nothing. Soften the wording, or fix it by returning done from handleLifecycleEventAdd once the failure is persistent.
5. ensureAdd hangs instead of throwing on load failure.
6. Sync onLoad throws escape add() directly; async ones go through _failLoading. Inconsistent with "safe to call from anywhere, including inside update".
7. No direct test for _failLoading. The rethrow, late-await, and double-await branches are all uncovered.

Nits

8. _addChild still returns FutureOr<void> that all callers discard; make the discard explicit.
9. Mention lifecycleEventsProcessed in the new prose as the batch-wait idiom (answers Luan's clunkiness point; blob_example.dart's Future.wait(...loaded) is the shape he disliked).

[erickzanardo]

Here are some things that we should consider before merging this PR:

Blocking

1. Deadlock warning is factually wrong.add() starts the child loading synchronously (hasLayout is always true by onLoad time), so await child.loaded in the parent's onLoad works fine. Only mounted deadlocks. Rewrite the warning in both the dartdoc and components.md; the PR's own flame_test/example/lib/game.dart and game.md contradict it.
2. _failLoading hangs loaded. Reading loaded after the error fired, or a second time, hangs forever. Persist the failure (_loadError field or state bit) and have loaded return Future.error(...).
3. GameWidget.errorBuilder no longer catches child load failures (verified: true on main, false here). Document in migration instructions, which are still commented out.

Should fix

4. Dartdoc oversells recoverability: a failed load permanently blocks the lifecycle queue, so catching via loaded recovers nothing. Soften the wording, or fix it by returning done from handleLifecycleEventAdd once the failure is persistent.
5. ensureAdd hangs instead of throwing on load failure.
6. Sync onLoad throws escape add() directly; async ones go through _failLoading. Inconsistent with "safe to call from anywhere, including inside update".
7. No direct test for _failLoading. The rethrow, late-await, and double-await branches are all uncovered.

Nits

8. _addChild still returns FutureOr<void> that all callers discard; make the discard explicit.
9. Mention lifecycleEventsProcessed in the new prose as the batch-wait idiom (answers Luan's clunkiness point; blob_example.dart's Future.wait(...loaded) is the shape he disliked).

@spydon addressed all, take another look please.

[spydon]

Lgtm!