Skip to content

Commit

Permalink
Remove the 'Moved" event
Browse files Browse the repository at this point in the history
We currently emit a 'Move' event when the position is set,
and a separate 'Moved' event when the position is set by the 'Motion' event.
(I believe this is baggage left over from how Multiway/etc used to work?)

This has a fairly high cost, since on a typical motion tick we emit 4 events,
two for each axis.

This PR simplifies things by
- Removing the 'Moved' event completely
- Adding a method that handles both x and y change in one pass.

The result is that linearMotionTick will only emit one move-related event per frame.
In limited performance testing, this had a fairly large impact.
In a test involving several hundred falling entities, linearMotion + event callbacks went from
consuming 13% of the total CPU time to 5.5%.
  • Loading branch information
starwed committed Jul 1, 2017
1 parent e8649ea commit 6dd3a40
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 43 deletions.
27 changes: 24 additions & 3 deletions src/spatial/2d.js
Original file line number Diff line number Diff line change
Expand Up @@ -678,8 +678,7 @@ Crafty.c("2D", {
* for an opposite direction.
*/
shift: function (x, y, w, h) {
if (x) this.x += x;
if (y) this.y += y;
if (x || y) this._setPosition(this._x + x, this._y + y);
if (w) this.w += w;
if (h) this.h += h;

Expand Down Expand Up @@ -877,6 +876,28 @@ Crafty.c("2D", {
this._setter2d('_y', y2 );
},

// A separate setter for the common case of moving an entity along both axes
_setPosition: function(x, y) {
if (x === this._x && y === this._y) return;
var old = Crafty.rectManager._pool.copy(this);
var mbr = this._mbr;
if (mbr) {
mbr._x -= this._x - x;
mbr._y -= this._y - y;
// cbr is a non-minimal bounding rectangle that contains both hitbox and mbr
// It will exist only when the collision hitbox sits outside the entity
if (this._cbr){
this._cbr._x -= this._x - x;
this._cbr._y -= this._y - y;
}
}
this._x = x;
this._y = y;
this.trigger("Move", old);
this.trigger("Invalidate");
Crafty.rectManager._pool.recycle(old);
},

// This is a setter method for all 2D properties including
// x, y, w, h, and rotation.
_setter2d: function (name, value) {
Expand All @@ -897,7 +918,7 @@ Crafty.c("2D", {
mbr = this._mbr;
if (mbr) {
mbr[name] -= this[name] - value;
// cbr is a non-minmal bounding rectangle that contains both hitbox and mbr
// cbr is a non-minimal bounding rectangle that contains both hitbox and mbr
// It will exist only when the collision hitbox sits outside the entity
if (this._cbr){
this._cbr[name] -= this[name] - value;
Expand Down
31 changes: 9 additions & 22 deletions src/spatial/motion.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ Crafty.c("AngularMotion", {
* @category 2D
* @kind Component
*
* @trigger Moved - When entity has moved due to velocity/acceleration on either x or y axis a Moved event is triggered. If the entity has moved on both axes for diagonal movement the event is triggered twice. - { axis: 'x' | 'y', oldValue: Number } - Old position
* @trigger NewDirection - When entity has changed direction due to velocity on either x or y axis a NewDirection event is triggered. The event is triggered once, if direction is different from last frame. - { x: -1 | 0 | 1, y: -1 | 0 | 1 } - New direction
* @trigger MotionChange - When a motion property has changed a MotionChange event is triggered. - { key: String, oldValue: Number } - Motion property name and old value
*
Expand Down Expand Up @@ -474,12 +473,12 @@ Crafty.c("Motion", {
*/
_linearMotionTick: function(frameData) {
var dt = frameData.dt / 1000; // time in s
var oldX = this._x, vx = this._vx, ax = this._ax,
oldY = this._y, vy = this._vy, ay = this._ay;
var vx = this._vx, ax = this._ax,
vy = this._vy, ay = this._ay;

// s += v * Δt + (0.5 * a) * Δt * Δt
var newX = oldX + vx * dt + 0.5 * ax * dt * dt;
var newY = oldY + vy * dt + 0.5 * ay * dt * dt;
var dx = vx * dt + 0.5 * ax * dt * dt;
var dy = vy * dt + 0.5 * ay * dt * dt;
// v += a * Δt
this.vx = vx + ax * dt;
this.vy = vy + ay * dt;
Expand All @@ -494,22 +493,10 @@ Crafty.c("Motion", {
this.trigger('NewDirection', oldDirection);
}

// Check if velocity has changed
var movedEvent = this.__movedEvent;
// Δs = s[t] - s[t-1]
this._dx = newX - oldX;
this._dy = newY - oldY;
if (this._dx !== 0) {
this.x = newX;
movedEvent.axis = 'x';
movedEvent.oldValue = oldX;
this.trigger('Moved', movedEvent);
}
if (this._dy !== 0) {
this.y = newY;
movedEvent.axis = 'y';
movedEvent.oldValue = oldY;
this.trigger('Moved', movedEvent);
}
this._dx = dx;
this._dy = dy;

// Set the position using the optimized _setPosition method
this._setPosition(this._x + dx, this._y + dy);
}
});
18 changes: 0 additions & 18 deletions tests/unit/spatial/motion.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@

var newDirectionEvents = 0,
newRotationDirectionEvents = 0,
movedEvents = 0,
rotatedEvents = 0,
motionEvents = 0;
e.bind("NewDirection", function(evt) {
Expand All @@ -198,9 +197,6 @@
e.bind("NewRotationDirection", function(evt) {
newRotationDirectionEvents++;
});
e.bind("Moved", function(evt) {
movedEvents++;
});
e.bind("Rotated", function(evt) {
rotatedEvents++;
});
Expand All @@ -213,10 +209,6 @@
_.strictEqual(evt.x, 0, "[1] - no motion along x axis");
_.strictEqual(evt.y, 1, "[1] - moving along +y axis");
});
e.one("Moved", function(evt) {
_.strictEqual(evt.axis, "y", "[1] - moved along y axis");
_.strictEqual(evt.oldValue, 0, "[1] - old y was 0");
});
e.one("MotionChange", function(evt) {
_.strictEqual(evt.key, "vy", "[1] - vy was set");
_.strictEqual(evt.oldValue, 0, "[1] - old vy was 0");
Expand All @@ -225,19 +217,10 @@
Crafty.timer.simulateFrames(1);

// group 2: set both vy and vx to be negative
var old_y = e.y;
e.one("NewDirection", function(evt) {
_.strictEqual(evt.x, -1, "[2] - Now moving along -x axis" );
_.strictEqual(evt.y, -1, "[2] - Now moving along -y axis");
});
e.one("Moved", function(evt) {
_.strictEqual(evt.axis, "x", "[2] - Moved along x axis");
_.strictEqual(evt.oldValue, 0, "[2] - old x was 0");
e.one("Moved", function(evt) {
_.strictEqual(evt.axis, "y", "[2] - Moved along y axis");
_.strictEqual(evt.oldValue, old_y, "[2] - old y value matches cached");
});
});
e.one("MotionChange", function(evt) {
_.strictEqual(evt.key, "vx", "[2] - vx was changed");
_.strictEqual(evt.oldValue, 0, "[2] - old vx was 0");
Expand Down Expand Up @@ -321,7 +304,6 @@
_.strictEqual(newDirectionEvents, 3, "NewDirection fired 3 times.");
_.strictEqual(newRotationDirectionEvents, 4, "NewRotationDirection fired 4 times.");
_.strictEqual(motionEvents, 13, "MotionChange fired 13 times.");
_.strictEqual(movedEvents, 3, "Moved fired 3 times.");
_.strictEqual(rotatedEvents, 3, "Rotated fired 3 times.");
e.destroy();
});
Expand Down

0 comments on commit 6dd3a40

Please sign in to comment.