From 0d4df1acc21fba0abbe5d5ca082fe438e5171b61 Mon Sep 17 00:00:00 2001 From: Erick Date: Mon, 15 Feb 2021 19:07:58 -0300 Subject: [PATCH] Preventing errors caused by the premature use of size property on game (#659) --- CHANGELOG.md | 1 + lib/src/game/base_game.dart | 12 ++++++--- lib/src/game/game.dart | 26 +++++++++++++++++--- test/base_game_test.dart | 14 +++++------ test/components/composed_component_test.dart | 5 ++-- test/components/has_game_ref_test.dart | 1 + test/components/resizable_test.dart | 2 ++ test/effects/effect_test_utils.dart | 1 + 8 files changed, 46 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7125e188d..a3ca97e3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Added `Color` extensions - Change RaisedButton to ElevatedButton in timer example - Overhaul the draggables api to fix issues relating to local vs global positions + - Preventing errors caused by the premature use of size property on game ## 1.0.0-rc6 - Use `Offset` type directly in `JoystickAction.update` calculations diff --git a/lib/src/game/base_game.dart b/lib/src/game/base_game.dart index 4211971d8..0835c1776 100644 --- a/lib/src/game/base_game.dart +++ b/lib/src/game/base_game.dart @@ -63,7 +63,13 @@ class BaseGame extends Game with FPSCounter { /// Prepares and registers a component to be added on the next game tick /// /// This methods is an async operation since it await the `onLoad` method of the component. Nevertheless, this method only need to be waited to finish if by some reason, your logic needs to be sure that the component has finished loading, otherwise, this method can be called without waiting for it to finish as the BaseGame already handle the loading of the component. + /// + /// *Note:* Do not add components on the game constructor. This method can only be called after the game already has its layout set, this can be verified by the [hasLayout] property, to add components upon a game initialization, the [onLoad] method can be used instead. Future add(Component c) async { + assert( + hasLayout, + '"add" called before the game is ready. Did you try to access it on the Game constructor? Use the "onLoad" method instead.', + ); prepare(c); final loadFuture = c.onLoad(); @@ -90,7 +96,7 @@ class BaseGame extends Game with FPSCounter { /// This implementation of render basically calls [renderComponent] for every component, making sure the canvas is reset for each one. /// - /// You can override it further to add more custom behaviour. + /// You can override it further to add more custom behavior. /// Beware of however you are rendering components if not using this; you must be careful to save and restore the canvas to avoid components messing up with each other. @override @mustCallSuper @@ -116,7 +122,7 @@ class BaseGame extends Game with FPSCounter { /// This implementation of update updates every component in the list. /// /// It also actually adds the components that were added by the [addLater] method, and remove those that are marked for destruction via the [Component.shouldRemove] method. - /// You can override it further to add more custom behaviour. + /// You can override it further to add more custom behavior. @override @mustCallSuper void update(double t) { @@ -139,7 +145,7 @@ class BaseGame extends Game with FPSCounter { /// This implementation of resize passes the resize call along to every component in the list, enabling each one to make their decisions as how to handle the resize. /// /// It also updates the [size] field of the class to be used by later added components and other methods. - /// You can override it further to add more custom behaviour, but you should seriously consider calling the super implementation as well. + /// You can override it further to add more custom behavior, but you should seriously consider calling the super implementation as well. @override @mustCallSuper void onResize(Vector2 size) { diff --git a/lib/src/game/game.dart b/lib/src/game/game.dart index 1c21b2574..e48ceb011 100644 --- a/lib/src/game/game.dart +++ b/lib/src/game/game.dart @@ -30,11 +30,28 @@ abstract class Game { /// Currently attached build context. Can be null if not attached. BuildContext get buildContext => _gameRenderBox?.buildContext; - /// Current game viewport size, updated every resize via the [resize] method hook - final Vector2 size = Vector2.zero(); - + /// Whether the game widget was attached to the Flutter tree. bool get isAttached => buildContext != null; + /// Current size of the game as provided by the framework; it will be null if layout has not been computed yet. + /// + /// Use [size] and [hasLayout] for safe access. + Vector2 _size; + + /// Current game viewport size, updated every resize via the [resize] method hook + Vector2 get size { + assert( + hasLayout, + '"size" is not ready yet. Did you try to access it on the Game constructor? Use the "onLoad" method instead.', + ); + return _size; + } + + /// Indicates if the this game instance had its layout layed into the GameWidget + /// Only this is true, the game is ready to have its size used or in the case + /// of a BaseGame, to receive components. + bool get hasLayout => _size != null; + /// Returns the game background color. /// By default it will return a black color. /// It cannot be changed at runtime, because the game widget does not get rebuild when this value changes. @@ -53,7 +70,7 @@ abstract class Game { /// The default implementation just sets the new size on the size field @mustCallSuper void onResize(Vector2 size) { - this.size.setFrom(size); + _size = (_size ?? Vector2.zero())..setFrom(size); } /// This is the lifecycle state change hook; every time the game is resumed, paused or suspended, this is called. @@ -96,6 +113,7 @@ abstract class Game { /// Should not be called manually. void detach() { _gameRenderBox = null; + _size = null; onDetach(); } diff --git a/test/base_game_test.dart b/test/base_game_test.dart index ccf936f10..380bc666f 100644 --- a/test/base_game_test.dart +++ b/test/base_game_test.dart @@ -67,7 +67,7 @@ void main() { final MyGame game = MyGame(); final MyComponent component = MyComponent(); - game.size.setFrom(size); + game.onResize(size); game.add(component); // runs a cycle to add the component game.update(0.1); @@ -80,7 +80,7 @@ void main() { final MyGame game = MyGame(); final MyAsyncComponent component = MyAsyncComponent(); - game.size.setFrom(size); + game.onResize(size); await game.add(component); // runs a cycle to add the component game.update(0.1); @@ -95,7 +95,7 @@ void main() { final MyGame game = MyGame(); final MyComponent component = MyComponent(); - game.size.setFrom(size); + game.onResize(size); game.add(component); expect(component.gameSize, size); @@ -106,7 +106,7 @@ void main() { final MyGame game = MyGame(); final MyComponent component = MyComponent(); - game.size.setFrom(size); + game.onResize(size); game.add(component); // The component is not added to the component list until an update has been performed game.update(0.0); @@ -119,7 +119,7 @@ void main() { final MyGame game = MyGame(); final MyComponent component = MyComponent(); - game.size.setFrom(size); + game.onResize(size); game.add(component); // The component is not added to the component list until an update has been performed game.update(0.0); @@ -132,7 +132,7 @@ void main() { final MyGame game = MyGame(); final MyComponent component = MyComponent(); - game.size.setFrom(size); + game.onResize(size); game.add(component); GameRenderBox renderBox; tester.pumpWidget( @@ -158,7 +158,7 @@ void main() { final MyGame game = MyGame(); final MyComponent component = MyComponent(); - game.size.setFrom(size); + game.onResize(size); game.add(component); // The component is not added to the component list until an update has been performed game.update(0.0); diff --git a/test/components/composed_component_test.dart b/test/components/composed_component_test.dart index 79fdbce14..4de638736 100644 --- a/test/components/composed_component_test.dart +++ b/test/components/composed_component_test.dart @@ -87,7 +87,7 @@ void main() { final MyTap child = MyTap(); final MyComposed wrapper = MyComposed(); - game.size.setFrom(size); + game.onResize(size); game.add(wrapper); wrapper.addChild(child); game.update(0.0); @@ -106,7 +106,7 @@ void main() { ..position = Vector2.all(100) ..size = Vector2.all(300); - game.size.setFrom(size); + game.onResize(size); game.add(wrapper); wrapper.addChild(child); game.update(0.0); @@ -119,6 +119,7 @@ void main() { test('updates and renders children', () { final MyGame game = MyGame(); + game.onResize(Vector2.all(100)); final MyTap child = MyTap(); final MyComposed wrapper = MyComposed(); diff --git a/test/components/has_game_ref_test.dart b/test/components/has_game_ref_test.dart index 14e0b3b2e..712370eb5 100644 --- a/test/components/has_game_ref_test.dart +++ b/test/components/has_game_ref_test.dart @@ -20,6 +20,7 @@ void main() { test('simple test', () { final MyComponent c = MyComponent(); final MyGame game = MyGame(); + game.onResize(Vector2.all(200)); game.add(c); c.foo(); expect(game.calledFoo, true); diff --git a/test/components/resizable_test.dart b/test/components/resizable_test.dart index 9b5a1076e..524554215 100644 --- a/test/components/resizable_test.dart +++ b/test/components/resizable_test.dart @@ -33,6 +33,7 @@ void main() { test('game calls resize after added', () { final MyComponent a = MyComponent('a'); final MyGame game = MyGame(); + game.onResize(Vector2.all(10)); game.add(a); game.onResize(size); expect(a.gameSize, size); @@ -40,6 +41,7 @@ void main() { test("game calls doesn't change component size", () { final MyComponent a = MyComponent('a'); final MyGame game = MyGame(); + game.onResize(Vector2.all(10)); game.add(a); game.onResize(size); expect(a.size, isNot(size)); diff --git a/test/effects/effect_test_utils.dart b/test/effects/effect_test_utils.dart index 4fa695738..d271fcc07 100644 --- a/test/effects/effect_test_utils.dart +++ b/test/effects/effect_test_utils.dart @@ -28,6 +28,7 @@ void effectTest( final Callback callback = Callback(); effect.onComplete = callback.call; final BaseGame game = BaseGame(); + game.onResize(Vector2.all(200)); game.add(component); component.addEffect(effect); final double duration = effect.iterationTime;