From dba7761e01bf9dbcd881244a7ca14ccd12340007 Mon Sep 17 00:00:00 2001 From: Erick Zanardo Date: Thu, 3 Mar 2022 14:49:48 -0300 Subject: [PATCH] feat: addressing PR comments --- lib/game/bloc/game_state.dart | 3 + lib/game/components/ball.dart | 11 +- lib/game/components/wall.dart | 42 +++++++- lib/game/pinball_game.dart | 31 +----- lib/game/view/pinball_game_view.dart | 10 +- lib/game/view/view.dart | 1 + lib/game/view/widgets/game_over_dialog.dart | 18 ++++ lib/game/view/widgets/widgets.dart | 1 + test/game/bloc/game_state_test.dart | 26 +++++ test/game/components/ball_test.dart | 70 +++++++++++++ test/game/components/wall_test.dart | 60 +++++++++-- test/game/pinball_game_test.dart | 105 +------------------- test/helpers/builders.dart | 17 ++++ test/helpers/helpers.dart | 2 + test/helpers/mocks.dart | 13 +++ test/helpers/pump_app.dart | 2 +- 16 files changed, 259 insertions(+), 153 deletions(-) create mode 100644 lib/game/view/widgets/game_over_dialog.dart create mode 100644 lib/game/view/widgets/widgets.dart create mode 100644 test/helpers/builders.dart create mode 100644 test/helpers/mocks.dart diff --git a/lib/game/bloc/game_state.dart b/lib/game/bloc/game_state.dart index 235e264d..862edf96 100644 --- a/lib/game/bloc/game_state.dart +++ b/lib/game/bloc/game_state.dart @@ -26,6 +26,9 @@ class GameState extends Equatable { /// Determines when the game is over. bool get isGameOver => balls == 0; + /// Determines when player has only one chance left. + bool get isLastBall => balls == 1; + GameState copyWith({ int? score, int? balls, diff --git a/lib/game/components/ball.dart b/lib/game/components/ball.dart index 39257bb6..fd5f5605 100644 --- a/lib/game/components/ball.dart +++ b/lib/game/components/ball.dart @@ -29,18 +29,15 @@ class Ball extends BodyComponent return world.createBody(bodyDef)..createFixture(fixtureDef); } - @override - void onRemove() { + void ballLost() { final bloc = gameRef.read(); - final shouldBallrespwan = bloc.state.balls > 1; + final shouldBallRespwan = bloc.state.balls > 1; bloc.add(const BallLost()); - if (shouldBallrespwan) { - gameRef.resetBall(); + if (shouldBallRespwan) { + gameRef.spawnBall(); } - - super.onRemove(); } } diff --git a/lib/game/components/wall.dart b/lib/game/components/wall.dart index de93e1ce..c80e5bb3 100644 --- a/lib/game/components/wall.dart +++ b/lib/game/components/wall.dart @@ -1,10 +1,50 @@ +// ignore_for_file: avoid_renaming_method_parameters + import 'package:flame_forge2d/flame_forge2d.dart'; +import 'package:pinball/game/components/components.dart'; + +class BallWallContactCallback extends ContactCallback { + @override + void begin(Ball ball, Wall wall, Contact contact) { + if (wall.type == WallType.bottom) { + ball + ..ballLost() + ..shouldRemove = true; + } + } + + @override + void end(_, __, ___) {} +} + +enum WallType { + top, + bottom, + left, + right, +} class Wall extends BodyComponent { - Wall(this.start, this.end); + Wall({ + required this.type, + required this.start, + required this.end, + }); + + factory Wall.bottom(Forge2DGame game) { + final bottomRight = game.screenToWorld(game.camera.viewport.effectiveSize); + final bottomLeft = Vector2(0, bottomRight.y); + + return Wall( + type: WallType.bottom, + start: bottomRight, + end: bottomLeft, + ); + } final Vector2 start; final Vector2 end; + final WallType type; @override Body createBody() { diff --git a/lib/game/pinball_game.dart b/lib/game/pinball_game.dart index 2ee242da..3d586f12 100644 --- a/lib/game/pinball_game.dart +++ b/lib/game/pinball_game.dart @@ -1,21 +1,9 @@ -// ignore_for_file: avoid_renaming_method_parameters - import 'package:flame_bloc/flame_bloc.dart'; import 'package:flame_forge2d/flame_forge2d.dart'; import 'package:pinball/game/game.dart'; -class BallWallContactCallback extends ContactCallback { - @override - void begin(Ball ball, Wall wall, Contact contact) { - ball.shouldRemove = true; - } - - @override - void end(_, __, ___) {} -} - class PinballGame extends Forge2DGame with FlameBloc { - void resetBall() { + void spawnBall() { add(Ball(position: ballStartingPosition)); } @@ -25,25 +13,14 @@ class PinballGame extends Forge2DGame with FlameBloc { camera.viewport.effectiveSize.y - 20, ), ) - - Vector2( - 0, - -20, - ); + Vector2(0, -20); @override Future onLoad() async { - await super.onLoad(); + spawnBall(); addContactCallback(BallScorePointsCallback()); - final topLeft = Vector2.zero(); - final bottomRight = screenToWorld(camera.viewport.effectiveSize); - final bottomLeft = Vector2(topLeft.x, bottomRight.y); - - await add( - Wall(bottomRight, bottomLeft), - ); + await add(Wall.bottom(this)); addContactCallback(BallWallContactCallback()); - - resetBall(); } } diff --git a/lib/game/view/pinball_game_view.dart b/lib/game/view/pinball_game_view.dart index 5fbc8ae8..44715c25 100644 --- a/lib/game/view/pinball_game_view.dart +++ b/lib/game/view/pinball_game_view.dart @@ -14,15 +14,7 @@ class PinballGameView extends StatelessWidget { showDialog( context: context, builder: (_) { - return const Dialog( - child: SizedBox( - width: 200, - height: 200, - child: Center( - child: Text('Game Over'), - ), - ), - ); + return const GameOverDialog(); }, ); } diff --git a/lib/game/view/view.dart b/lib/game/view/view.dart index 66134941..39cdb071 100644 --- a/lib/game/view/view.dart +++ b/lib/game/view/view.dart @@ -1,2 +1,3 @@ export 'pinball_game_page.dart'; export 'pinball_game_view.dart'; +export 'widgets/widgets.dart'; diff --git a/lib/game/view/widgets/game_over_dialog.dart b/lib/game/view/widgets/game_over_dialog.dart new file mode 100644 index 00000000..586d6c56 --- /dev/null +++ b/lib/game/view/widgets/game_over_dialog.dart @@ -0,0 +1,18 @@ +import 'package:flutter/material.dart'; + +class GameOverDialog extends StatelessWidget { + const GameOverDialog({Key? key}) : super(key: key); + + @override + Widget build(BuildContext context) { + return const Dialog( + child: SizedBox( + width: 200, + height: 200, + child: Center( + child: Text('Game Over'), + ), + ), + ); + } +} diff --git a/lib/game/view/widgets/widgets.dart b/lib/game/view/widgets/widgets.dart new file mode 100644 index 00000000..9c457b1c --- /dev/null +++ b/lib/game/view/widgets/widgets.dart @@ -0,0 +1 @@ +export 'game_over_dialog.dart'; diff --git a/test/game/bloc/game_state_test.dart b/test/game/bloc/game_state_test.dart index f62bae67..c5131492 100644 --- a/test/game/bloc/game_state_test.dart +++ b/test/game/bloc/game_state_test.dart @@ -62,6 +62,32 @@ void main() { }); }); + group('isGameOver', () { + test( + 'is true ' + 'when there is only on ball left', + () { + const gameState = GameState( + balls: 1, + score: 0, + ); + expect(gameState.isLastBall, isTrue); + }, + ); + + test( + 'is false ' + 'when there are more balls left', + () { + const gameState = GameState( + balls: 2, + score: 0, + ); + expect(gameState.isLastBall, isFalse); + }, + ); + }); + group('copyWith', () { test( 'throws AssertionError ' diff --git a/test/game/components/ball_test.dart b/test/game/components/ball_test.dart index c4576c68..8521a80f 100644 --- a/test/game/components/ball_test.dart +++ b/test/game/components/ball_test.dart @@ -1,10 +1,14 @@ // ignore_for_file: cascade_invocations +import 'package:bloc_test/bloc_test.dart'; import 'package:flame_forge2d/flame_forge2d.dart'; import 'package:flame_test/flame_test.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:mocktail/mocktail.dart'; import 'package:pinball/game/game.dart'; +import '../../helpers/helpers.dart'; + void main() { TestWidgetsFlutterBinding.ensureInitialized(); @@ -79,5 +83,71 @@ void main() { }, ); }); + + group('resetting a ball', () { + late GameBloc gameBloc; + + setUp(() { + gameBloc = MockGameBloc(); + whenListen( + gameBloc, + const Stream.empty(), + initialState: const GameState.initial(), + ); + }); + + final tester = flameBlocTester( + gameBlocBuilder: () { + return gameBloc; + }, + ); + + tester.widgetTest( + 'adds BallLost to GameBloc', + (game, tester) async { + await game.ready(); + + game.children.whereType().first.ballLost(); + await tester.pump(); + + verify(() => gameBloc.add(const BallLost())).called(1); + }, + ); + + tester.widgetTest( + 'resets the ball if the game is not over', + (game, tester) async { + await game.ready(); + + game.children.whereType().first.removeFromParent(); + await game.ready(); // Making sure that all additions are done + + expect( + game.children.whereType().length, + equals(1), + ); + }, + ); + + tester.widgetTest( + 'no ball is added on game over', + (game, tester) async { + whenListen( + gameBloc, + const Stream.empty(), + initialState: const GameState(score: 10, balls: 1), + ); + await game.ready(); + + game.children.whereType().first.removeFromParent(); + await tester.pump(); + + expect( + game.children.whereType().length, + equals(0), + ); + }, + ); + }); }); } diff --git a/test/game/components/wall_test.dart b/test/game/components/wall_test.dart index e17537c0..2c4b241a 100644 --- a/test/game/components/wall_test.dart +++ b/test/game/components/wall_test.dart @@ -3,18 +3,46 @@ import 'package:flame_forge2d/flame_forge2d.dart'; import 'package:flame_test/flame_test.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:mocktail/mocktail.dart'; import 'package:pinball/game/game.dart'; +import '../../helpers/helpers.dart'; + void main() { TestWidgetsFlutterBinding.ensureInitialized(); group('Wall', () { + group('BallWallContactCallback', () { + test( + 'removes the ball on begin contact when the wall is a bottom one', + () { + final game = MockPinballGame(); + final wall = MockWall(); + final ball = MockBall(); + + when(() => wall.type).thenReturn(WallType.bottom); + when(() => ball.gameRef).thenReturn(game); + + BallWallContactCallback() + // Remove once https://github.com/flame-engine/flame/pull/1415 + // is merged + ..end(MockBall(), MockWall(), MockContact()) + ..begin(ball, wall, MockContact()); + + verify(() => ball.shouldRemove = true).called(1); + }, + ); + }); final flameTester = FlameTester(PinballGame.new); flameTester.test( 'loads correctly', (game) async { - final wall = Wall(Vector2.zero(), Vector2(100, 0)); + final wall = Wall( + type: WallType.bottom, + start: Vector2.zero(), + end: Vector2(100, 0), + ); await game.ensureAdd(wall); expect(game.contains(wall), isTrue); @@ -25,7 +53,11 @@ void main() { flameTester.test( 'positions correctly', (game) async { - final wall = Wall(Vector2.zero(), Vector2(100, 0)); + final wall = Wall( + type: WallType.top, + start: Vector2.zero(), + end: Vector2(100, 0), + ); await game.ensureAdd(wall); game.contains(wall); @@ -36,7 +68,11 @@ void main() { flameTester.test( 'is static', (game) async { - final wall = Wall(Vector2.zero(), Vector2(100, 0)); + final wall = Wall( + type: WallType.top, + start: Vector2.zero(), + end: Vector2(100, 0), + ); await game.ensureAdd(wall); expect(wall.body.bodyType, equals(BodyType.static)); @@ -48,7 +84,11 @@ void main() { flameTester.test( 'exists', (game) async { - final wall = Wall(Vector2.zero(), Vector2(100, 0)); + final wall = Wall( + type: WallType.top, + start: Vector2.zero(), + end: Vector2(100, 0), + ); await game.ensureAdd(wall); expect(wall.body.fixtures[0], isA()); @@ -58,7 +98,11 @@ void main() { flameTester.test( 'has restitution equals 0', (game) async { - final wall = Wall(Vector2.zero(), Vector2(100, 0)); + final wall = Wall( + type: WallType.top, + start: Vector2.zero(), + end: Vector2(100, 0), + ); await game.ensureAdd(wall); final fixture = wall.body.fixtures[0]; @@ -69,7 +113,11 @@ void main() { flameTester.test( 'has friction', (game) async { - final wall = Wall(Vector2.zero(), Vector2(100, 0)); + final wall = Wall( + type: WallType.top, + start: Vector2.zero(), + end: Vector2(100, 0), + ); await game.ensureAdd(wall); final fixture = wall.body.fixtures[0]; diff --git a/test/game/pinball_game_test.dart b/test/game/pinball_game_test.dart index 582fc11b..75a77aa9 100644 --- a/test/game/pinball_game_test.dart +++ b/test/game/pinball_game_test.dart @@ -1,108 +1,9 @@ -import 'package:bloc_test/bloc_test.dart'; -import 'package:flame_forge2d/flame_forge2d.dart'; -import 'package:flame_test/flame_test.dart'; -import 'package:flutter_bloc/flutter_bloc.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:mockingjay/mockingjay.dart'; -import 'package:pinball/game/game.dart'; - -class MockPinballGame extends Mock implements PinballGame {} - -class MockWall extends Mock implements Wall {} - -class MockBall extends Mock implements Ball {} - -class MockContact extends Mock implements Contact {} - -class MockGameBloc extends Mock implements GameBloc {} void main() { - // TODO(alestiago): test if [PinballGame] registers - // [BallScorePointsCallback] once the following issue is resolved: - // https://github.com/flame-engine/flame/issues/1416 group('PinballGame', () { - group('BallWallContactCallback', () { - test('removes the ball on begin contact', () { - final game = MockPinballGame(); - final wall = MockWall(); - final ball = MockBall(); - - when(() => ball.gameRef).thenReturn(game); - - BallWallContactCallback() - // Remove once https://github.com/flame-engine/flame/pull/1415 - // is merged - ..end(MockBall(), MockWall(), MockContact()) - ..begin(ball, wall, MockContact()); - - verify(() => ball.shouldRemove = true).called(1); - }); - }); - - group('resetting a ball', () { - late GameBloc gameBloc; - - setUp(() { - gameBloc = MockGameBloc(); - whenListen( - gameBloc, - const Stream.empty(), - initialState: const GameState.initial(), - ); - }); - - FlameTester( - PinballGame.new, - pumpWidget: (gameWidget, tester) async { - await tester.pumpWidget( - BlocProvider.value( - value: gameBloc, - child: gameWidget, - ), - ); - }, - ) - ..widgetTest('adds BallLost to GameBloc', (game, tester) async { - await game.ready(); - - game.children.whereType().first.removeFromParent(); - await tester.pump(); - - verify(() => gameBloc.add(const BallLost())).called(1); - }) - ..widgetTest( - 'resets the ball if the game is not over', - (game, tester) async { - await game.ready(); - - game.children.whereType().first.removeFromParent(); - await game.ready(); // Making sure that all additions are done - - expect( - game.children.whereType().length, - equals(1), - ); - }, - ) - ..widgetTest( - 'no ball is added on game over', - (game, tester) async { - whenListen( - gameBloc, - const Stream.empty(), - initialState: const GameState(score: 10, balls: 1), - ); - await game.ready(); - - game.children.whereType().first.removeFromParent(); - await tester.pump(); - - expect( - game.children.whereType().length, - equals(0), - ); - }, - ); - }); + // TODO(alestiago): test if [PinballGame] registers + // [BallScorePointsCallback] once the following issue is resolved: + // https://github.com/flame-engine/flame/issues/1416 }); } diff --git a/test/helpers/builders.dart b/test/helpers/builders.dart new file mode 100644 index 00000000..5ef98226 --- /dev/null +++ b/test/helpers/builders.dart @@ -0,0 +1,17 @@ +import 'package:flame_test/flame_test.dart'; +import 'package:flutter_bloc/flutter_bloc.dart'; +import 'package:pinball/game/game.dart'; + +FlameTester flameBlocTester({required GameBloc Function() gameBlocBuilder}) { + return FlameTester( + PinballGame.new, + pumpWidget: (gameWidget, tester) async { + await tester.pumpWidget( + BlocProvider.value( + value: gameBlocBuilder(), + child: gameWidget, + ), + ); + }, + ); +} diff --git a/test/helpers/helpers.dart b/test/helpers/helpers.dart index 695f8309..97bc22be 100644 --- a/test/helpers/helpers.dart +++ b/test/helpers/helpers.dart @@ -5,4 +5,6 @@ // license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +export 'builders.dart'; +export 'mocks.dart'; export 'pump_app.dart'; diff --git a/test/helpers/mocks.dart b/test/helpers/mocks.dart new file mode 100644 index 00000000..559ea87a --- /dev/null +++ b/test/helpers/mocks.dart @@ -0,0 +1,13 @@ +import 'package:flame_forge2d/flame_forge2d.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:pinball/game/game.dart'; + +class MockPinballGame extends Mock implements PinballGame {} + +class MockWall extends Mock implements Wall {} + +class MockBall extends Mock implements Ball {} + +class MockContact extends Mock implements Contact {} + +class MockGameBloc extends Mock implements GameBloc {} diff --git a/test/helpers/pump_app.dart b/test/helpers/pump_app.dart index ed5d4b1c..2c1efd9f 100644 --- a/test/helpers/pump_app.dart +++ b/test/helpers/pump_app.dart @@ -13,7 +13,7 @@ import 'package:mockingjay/mockingjay.dart'; import 'package:pinball/game/game.dart'; import 'package:pinball/l10n/l10n.dart'; -class MockGameBloc extends Mock implements GameBloc {} +import 'helpers.dart'; extension PumpApp on WidgetTester { Future pumpApp(