From 9282cc38f06cd6c87ed3a1880d28d5c9f290cc04 Mon Sep 17 00:00:00 2001 From: William Shakour Date: Wed, 6 Mar 2024 13:47:00 +0000 Subject: [PATCH] fix: Clamp opacity set by the `ColorEffect` to 1.0 (#3069) Clamp opacity set by the ColorEffect to 1.0; this was causing an issue when the tween returned values greater than 1. The test shows that when dt is greater than 0.5 and less than 0.75 for a period of 1 the tween returns approx 1.2 which causes Color to throw an exception. This has now been fixed. Since https://github.com/flutter/flutter/issues/89433 has also now been fixed, I have changed the min opacity to 0. --- .../flame/lib/src/effects/color_effect.dart | 5 +- .../flame/test/effects/color_effect_test.dart | 58 +++++++++++++++++-- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/packages/flame/lib/src/effects/color_effect.dart b/packages/flame/lib/src/effects/color_effect.dart index 4fe5cffcd5e..3a1c291d3e2 100644 --- a/packages/flame/lib/src/effects/color_effect.dart +++ b/packages/flame/lib/src/effects/color_effect.dart @@ -43,10 +43,7 @@ class ColorEffect extends ComponentEffect { @override void apply(double progress) { final currentColor = color.withOpacity( - // Currently there is a bug when opacity is 0 in the color filter. - // "Expected a value of type 'SkDeletable', but got one of type 'Null'" - // https://github.com/flutter/flutter/issues/89433 - max(_tween.transform(progress), 1 / 255), + min(max(_tween.transform(progress), 0), 1), ); target.tint(currentColor, paintId: paintId); } diff --git a/packages/flame/test/effects/color_effect_test.dart b/packages/flame/test/effects/color_effect_test.dart index 3030adc46c4..39650ecb175 100644 --- a/packages/flame/test/effects/color_effect_test.dart +++ b/packages/flame/test/effects/color_effect_test.dart @@ -18,11 +18,7 @@ void main() { game.update(0); expect( component.paint.colorFilter.toString(), - // Once https://github.com/flutter/flutter/issues/89433 has been fixed - // the two equals lines should be swapped and the ColorEffect should go - // to opacity 0. - //equals('ColorFilter.mode(Color(0x00f44336), BlendMode.srcATop)'), - equals('ColorFilter.mode(Color(0x01f44336), BlendMode.srcATop)'), + equals('ColorFilter.mode(Color(0x00f44336), BlendMode.srcATop)'), ); game.update(0.5); @@ -62,6 +58,36 @@ void main() { }, ); + testWithFlameGame( + 'resets the color filter to the original state', + (game) async { + final component = _PaintComponent(); + await game.ensureAdd(component); + + final originalColorFilter = component.paint.colorFilter; + const color = Colors.red; + + final effect = ColorEffect( + color, + EffectController(duration: 1), + ); + await component.add(effect); + game.update(0.5); + + expect( + originalColorFilter, + isNot(equals(component.paint.colorFilter)), + ); + + effect.reset(); + + expect( + originalColorFilter, + equals(component.paint.colorFilter), + ); + }, + ); + testWithFlameGame( 'can be re-added in the component tree', (game) async { @@ -84,6 +110,28 @@ void main() { }, ); + testWithFlameGame( + 'will clamp controllers that over or under set the progress value', + (game) async { + final component = _PaintComponent(); + await game.ensureAdd(component); + + final effect = ColorEffect( + opacityFrom: 1, + opacityTo: 0, + Colors.black, + ZigzagEffectController( + period: 1, + ), + ); + await component.ensureAdd(effect); + expect( + () => game.update(0.56), + returnsNormally, + ); + }, + ); + test('Validates opacity values', () { expect( () => ColorEffect(