Allow ComponentRef access in RiverpodGameMixin

[tibotix]

What could be improved

I was wondering if it is possible to allow RiverpodComponentMixin and RiverpodGameMixin to be a mixin on the same class. Currently this is not possible due to the same variable name being used in both mixins (_onBuildCallbacks). One example on how this could be used then:

class MyGame extends FlameGame with RiverpodGameMixin, RiverpodComponentMixin {
  @override
  void onMount() {
    addToGameWidgetBuild(() {
      ref.listen(pausedStateStreamProvider, (p0, p1) {
        if (p1.hasValue) {
         paused = p1.value!;
        }
      });
    });
    super.onMount();
  }
}

Why should this be improved

It would make it easier to react upon provider changes on the Game instance level, for example to pause the game or do other things where access to the game instance is needed. Currently, a separate Component, which uses findGame() or HasGameRef, is needed for these use cases. Nevertheless, maybe a separate component is in fact the cleaner and recommended solution, so what do you think about this?

Any risks?

As far as i know, the mixins don't interfere with each other. However, based on the mixin names it might be a bit irritating to have both RiverpodGameMixin and RiverpodComponentMixin on one FlameGame class, but as FlameGame also extends Component, i think this is justified.

More information

I can write the PR if this is considered useful.

[spydon]

@markvideon what do you think about this suggestion?

[markvideon]

Seems fine, I suggest renaming the property in RiverpodGameMixin to _allOnBuildCallbacks.

[tibotix]

All right, i will make a PR with this change.

[markvideon]

@tibotix I suspect that applying RiverpodComponentMixin to a class that extends FlameGame will not successfully resolve the game reference, have you tested this locally?

[tibotix]

The game reference is successfully resolved, but there are general problems with the callback system as i've found out. Specifically, the RiverpodComponentMixin relies on the FlameGame to rebuild in order to call all the callbacks. But the game is only forced to rebuild by the RiverpodComponentMixin if the game is already mounted, and as the request to rebuild is initiated in onMount, the game has obviously not finished mounting. So the callbacks are never called. Also, when disposing the game, the RiverpodComponentMixin.onRemove method is called which in turn tries to rebuild the game again, using the assumption, that the GameWidget is still in the widget tree (which is in fact the case when using RivperpodComponentMixin on actual Components). Because of this the null check operator for key.currentState! in rebuildGameWidget fails, as currentState is null in that moment of time. Maybe it would be better to add functionality to the RiverpodGameMixin to allow access to ComponentRef methods, and leave RiverpodComponentMixin as it is. This way we don't mix up Component and Game logic, which is actually quite different.

[tibotix]

So, as far as i know we need a mechanism to call forceBuild() once the game is completely mounted, so we don't call setState during build-phase, or invent a way to prevent setState from being called during build-phase. Unfortunately, the Component.mounted future will never be completed, as Game has no longer access to the _mountedCompleter Completer. But we can theoretically make our own _mountedCompleter and use this. The good news is that we don't need to manage removal of riverpod dependencies and subscriptions, since they are closed anyway when disposing the Game instance.

For the rebuild of the game widget there are several possibilities:

• Make mounted (and eventually loaded and removed) Futures working on the FlameGame class and use these.
• Somehow expose a setter to the _requiresRebuild variable in GameWidgetState, so that we can set the flag and use the functionality of _protectedBuild instead of simply calling setState.
• Make a Future similar to ready() and periodically check isMounted. (not preferable due to performance)

If we have this functionality we can then do something like this in RiverpodGameMixin:

mixin RiverpodGameMixin<W extends World> on FlameGame<W> {
  // [...]
  final ComponentRef ref = ComponentRef(game: null);
  
  @mustCallSuper
  @override
  FutureOr<void> onLoad() {
    ref.game = this;
    return super.onLoad();
  }

  void addToGameWidgetBuild(Function() cb) {
    _allOnBuildCallbacks.add(cb);
  }

  @override
  void onMount() {
    mounted.whenComplete(() {
      key!.currentState!.forceBuild();
    });
    // OR: (if we use requiresRebuild instead of knowing when the game finished mounting)
    key!.currentState!.forceBuild();
  }
}

with RiverpodAwareGameWidgetState.forceBuild in case of requiresRebuild strategy like this:

void forceBuild() {
    if (_isForceBuilding) {
      _hasQueuedBuild = true;
      return;
    }
    _isForceBuilding = true;
    setRequiresRebuild();
    // setState(() {}); 
}

What do you think?

[tibotix]

@spydon what do you think of the proposed solutions (the mounted Future and exposure of _requiresRebuild) and their applicability? Or do you know any other ways of knowing when the game finished mounting?

[spydon]

@spydon what do you think of the proposed solutions (the mounted Future and exposure of _requiresRebuild) and their applicability? Or do you know any other ways of knowing when the game finished mounting?

I think we should investigate why the mounted completer isn't called, fix that and then use that one, since it does exist on the FlameGame. Sorry for the late answer, I've been away on vacation. :)

[tibotix]

I think we should investigate why the mounted completer isn't called, fix that and then use that one, since it does exist on the FlameGame. Sorry for the late answer, I've been away on vacation. :)

No Problems :) . I opened a new Issue to discuss the mounted completer issue.