Skip to content

Commit

Permalink
fix: Stop auto-resizing on external size change in sprite based compo…
Browse files Browse the repository at this point in the history
…nents (#2467)

This PR fixes a regression introduced in Flame 1.7.0 and discussed in this thread on discord.

The whole idea is to detect if size is being modified by some external call apart from the autoResizeing code and stop auto-resize from that point onwards.
  • Loading branch information
ufrshubham authored Apr 5, 2023
1 parent be31336 commit df236af
Show file tree
Hide file tree
Showing 8 changed files with 212 additions and 16 deletions.
45 changes: 37 additions & 8 deletions packages/flame/lib/src/components/sprite_animation_component.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class SpriteAnimationComponent extends PositionComponent
with HasPaint
implements SizeProvider {
/// The animation used by the component.
SpriteAnimation? animation;
SpriteAnimation? _animation;

/// If the component should be removed once the animation has finished.
/// Needs the animation to have `loop = false` to ever remove the component,
Expand All @@ -26,7 +26,7 @@ class SpriteAnimationComponent extends PositionComponent

/// Creates a component with an empty animation which can be set later
SpriteAnimationComponent({
this.animation,
SpriteAnimation? animation,
bool? autoResize,
this.removeOnFinish = false,
this.playing = true,
Expand All @@ -43,11 +43,16 @@ class SpriteAnimationComponent extends PositionComponent
(size == null) == (autoResize ?? size == null),
'''If size is set, autoResize should be false or size should be null when autoResize is true.''',
),
_animation = animation,
_autoResize = autoResize ?? size == null,
super(size: size ?? animation?.getSprite().srcSize) {
if (paint != null) {
this.paint = paint;
}

/// Register a listener to differentiate between size modification done by
/// external calls v/s the ones done by [_resizeToSprite].
this.size.addListener(_handleAutoResizeState);
}

/// Creates a SpriteAnimationComponent from a [size], an [image] and [data].
Expand Down Expand Up @@ -92,10 +97,25 @@ class SpriteAnimationComponent extends PositionComponent
_resizeToSprite();
}

/// This flag helps in detecting if the size modification is done by
/// some external call vs [_autoResize]ing code from [_resizeToSprite].
bool _isAutoResizing = false;

/// Returns the [SpriteAnimation] used by this component.
SpriteAnimation? get animation => _animation;

/// Sets the given [value] as [SpriteAnimation] to be used.
set animation(SpriteAnimation? value) {
if (_animation != value) {
_animation = value;
_resizeToSprite();
}
}

@mustCallSuper
@override
void render(Canvas canvas) {
animation?.getSprite().render(
_animation?.getSprite().render(
canvas,
size: size,
overridePaint: paint,
Expand All @@ -106,23 +126,32 @@ class SpriteAnimationComponent extends PositionComponent
@override
void update(double dt) {
if (playing) {
animation?.update(dt);
_animation?.update(dt);
_resizeToSprite();
}
if (removeOnFinish && (animation?.done() ?? false)) {
if (removeOnFinish && (_animation?.done() ?? false)) {
removeFromParent();
}
}

/// Updates the size to current animation sprite's srcSize if
/// Updates the size to current [animation] sprite's srcSize if
/// [autoResize] is true.
void _resizeToSprite() {
if (_autoResize) {
if (animation != null) {
size.setFrom(animation!.getSprite().srcSize);
_isAutoResizing = true;
if (_animation != null) {
size.setFrom(_animation!.getSprite().srcSize);
} else {
size.setZero();
}
_isAutoResizing = false;
}
}

/// Turns off [_autoResize]ing if a size modification is done by user.
void _handleAutoResizeState() {
if (_autoResize && (!_isAutoResizing)) {
_autoResize = false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class SpriteAnimationGroupComponent<T> extends PositionComponent
final Map<T, bool> removeOnFinish;

/// Map with the available states for this animation group
Map<T, SpriteAnimation>? animations;
Map<T, SpriteAnimation>? _animations;

/// Whether the animation is paused or playing.
bool playing;
Expand All @@ -27,7 +27,7 @@ class SpriteAnimationGroupComponent<T> extends PositionComponent

/// Creates a component with an empty animation which can be set later
SpriteAnimationGroupComponent({
this.animations,
Map<T, SpriteAnimation>? animations,
T? current,
bool? autoResize,
this.playing = true,
Expand All @@ -46,11 +46,16 @@ class SpriteAnimationGroupComponent<T> extends PositionComponent
'''If size is set, autoResize should be false or size should be null when autoResize is true.''',
),
_current = current,
_animations = animations,
_autoResize = autoResize ?? size == null,
super(size: size ?? animations?[current]?.getSprite().srcSize) {
if (paint != null) {
this.paint = paint;
}

/// Register a listener to differentiate between size modification done by
/// external calls v/s the ones done by [_resizeToSprite].
this.size.addListener(_handleAutoResizeState);
}

/// Creates a SpriteAnimationGroupComponent from a [size], an [image] and
Expand Down Expand Up @@ -96,7 +101,7 @@ class SpriteAnimationGroupComponent<T> extends PositionComponent
priority: priority,
);

SpriteAnimation? get animation => animations?[current];
SpriteAnimation? get animation => _animations?[current];

/// Returns the current group state.
T? get current => _current;
Expand All @@ -109,6 +114,17 @@ class SpriteAnimationGroupComponent<T> extends PositionComponent
_resizeToSprite();
}

/// Returns the animations state map.
Map<T, SpriteAnimation>? get animations => _animations;

/// Sets the given [value] as animation state map.
set animations(Map<T, SpriteAnimation>? value) {
if (_animations != value) {
_animations = value;
_resizeToSprite();
}
}

/// Returns current value of auto resize flag.
bool get autoResize => _autoResize;

Expand All @@ -121,6 +137,10 @@ class SpriteAnimationGroupComponent<T> extends PositionComponent
_resizeToSprite();
}

/// This flag helps in detecting if the size modification is done by
/// some external call vs [_autoResize]ing code from [_resizeToSprite].
bool _isAutoResizing = false;

@mustCallSuper
@override
void render(Canvas canvas) {
Expand All @@ -147,11 +167,20 @@ class SpriteAnimationGroupComponent<T> extends PositionComponent
/// [autoResize] is true.
void _resizeToSprite() {
if (_autoResize) {
_isAutoResizing = true;
if (animation != null) {
size.setFrom(animation!.getSprite().srcSize);
} else {
size.setZero();
}
_isAutoResizing = true;
}
}

/// Turns off [_autoResize]ing if a size modification is done by user.
void _handleAutoResizeState() {
if (_autoResize && (!_isAutoResizing)) {
_autoResize = false;
}
}
}
17 changes: 17 additions & 0 deletions packages/flame/lib/src/components/sprite_component.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ class SpriteComponent extends PositionComponent
if (paint != null) {
this.paint = paint;
}

/// Register a listener to differentiate between size modification done by
/// external calls v/s the ones done by [_resizeToSprite].
this.size.addListener(_handleAutoResizeState);
}

SpriteComponent.fromImage(
Expand Down Expand Up @@ -87,6 +91,10 @@ class SpriteComponent extends PositionComponent
_resizeToSprite();
}

/// This flag helps in detecting if the size modification is done by
/// some external call vs [_autoResize]ing code from [_resizeToSprite].
bool _isAutoResizing = false;

/// Returns the current sprite rendered by this component.
Sprite? get sprite => _sprite;

Expand Down Expand Up @@ -119,11 +127,20 @@ class SpriteComponent extends PositionComponent
/// Updates the size [sprite]'s srcSize if [autoResize] is true.
void _resizeToSprite() {
if (_autoResize) {
_isAutoResizing = true;
if (_sprite != null) {
size.setFrom(_sprite!.srcSize);
} else {
size.setZero();
}
_isAutoResizing = false;
}
}

/// Turns off [_autoResize]ing if a size modification is done by user.
void _handleAutoResizeState() {
if (_autoResize && (!_isAutoResizing)) {
_autoResize = false;
}
}
}
39 changes: 34 additions & 5 deletions packages/flame/lib/src/components/sprite_group_component.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ class SpriteGroupComponent<T> extends PositionComponent
T? _current;

/// Map with the available states for this sprite group
Map<T, Sprite>? sprites;
Map<T, Sprite>? _sprites;

/// When set to true, the component is auto-resized to match the
/// size of current sprite.
bool _autoResize;

/// Creates a component with an empty animation which can be set later
SpriteGroupComponent({
this.sprites,
Map<T, Sprite>? sprites,
T? current,
bool? autoResize,
Paint? paint,
Expand All @@ -40,14 +40,19 @@ class SpriteGroupComponent<T> extends PositionComponent
'''If size is set, autoResize should be false or size should be null when autoResize is true.''',
),
_current = current,
_sprites = sprites,
_autoResize = autoResize ?? size == null,
super(size: size ?? sprites?[current]?.srcSize) {
if (paint != null) {
this.paint = paint;
}

/// Register a listener to differentiate between size modification done by
/// external calls v/s the ones done by [_resizeToSprite].
this.size.addListener(_handleAutoResizeState);
}

Sprite? get sprite => sprites?[current];
Sprite? get sprite => _sprites?[current];

/// Returns the current group state.
T? get current => _current;
Expand All @@ -63,6 +68,17 @@ class SpriteGroupComponent<T> extends PositionComponent
/// Returns current value of auto resize flag.
bool get autoResize => _autoResize;

/// Returns the sprites map.
Map<T, Sprite>? get sprites => _sprites;

/// Sets the given [value] as sprites map.
set sprites(Map<T, Sprite>? value) {
if (_sprites != value) {
_sprites = value;
_resizeToSprite();
}
}

/// Sets the given value of autoResize flag.
///
/// Will update the [size] to fit srcSize of
Expand All @@ -72,15 +88,19 @@ class SpriteGroupComponent<T> extends PositionComponent
_resizeToSprite();
}

/// This flag helps in detecting if the size modification is done by
/// some external call vs [_autoResize]ing code from [_resizeToSprite].
bool _isAutoResizing = false;

@override
@mustCallSuper
void onMount() {
assert(
sprites != null,
_sprites != null,
'You have to set the sprites in either the constructor or in onLoad',
);
assert(
current != null,
_current != null,
'You have to set current in either the constructor or in onLoad',
);
}
Expand All @@ -98,11 +118,20 @@ class SpriteGroupComponent<T> extends PositionComponent
/// Updates the size to current [sprite]'s srcSize if [autoResize] is true.
void _resizeToSprite() {
if (_autoResize) {
_isAutoResizing = true;
if (sprite != null) {
size.setFrom(sprite!.srcSize);
} else {
size.setZero();
}
_isAutoResizing = false;
}
}

/// Turns off [_autoResize]ing if a size modification is done by user.
void _handleAutoResizeState() {
if (_autoResize && (!_isAutoResizing)) {
_autoResize = false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -336,5 +336,29 @@ Future<void> main() async {
component.autoResize = true;
expect(component.size, spriteList[1].srcSize);
});

test('stop autoResizing on external size modifications', () {
final spriteList = [
Sprite(image, srcSize: Vector2.all(1)),
Sprite(image, srcSize: Vector2.all(2)),
];
final animation = SpriteAnimation.spriteList(
spriteList,
stepTime: 1,
loop: false,
);
final testSize = Vector2(83, 100);
final component = SpriteAnimationComponent();

// NOTE: Sequence of modifications is important here. Changing the size
// first disables the auto-resizing. So even if animation is changed
// later, the component should still maintain testSize.
component
..size = testSize
..animation = animation;

expectDouble(component.size.x, testSize.x);
expectDouble(component.size.y, testSize.y);
});
});
}
Loading

0 comments on commit df236af

Please sign in to comment.