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.

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

MoveToEffect, RemoveEffect and SpeedEffectController not working inside SequenceEffect #2096

Closed
juanoboif25 opened this issue Oct 19, 2022 · 7 comments · Fixed by #2110 or #2173
Closed
Labels

Comments

@juanoboif25
Copy link

Current bug behaviour

When i put a MoveToEffect with a SpeedEffectControllerand a LinearEffectController and also a RemoveEffect inside a SequenceEffect, the component subject to them doesn´t move but instantly teleports to the desired Vector2 (regardless of what speed i specify) and it doesn´t get removed from the game. If the Speed and Linear EffectController's get replaced by a EffectController object, the component does move in a trasition-like manner. Taking them out of the SequenceEffect resolves all of the unexpected behaviours, regardless of what controller i use.

Expected behaviour

Move the component to a desired Vector2 in a timely manner and then removing it from the game.

Steps to reproduce

Add a SequenceEffect containing a MoveToEffect and a RemoveEffect to a PositionComponent in my case a CircleComponent,
use SpeedEffectControllerand a LinearEffectController for the MoveToEffect

Flutter doctor output

Output of: flutter doctor -v
[√] Flutter (Channel stable, 3.3.4, on Microsoft Windows [Version 10.0.22000.1098], locale en-US)
    • Flutter version 3.3.4 on channel stable at REDACTED
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision eb6d86ee27 (2 weeks ago), 2022-10-04 22:31:45 -0700
    • Engine revision c08d7d5efc
    • Dart version 2.18.2
    • DevTools version 2.15.0

[√] Android toolchain - develop for Android devices (Android SDK version 31.0.0)
    • Android SDK at REDACTED
    • Platform android-31, build-tools 31.0.0
    • Java binary at: REDACTED
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)
    • All Android licenses accepted.

    • Pixel 3a XL (mobile) • 93WAX0BHDF • android-arm64  • Android 12 (API 32)
    • Windows (desktop)    • windows    • windows-x64    • Microsoft Windows [Version 10.0.22000.1098]
    • Edge (web)           • edge       • web-javascript • Microsoft Edge 106.0.1370.42

[√] HTTP Host Availability
    • All required HTTP hosts are available

More environment information

  • Flame version: 1.4.0
  • Platform affected: android (haven't tested any other)
  • Platform version affected: android 12 (haven't tested any other)
@munsterlander
Copy link
Contributor

munsterlander commented Oct 22, 2022

Ok, I can confirm the bug. If you use MoveEffectTo it teleports and if you use MoveEffectBy it moves correctly; however, in both cases, the RemoveEffect does nothing. If you run RemoveEffect by itself, it works correctly, but RemoveEffect in a SequenceEffect - even by itself - does not work at all.

import 'package:doc_flame_examples/flower.dart';
import 'package:flame/effects.dart';
import 'package:flame/experimental.dart';
import 'package:flame/game.dart';

class SequenceEffectGame extends FlameGame with HasTappableComponents {
  @override
  Future<void> onLoad() async {
    final flower = Flower(
      size: 40,
      position: canvasSize / 2,
      onTap: (flower) {
        flower.add(
          SequenceEffect([
            MoveEffect.to(
              Vector2(30, 30),
              SpeedEffectController(
                LinearEffectController(1.0),
                speed: 10.0,
              ),
            ),
            RemoveEffect(delay: 1.0),
          ]),
        );
      },
    );
    add(flower);
  }
}

@st-pasha
Copy link
Contributor

Thanks for checking @munsterlander. It means there are probably 2 different bugs here: one with the RemoveEffect, and another with the MoveEffect.

@munsterlander
Copy link
Contributor

munsterlander commented Oct 23, 2022

Well, I should add, the error is only with that controller. Other controllers work correctly - see the existing sequence effect live example. So I guess it could be with that controller on the effect? I have never really looked at how MoveEffect implements the controller.

Edit:

So yes, MoveEffectTo works fine with other controllers in a sequence, but RemoveEffect still fails:

SequenceEffect([
            MoveEffect.to(
              Vector2(30, 30),
              EffectController(duration: 1.0),
            ),
            RemoveEffect(delay: 1.0),
          ]),

@tksoh
Copy link

tksoh commented Oct 23, 2022

[disclaimer: I am new to flutter]

I traced into apply() of RemoveEffect which tries to remove it's parent. But when wrapped within SequenceEffect, its parent is not the component, but SequenceEffect itself. I'm not sure what would happen if there are more effects after RemoveEffect, though I'd believe they will just be ignored since SequenceEffect has been removed?

 parent?.removeFromParent();

For now I just utilize the onComplete callback to remove the component manually. Not the best solution, but allow me to move on.

@spydon
Copy link
Member

spydon commented Oct 23, 2022

You are completely correct @tksoh, it should be using target and not parent.
I'll write up a PR for it.

@pgirald
Copy link
Contributor

pgirald commented Nov 19, 2022

Hey @spydon, I just saw that the mentioned SpeedEffectController problem was not addressed. In fact, I noticed that such controller does not work since (in the Effect's advance() method) the MoveToEffect's onStart() method does not get executed before the advance() controller's method does:

@internal
  double advance(double dt) {
    final remainingDt = controller.advance(dt);//<--`advance()` executes first
    if (!_started && controller.started) {
      _started = true;
      onStart();//<--then `onStart()` is executed
    }
    if (_started) {
      final progress = controller.progress;
      apply(progress);
      _lastProgress = progress;
    }
    if (!_finished && controller.completed) {
      _finished = true;
      onFinish();
    }
    return remainingDt;
  }

So SpeedEffectController gets 0.0 from the moveToEffect's measure() method. So I simply changed the order:

@internal
  double advance(double dt) {
    if (!_started && controller.started) {
      _started = true;
      onStart();//<--first `onStart()` is executed
    }
    final remainingDt = controller.advance(dt);//<--then `advance()` executes
    if (_started) {
      final progress = controller.progress;
      apply(progress);
      _lastProgress = progress;
    }
    if (!_finished && controller.completed) {
      _finished = true;
      onFinish();
    }
    return remainingDt;
  }

In fact, that's the order you use in the update method, and that's why SpeedEffectController works outside a sequence effect.

@spydon
Copy link
Member

spydon commented Nov 19, 2022

@pgirald good catch! Do you want to put up a PR for that?

spydon pushed a commit that referenced this issue Nov 22, 2022
…art() (#2173)

SpeedEffectController always completes inmediatly when it is inside a SequenceEffect. That happens because SpeedEffectController's advance() method executes before it's MeasurableEffect's onStart() method, that is, before it's parent effect is initialized (see #2096).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants