[ParticleEmitterConfig] Speed options ignored in conjunction with moveTo

[kootoopas]

Version

3.55.2

Description

Particle emission speed-related options, speedX, speedY and speed, are ignored when used in conjunction with moveToX or moveToY.

However, in terms of resulting visuals, reducing the lifespan of the particles increases visual speed although it slightly shifts the target.

Proposal

If 'moveTo' and 'speed' options are intended to not be used with one another, it should be communicated to the programmer that this is the case through types, docs, an error throw, or a runtime warning. Although I do believe that the use case of source, target and speed used together should be a common one, therefore warranting some sort of fix.

Example Test Code

The following configs can be used on the related example page: https://labs.phaser.io/edit.html?src=src/game%20objects/particle%20emitter/move%20to.js

ParticleEmitterConfig w/ speed:

    var emitter = particles.createEmitter({
        frame: { frames: [ 'red', 'green', 'blue' ], cycle: true, quantity: 2 },
        x: -400,
        y: -100,
        moveToX: 400,
        moveToY: 600,
        speed: 1000000, // <--
        lifespan: 1000,
        scale: 0.5,
        quantity: 4,
        _frequency: 20,
        blendMode: 'ADD',
        emitZone: { source: rect }
    });

ParticleEmitterConfig w/ target-vector-shifting reduced lifespan:

    var emitter = particles.createEmitter({
        frame: { frames: [ 'red', 'green', 'blue' ], cycle: true, quantity: 2 },
        x: -400,
        y: -100,
        moveToX: 400,
        moveToY: 600,
        lifespan: 500,
        scale: 0.5,
        quantity: 4,
        _frequency: 20,
        blendMode: 'ADD',
        emitZone: { source: rect }
    });

[kootoopas]

Velocity assignment code when moveTo coords specified: https://github.com/photonstorm/phaser/blob/aa5f54cfa268c86823bf1dd52c83099fa0ef9ac2/src/gameobjects/particles/Particle.js#L346-L352

[samme]

One way to do this is

particles.createEmitter({
  angle: (p) => Phaser.Math.RadToDeg(Phaser.Math.Angle.BetweenPoints(p, target)),
  speed: SPEED
});

[kootoopas]

One way to do this is

particles.createEmitter({
  angle: (p) => Phaser.Math.RadToDeg(Phaser.Math.Angle.BetweenPoints(p, target)),
  speed: SPEED
});

@samme How does this guarantee target vector (moveTo) apart from angle?

[samme]

Guarantee how?

[kootoopas]

@samme The snippet you posted does not move emitted particles to a specific xy point. It requires lifespan to be calibrated in relation to speed so that the particles are destroyed on desired xy point.

In essence, conjunction of x, y, angle (edit: ~~speed~~) and lifespan with proper callibration of speed and lifespan can emulate conjunction of x, y, moveTo and lifespan.

[samme]

That could be done by adding a death zone or using

particles.createEmitter({
  moveToX: target.x,
  moveToY: target.y,
  lifespan: (p) => Phaser.Math.Distance.BetweenPoints(p, target) / SPEED
});

[kootoopas]

@samme Haven't tested if above code produces desired effect but in the case that it does, the internal speed on following line, https://github.com/photonstorm/phaser/blob/aa5f54cfa268c86823bf1dd52c83099fa0ef9ac2/src/gameobjects/particles/Particle.js#L346

will be in essence computed as:

 var speed = DistanceBetween(p, target) / ((DistanceBetween(p, target) / SPEED) / 1000); 

[samme]

If you want to avoid that then use

particles.createEmitter({
  angle: (p) => Phaser.Math.RadToDeg(Phaser.Math.Angle.BetweenPoints(p, target)),
  lifespan: (p) => (1000 * Phaser.Math.Distance.BetweenPoints(p, target)) / SPEED,
  speed: SPEED,
});

[kootoopas]

@samme I validated your latest reply (angle x lifespan x speed) and it seems to be working based on this demo: https://pastebin.com/UywaC3NH

However, the topic of this issue which is that speed is being ignored when used with moveTo & lifespan without notifying the developer or suggesting valid usage through code docs still stands. In the case that this is implemented through a console warning, in order to block the message from production environments, an env var could be set and checked before printing the message, f.e:

diff --git a/src/gameobjects/particles/ParticleEmitter.js b/src/gameobjects/particles/ParticleEmitter.js
index c6bb7d9b3..7f58a9591 100644
--- a/src/gameobjects/particles/ParticleEmitter.js
+++ b/src/gameobjects/particles/ParticleEmitter.js
@@ -21,6 +21,10 @@ var StableSort = require('../../utils/array/StableSort');
 var Vector2 = require('../../math/Vector2');
 var Wrap = require('../../math/Wrap');
 
+function warn(message) {
+    process.env.NODE_ENV === 'dev' && console.warn(message)
+}
+
 /**
  * @classdesc
  * A particle emitter represents a single particle stream.
@@ -312,6 +316,8 @@ var ParticleEmitter = new Class({
          */
         this.moveTo = false;
 
+        (config.moveToX || config.moveToY) && (config.speed || config.speedX || config.speedY) && warn('Speed incompatible with moveTo options - use lifespan instead.')
+
         /**
          * The x-coordinate emitted particles move toward, when {@link Phaser.GameObjects.Particles.ParticleEmitter#moveTo} is true.
          *

[samme]

I updated the docs (#6073).

I guess a warning is possible. The thing is it's not completely wrong to set both moveToX and speed etc. because you can toggle moveTo on or off after the emitter is created if you want.

[kootoopas]

@samme Conditions for warning can be checked when particle emission occurs (in Particle#fire) since speed and moveTo state does not change across lifecycle of particle; they're copied from emitter state when the particle fires.

[samme]

I think fromJSON() would be the best place for a warning.

[kootoopas]

So basically you're suggesting for it to be on particle emitter initialization too apart from fromJSON() since it's being called there. The problem you described above still stands if done so:

The thing is it's not completely wrong to set both moveToX and speed etc. because you can toggle moveTo on or off after the emitter is created if you want.

It's probably better to check for the warning whenever a dependent property is assigned, so apart from the json method, which is the only place that moveTo props are assigned, setSpeed, setSpeedX and setSpeedY should also have the check & message.

edit: A js setter can be used for moveTo too so that the check can be performed before its assignment.

[samme]

Well, Rich can decide. I think the docs change should be enough.

[photonstorm]

I agree, docs change is enough, although I've also added the information to the config docs, as I think it's more likely to be spotted there.