feat!: Pause game when backgrounded #2642

adil192 · 2023-08-08T18:55:26Z

Description

When the game is backgrounded and then resumed, the update function is called with a very large dt time step.

This might cause the physics engine to hit the maximum translation per timestep, causing moving bodies to slow down to almost-zero velocities.

This PR makes the game pause if the game is backgrounded. If you'd like to change this behaviour, set the pauseWhenBackgrounded flag to true (default) or false.

class MyGame extends FlameGame {
  MyGame() {
    pauseWhenBackgrounded = false;
  }
}

This is currently not working on the web, see flutter/flutter#53107.

You can view previous discussion about this here: flame-engine/forge2d#84.

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

If you override the lifecycleStateChange method in your Game class, you must now call super.lifecycleStateChange.

If you'd like the game not to pause when backgrounded, set the pauseWhenBackgrounded flag in your Game class to false.

Related Issues

Closes flame-engine/forge2d#84

spydon

This should not be done like this directly in the engine (it's also a breaking change), the user should instead handle pausing the engine on events when the app is backgrounded. Not sure if there are any such events to react to when switching browser tabs though.

adil192 · 2023-08-08T19:05:48Z

Should this not be the default out-of-the-box behaviour though?
If the physics engine can't handle time steps of multiple seconds, then the sensible default is to avoid such time steps

spydon · 2023-08-08T19:12:04Z

Should this not be the default out-of-the-box behaviour though?
If the physics engine can't handle time steps of multiple seconds, then the sensible default is to avoid such time steps

That depends on the rest of your settings though, and with a zoom it should very rarely need this except when backgrounded or having massive lags. We should follow how box2d handles it.

The better solution that solves most of these problems is: #2613

adil192 · 2023-08-08T19:18:16Z

with a zoom it should very rarely need this except when backgrounded or having massive lags

But an app being backgrounded is not rare at all. The flutter package should handle a common event in a flutter app.

The better solution that solves most of these problems is: #2613

I would argue that this doesn't solve this problem with backgrounding. Our previous discussion in flame-engine/forge2d#84 was about hitting the speed limit with fast bodies, but this PR is only concerned with backgrounding.

Even if we raise the speed limit with some unit conversion like zoom or a meter-to-pixels constant, the app being backgrounded for a few seconds will still hit any speed limit and break expected behaviour.

adil192 · 2023-08-08T19:20:12Z

Perhaps we could use a getter for maxDt instead of a constant, so when devs extend the Forge2DGame they can opt-in to pause the simulation when backgrounded like this:

class MyGame extends Forge2DGame {
  @override
  double get maxDt => 0.5;
}

or even like this:

class MyGame extends Forge2DGame {
  MyGame(): super(
    pauseWhenBackgrounded: true,
  );
}

spydon · 2023-08-08T19:25:23Z

But an app being backgrounded is not rare at all. The flutter package should handle a common event in a flutter app.

This covers a lot more cases than just backgrounding an app though.
A PR that pauses the physics engine when an app is backgrounded would be accepted, but this throws away timesteps as default which should not be the case for a physics engine.

spydon · 2023-08-08T19:31:17Z

Something like this should be used: https://api.flutter.dev/flutter/widgets/WidgetsBindingObserver/didChangeAppLifecycleState.html

I'm not sure if that would work for web though.

adil192 · 2023-08-08T20:02:39Z

I'm unsure if the AppLifecycleState would work in this scenario, since I think the update method is called after the app is foregrounded again. I need to test it to make sure.
If I'm correct about that, I'll need to modify the code to only restore AppLifecycleState.resumed after the first discarded update

spydon · 2023-08-09T07:18:57Z

I'm unsure if the AppLifecycleState would work in this scenario, since I think the update method is called after the app is foregrounded again. I need to test it to make sure. If I'm correct about that, I'll need to modify the code to only restore AppLifecycleState.resumed after the first discarded update

I'm not sure if I understand you correctly here, but you are not the one setting the AppLifecycleState, you get that from overriding didChangeAppLifecycleState.

EDIT: Sorry, missed that you already pushed an update!

packages/flame/lib/src/game/game.dart

adil192 · 2023-08-13T03:41:06Z

Did it work in the browser?

Nope :(
Running this code

  @override
  void didChangeAppLifecycleState(AppLifecycleState state) {
    print('AppLifecycleState: $state');
    super.didChangeAppLifecycleState(state);
  }

tells me that didChangeAppLifecycleState isn't called at all when switching tabs.

It runs as expected on Android though, so I'm guessing this is a Flutter bug since it's easy to detect tab focus with js.

I/flutter (28489): AppLifecycleState: AppLifecycleState.inactive
I/flutter (28489): AppLifecycleState: AppLifecycleState.paused
I/flutter (28489): update: dt = 0.008337, isAppInForeground = false
I/FA-Ads  (28489): Application backgrounded at: timestamp_millis: 1691897854812
I/flutter (28489): AppLifecycleState: AppLifecycleState.resumed
I/flutter (28489): update: dt = 2.482243, isAppInForeground = true

Edit: Just realised that this means isAppInForeground is true when it shouldn't be. I'm thinking I should make an enum like

enum ForegroundState {
  background,
  foregroundFirstFrame,
  foreground,
}

spydon · 2023-08-18T08:21:29Z

Edit: Just realised that this means isAppInForeground is true when it shouldn't be. I'm thinking I should make an enum like

I didn't follow, in which case is the app in the background but with the boolean set to true?

adil192 · 2023-08-18T13:26:34Z

I didn't follow, in which case is the app in the background but with the boolean set to true?

Yes. What happened was:

I backgrounded the app. This sets isAppInForeground to false.
An update was ran while the app was still backgrounded, which incorrectly set isAppInForeground back to true.
I reopened the app.
An update ran with a dt of 2.482243 seconds because the app thinks it has been in the foreground.

packages/flame_forge2d/lib/forge2d_game.dart

packages/flame/lib/src/game/game.dart

adil192 · 2023-08-24T14:00:18Z

This private method should use pauseEngine and resumeEngine instead of using an early return.
Maybe foregroundedFirstUpdate won't be a problem if those two methods are used instead either?

Testing this on Android yields:

I/flutter (20126): update dt: 0.041648
I/flutter (20126): update dt: 0.499768
I/FA-Ads  (20126): Application backgrounded at: timestamp_millis: 1692885388255
I/flutter (20126): update dt: 2.497637
I/flutter (20126): update dt: 0.008329
I/flutter (20126): update dt: 0.008326

which means I need to add foregroundedFirstUpdate (foregrounding) back in

spydon · 2023-08-24T14:02:31Z

which means I need to add foregroundedFirstUpdate (foregrounding) back in

Probably better to modify how pause/resume works instead, that behaviour doesn't look like it would be intentional

spydon · 2023-09-08T08:48:47Z

Probably better to modify how pause/resume works instead, that behaviour doesn't look like it would be intentional

Did you have any look at this @adil192?

spydon · 2023-09-11T22:17:57Z

Added a test in #2731 that makes sure that dt isn't accumulating meanwhile a game is paused with the pauseEngine method.

adil192 · 2023-09-12T16:53:07Z

No sorry I've been quite busy recently

adil192 · 2023-09-12T17:16:04Z

With some extra logging (and rebasing the branch), it seems everything is in order.
In the previous log, I must have forgotten to set pauseWhenBackgrounded to true in the constructor.

I/flutter (22999): update dt: 0.008343
I/flutter (22999): update dt: 0.008343
I/flutter (22999): update dt: 0.008344
I/flutter (22999): update dt: 0.00834
I/flutter (22999): lifecycleStateChange: AppLifecycleState.hidden
I/flutter (22999): Pausing engine
I/flutter (22999): lifecycleStateChange: AppLifecycleState.paused
I/flutter (22999): Pausing engine
I/FA-Ads  (22999): Application backgrounded at: timestamp_millis: 1694538751731
I/flutter (22999): lifecycleStateChange: AppLifecycleState.hidden
I/flutter (22999): Pausing engine
I/flutter (22999): lifecycleStateChange: AppLifecycleState.inactive
I/flutter (22999): Resuming engine
I/flutter (22999): update dt: 0.0
I/flutter (22999): update dt: 0.008329
I/flutter (22999): update dt: 0.008342
I/flutter (22999): update dt: 0.008331
I/flutter (22999): update dt: 0.008316
I/flutter (22999): update dt: 0.008332
I/flutter (22999): lifecycleStateChange: AppLifecycleState.resumed
I/flutter (22999): update dt: 0.058368
I/flutter (22999): update dt: 0.008339

spydon · 2023-09-14T08:52:49Z

@adil192 did you try this on iOS?

adil192 · 2023-09-14T13:17:57Z

@adil192 did you try this on iOS?

I haven't, no. I don't have access to a macOS/iOS device currently

spydon · 2023-09-14T13:41:56Z

@adil192 did you try this on iOS?

I haven't, no. I don't have access to a macOS/iOS device currently

Alright, we'll have a test on those then. Which platforms have you tried?

adil192 · 2023-09-14T13:43:10Z

Alright, we'll have a test on those then. Which platforms have you tried?

Just Android at the moment. I can test other platforms if you want me to

spydon · 2023-09-14T13:46:15Z

Just Android at the moment. I can test other platforms if you want me to

Please do, I guess you at least have access to one desktop platform and web too?
Would be interesting to see if this also would work if you switch tabs for example, or minimize the application on desktop.

spydon · 2023-09-15T11:26:25Z

Alright we had a chat about this, let's make it a breaking change and set it to true by default. :)

adil192 · 2023-09-16T02:42:37Z

This doesn't work on Windows when you minimise an application, it still updates as if it was foregounded. I assume that's another implementation that Flutter needs.

The only lifecycleStateChanges are from when you dispose of the GameWidget widget.

flutter: lifecycleStateChange: AppLifecycleState.resumed # opened game with play button
flutter: lifecycleStateChange: AppLifecycleState.paused # pressed back button
flutter: lifecycleStateChange: AppLifecycleState.resumed # pressed the play button again
# no lifecycleStateChange events if you minimise the window

The web implementation is similarly missing, see flutter/flutter#53107.
I'm not sure if there is a corresponding issue for Windows/other desktop platforms

I'm guessing that AppLifecycleState is only supported on Android and iOS. Do you still want to make pauseWhenBackgrounded true by default?

spydon · 2023-09-16T14:26:49Z

I'm guessing that AppLifecycleState is only supported on Android and iOS. Do you still want to make pauseWhenBackgrounded true by default?

Yeah, we should mention it in the docs though.

spydon · 2023-09-17T22:16:13Z

Some small issues that needs to be fixed:

Add Backgrounding to the dictionary (probably update the branch first, it might already be added)
Run dart format .
Make sure no lines in the markdown are longer than 100 chars and 2 empty lines above titles.

doc/flame/game.md:236 MD022/blanks-around-headings/blanks-around-headers Headings should be surrounded by blank lines [Expected: 2; Actual: 1; Above] [Context: "### Backgrounding"]
doc/flame/game.md:249:101 MD013/line-length Line length [Expected: 100; Actual: 113]

adil192 · 2023-09-17T22:52:11Z

probably update the branch first, it might already be added

I will do this shortly!

spydon · 2023-09-18T18:01:41Z

It seems to somehow break the lifecycle according to one of the lifecycle tests 🤔

fix: Pause forge2d when backgrounded chore: temporarily add print statements Revert "chore: temporarily add print statements" This reverts commit 5f672df. fix: prevent duplicate `pauseEngine` calls ref: move `pauseWhenBackgrounded` to `FlameGame` fix: remove outdated comment ref: move test to flame package chore: dart format . chore: remove unused import ref: move variables closer to usage ref: remove `pauseWhenBackgrounded` from constructor feat: make pauseWhenBackgrounded true by default docs: mention only working on mobile docs: add doc for pauseWhenBackgrounded flag

adil192 · 2023-09-18T22:08:30Z

It seems to somehow break the lifecycle according to one of the lifecycle tests 🤔

With some added events:

Expected: [
            'onGameResize',
            'onLoad',
            'onMount',
            'update',
            'render',
            'update',
            'onRemove',
            'onDispose',
            'onGameResize',
            'onMount',
            'update', // <-- this is missing
            'render'
          ]
  Actual: [
            'onGameResize',
            'onLoad',
            'onMount',
            'update',
            // 'lifecycleStateChange(AppLifecycleState.resumed)',
            'render',
            'update',
            // 'lifecycleStateChange(AppLifecycleState.paused)',
            'onRemove',
            'onDispose',
            'onGameResize',
            'onMount',
            // 'lifecycleStateChange(AppLifecycleState.resumed)',
            'render'
          ]

It looks like the first update is discarded because of the use of pauseEngine and resumeEngine. Are you happy for me to just remove the expected 'update', line?

spydon · 2023-09-18T22:11:16Z

It looks like the first update is discarded because of the use of pauseEngine and resumeEngine. Are you happy for me to just remove the expected 'update', line?

Unfortunately not, we need to be calling update before render when coming back from being unmounted.

spydon · 2023-09-18T22:30:13Z

packages/flame/lib/src/game/game_widget/game_widget.dart

@@ -254,6 +254,9 @@ class GameWidgetState<T extends Game> extends State<GameWidget<T>> {
      currentGame = widget.game!;
    }
    currentGame.addGameStateListener(_onGameStateChange);
+    currentGame.lifecycleStateChange(


Hmm, it's pretty weird to call these manually since these state changes should be coming from the framework.

I think it makes more sense for the game to act identically to if the app is backgrounded when really it was just unmounted.
This makes the link between "backgrounding the app" and "backgrounding the game (widget)"

Plus as an added bonus, it makes the behaviour more consistent on non-mobile platforms since the game will be paused when the widget is disposed of

I see what you mean, it feels like the state machine could become inconsistent with the one on Flutter's side though, which might confuse developers and be bug prone.

spydon · 2023-09-19T08:03:07Z

packages/flame/test/game/flame_game_test.dart

@@ -698,6 +698,28 @@ void main() {
        });
      });
    });
+
+    group('pauseWhenBackgrounded:', () {


Can you add some tests where it is using a widget and not calling lifecycleStateChange manually? There should be some other widget tests that you can take inspiration from.

I'll add some tests, so that we can get this PR into the release.

spydon reviewed Aug 8, 2023

View reviewed changes

adil192 changed the title ~~fix: Pause forge2d when backgrounded~~ fix!: Pause forge2d when backgrounded Aug 8, 2023

adil192 commented Aug 12, 2023

View reviewed changes

packages/flame/lib/src/game/game.dart Outdated Show resolved Hide resolved

adil192 marked this pull request as ready for review August 12, 2023 01:12

adil192 mentioned this pull request Aug 13, 2023

didChangeAppLifecycleState isn't called on web flutter/flutter#132448

Closed

2 tasks

adil192 marked this pull request as draft August 13, 2023 15:59

spydon reviewed Aug 18, 2023

View reviewed changes

packages/flame_forge2d/lib/forge2d_game.dart Outdated Show resolved Hide resolved

packages/flame/lib/src/game/game.dart Outdated Show resolved Hide resolved

adil192 force-pushed the adil192-patch-1 branch from a6652a5 to b58c81d Compare August 24, 2023 13:25

spydon changed the title ~~fix!: Pause forge2d when backgrounded~~ fix!: Pause game when backgrounded Sep 8, 2023

spydon mentioned this pull request Sep 11, 2023

test: Test so that paused gameloop doesn't count dt #2731

Merged

5 tasks

adil192 force-pushed the adil192-patch-1 branch 2 times, most recently from 5fc5e4c to 5f672df Compare September 12, 2023 17:06

adil192 marked this pull request as ready for review September 12, 2023 17:18

spydon changed the title ~~feat: Optionally pause game when backgrounded~~ feat!: Optionally pause game when backgrounded Sep 18, 2023

adil192 force-pushed the adil192-patch-1 branch from a3bfd07 to 61998e3 Compare September 18, 2023 21:55

adil192 force-pushed the adil192-patch-1 branch from 61998e3 to c0c900f Compare September 18, 2023 21:55

fix: call lifecycleStateChange before onMount

3a49b64

adil192 changed the title ~~feat!: Optionally pause game when backgrounded~~ feat!: Pause game when backgrounded Sep 18, 2023

adil192 marked this pull request as ready for review September 18, 2023 22:28

adil192 requested a review from spydon September 18, 2023 22:28

spydon reviewed Sep 18, 2023

View reviewed changes

spydon reviewed Sep 19, 2023

View reviewed changes

spydon added 2 commits September 21, 2023 18:24

Merge branch 'main' into adil192-patch-1

a6fa4ae

Add widget tests

653dab7

spydon approved these changes Sep 21, 2023

View reviewed changes

spydon enabled auto-merge (squash) September 21, 2023 16:42

Remove outer material app

8ac9626

spydon merged commit 521e56b into flame-engine:main Sep 21, 2023
7 checks passed

adil192 deleted the adil192-patch-1 branch September 26, 2023 10:07

feat!: Pause game when backgrounded #2642

feat!: Pause game when backgrounded #2642

Conversation

adil192 commented Aug 8, 2023 • edited Loading

Description

Checklist

Breaking Change?

Migration instructions

Related Issues

spydon left a comment

Choose a reason for hiding this comment

adil192 commented Aug 8, 2023

spydon commented Aug 8, 2023

adil192 commented Aug 8, 2023

adil192 commented Aug 8, 2023 • edited Loading

spydon commented Aug 8, 2023

spydon commented Aug 8, 2023

adil192 commented Aug 8, 2023

spydon commented Aug 9, 2023 • edited Loading

adil192 commented Aug 13, 2023 • edited Loading

spydon commented Aug 18, 2023

adil192 commented Aug 18, 2023

adil192 commented Aug 24, 2023

spydon commented Aug 24, 2023

spydon commented Sep 8, 2023

spydon commented Sep 11, 2023

adil192 commented Sep 12, 2023

adil192 commented Sep 12, 2023 • edited Loading

spydon commented Sep 14, 2023

adil192 commented Sep 14, 2023

spydon commented Sep 14, 2023

adil192 commented Sep 14, 2023

spydon commented Sep 14, 2023

spydon commented Sep 15, 2023

adil192 commented Sep 16, 2023 • edited Loading

spydon commented Sep 16, 2023

spydon commented Sep 17, 2023 • edited Loading

adil192 commented Sep 17, 2023

spydon commented Sep 18, 2023

adil192 commented Sep 18, 2023 • edited Loading

spydon commented Sep 18, 2023

spydon Sep 18, 2023

Choose a reason for hiding this comment

adil192 Sep 18, 2023

Choose a reason for hiding this comment

adil192 Sep 18, 2023

Choose a reason for hiding this comment

spydon Sep 18, 2023

Choose a reason for hiding this comment

spydon Sep 19, 2023

Choose a reason for hiding this comment

spydon Sep 21, 2023

Choose a reason for hiding this comment

adil192 commented Aug 8, 2023 •

edited

Loading

adil192 commented Aug 8, 2023 •

edited

Loading

spydon commented Aug 9, 2023 •

edited

Loading

adil192 commented Aug 13, 2023 •

edited

Loading

adil192 commented Sep 12, 2023 •

edited

Loading

adil192 commented Sep 16, 2023 •

edited

Loading

spydon commented Sep 17, 2023 •

edited

Loading

adil192 commented Sep 18, 2023 •

edited

Loading