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.

Sign up for GitHub

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

Jump to bottom

fix: Deprecate pause and resume in GameLoop #1240

Merged
merged 3 commits into from
Jan 16, 2022
Merged

fix: Deprecate pause and resume in GameLoop #1240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
838e72f
Deprecate pause and resume in GameLoop
st-pasha Dec 17, 2021
1f3b995
GameLoop.previous is now private
st-pasha Dec 17, 2021
f94a3dd
Merge branch 'main' into ps/gameloop-pause-resume
st-pasha Jan 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 16 additions & 22 deletions packages/flame/lib/src/game/game_loop.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,47 +2,41 @@ import 'package:flutter/scheduler.dart';

class GameLoop {
void Function(double dt) callback;
Duration previous = Duration.zero;
late Ticker _ticker;
Duration _previous = Duration.zero;
late final Ticker _ticker;

GameLoop(this.callback) {
_ticker = Ticker(_tick);
}

void _tick(Duration timestamp) {
final dt = _computeDeltaT(timestamp);
final durationDelta = timestamp - _previous;
final dt = durationDelta.inMicroseconds / Duration.microsecondsPerSecond;
_previous = timestamp;
callback(dt);
}

double _computeDeltaT(Duration now) {
final delta = previous == Duration.zero ? Duration.zero : now - previous;
previous = now;
return delta.inMicroseconds / Duration.microsecondsPerSecond;
}

void start() {
_ticker.start();
if (!_ticker.isActive) {
_ticker.start();
}
}

void stop() {
_ticker.stop();
_previous = Duration.zero;
}

void dispose() {
_ticker.dispose();
}

void pause() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think they were meant to be the same, IIRC @erickzanardo added those to fix a bug or something

Copy link
Member

@erickzanardo erickzanardo Dec 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I do remember something like this too, but I can remember exactly what bug and why, we should hold this until we can remember

Copy link
Member

@spydon spydon Dec 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also thought we added these, but apparently these were added by someone called @erf 20 months ago, maybe they could explain?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the PR: #302

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spydon hi, i took a look now at my PR where i simplified the game loop to use a Ticker. It looks like start and stop replaced the scheduleTick and unscheduleTick calls. The only different between those were that previous Duration were set to zero in pause. It think it would be fine to combine these calls. Not sure if there would be a case where e.g. the user would e.g. want to pause the callbacks, but still make the timer move forward or something like that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you file an issue for that @st-pasha since you have the most context?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, in your test, did you declare the game outside of the GameWidget?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested with the standard example from our Dashbook, I'll have to check if it recreates the game instance upon frame rotation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I checked again -- after making sure that the game instance is declared only once in the examples app, the "bug" disappears (i.e. the game state keeps being paused after rotating the frame or changing window size, or opening the side menu). This happens both in the main branch and in this branch.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super, then it works as intended!

_ticker.muted = true;
previous = Duration.zero;
}
@Deprecated('Internal variable')
Duration get previous => _previous;

void resume() {
_ticker.muted = false;
// If the game has started paused, we need to start the ticker
// as it would not have been started yet
if (!_ticker.isActive) {
start();
}
}
@Deprecated('Use stop() instead')
void pause() => stop();

@Deprecated('Use start() instead')
void resume() => start();
}
5 changes: 2 additions & 3 deletions packages/flame/lib/src/game/game_render_box.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import 'package:flutter/widgets.dart' hide WidgetBuilder;
import 'game_loop.dart';
import 'mixins/game.dart';

// ignore: prefer_mixin
class GameRenderBox extends RenderBox with WidgetsBindingObserver {
BuildContext buildContext;
Game game;
Expand All @@ -30,8 +29,8 @@ class GameRenderBox extends RenderBox with WidgetsBindingObserver {

final gameLoop = this.gameLoop = GameLoop(gameLoopCallback);

game.pauseEngineFn = gameLoop.pause;
game.resumeEngineFn = gameLoop.resume;
game.pauseEngineFn = gameLoop.stop;
game.resumeEngineFn = gameLoop.start;

if (!game.paused) {
gameLoop.start();
Expand Down