Skip to content

Commit

Permalink
Handle restart more gracefully
Browse files Browse the repository at this point in the history
This fixes a bundle of bugs related to calling `Craft.stop()` and then `Crafty.init()`.

- Clear global bound events on stop (regression caused by the new callback methods)
- Add a `Crafty._preBind` method for internal use -- anything registered here will be bound when `Crafty.init()` is called (unless we did a soft stop).  This works around a problem where we bound some events only once, when crafty.js was loaded.  (Currently used in inputs.js and viewport.js)
- Make sure that "CraftyStop" is triggered *before* we remove all event handlers!  (This was messing up cleanup of dom event handlers, which in turn messed up registering new ones.)
- Delete references to previously created graphics layers
- Have graphics layers set their initial values in their init functions

Some of these are really work arounds for how we initialize graphics layers, the viewport, etc.  I think if we switch these to using the new systems idiom we can remove a lot of the changes above.  (Definitely the "prebind" hack.)
  • Loading branch information
starwed committed Sep 27, 2015
1 parent 120d227 commit 30fe086
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 26 deletions.
5 changes: 2 additions & 3 deletions src/controls/inputs.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ Crafty.extend({
* @see Crafty.multitouch
*/
mouseDispatch: function (e) {

if (!Crafty.mouseObjs) return;
Crafty.lastEvent = e;

Expand Down Expand Up @@ -592,7 +591,7 @@ Crafty.extend({
});

//initialize the input events onload
Crafty.bind("Load", function () {
Crafty._preBind("Load", function () {
Crafty.addEvent(this, "keydown", Crafty.keyboardDispatch);
Crafty.addEvent(this, "keyup", Crafty.keyboardDispatch);

Expand All @@ -616,7 +615,7 @@ Crafty.bind("Load", function () {
Crafty.addEvent(this, Crafty.stage.elem, "mousewheel", Crafty.mouseWheelDispatch);
});

Crafty.bind("CraftyStop", function () {
Crafty._preBind("CraftyStop", function () {
Crafty.removeEvent(this, "keydown", Crafty.keyboardDispatch);
Crafty.removeEvent(this, "keyup", Crafty.keyboardDispatch);

Expand Down
42 changes: 38 additions & 4 deletions src/core/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1136,6 +1136,16 @@ Crafty.extend({
* @see Crafty.stop, Crafty.viewport
*/
init: function (w, h, stage_elem) {

// If necessary, attach any event handlers registered before Crafty started
if (!this._preBindDone) {
for(var i = 0; i < this._bindOnInit.length; i++) {

var preBind = this._bindOnInit[i];
Crafty.bind(preBind.event, preBind.handler);
}
}

Crafty.viewport.init(w, h, stage_elem);

//call all arbitrary functions attached to onload
Expand All @@ -1145,6 +1155,17 @@ Crafty.extend({
return this;
},

// There are some events that need to be bound to Crafty when it's started/restarted, so store them here
// Switching Crafty's internals to use the new system idiom should allow removing this hack
_bindOnInit: [],
_preBindDone: false,
_preBind: function(event, handler) {
this._bindOnInit.push({
event: event,
handler: handler
});
},

/**@
* #Crafty.getVersion
* @category Core
Expand All @@ -1165,7 +1186,7 @@ Crafty.extend({
/**@
* #Crafty.stop
* @category Core
* @trigger CraftyStop - when the game is stopped
* @trigger CraftyStop - when the game is stopped - {bool clearState}
* @sign public this Crafty.stop([bool clearState])
* @param clearState - if true the stage and all game state is cleared.
*
Expand All @@ -1175,19 +1196,32 @@ Crafty.extend({
* @see Crafty.init
*/
stop: function (clearState) {
Crafty.trigger("CraftyStop", clearState);

this.timer.stop();
if (clearState) {
// Remove audio
Crafty.audio.remove();

// Remove the stage element, and re-add a div with the same id
if (Crafty.stage && Crafty.stage.elem.parentNode) {
var newCrStage = document.createElement('div');
newCrStage.id = Crafty.stage.elem.id;
Crafty.stage.elem.parentNode.replaceChild(newCrStage, Crafty.stage.elem);
}
initState();
}

Crafty.trigger("CraftyStop");
// Reset references to the now destroyed graphics layers
delete Crafty.canvasLayer.context;
delete Crafty.domLayer._div;
delete Crafty.webgl.context;

// reset callbacks, and indicate that prebound functions need to be bound on init again
Crafty._unbindAll();
Crafty._addCallbackMethods(Crafty);
this._preBindDone = false;

initState();
}
return this;
},

Expand Down
24 changes: 9 additions & 15 deletions src/core/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,22 +177,18 @@ module.exports = {

//save anonymous function to be able to remove
var afn = function (e) {
e = e || window.event;

if (typeof callback === 'function') {
callback.call(ctx, e);
}
callback.call(ctx, e);
},
id = ctx[0] || "";

if (!this._events[id + obj + type + callback]) this._events[id + obj + type + callback] = afn;
else return;

if (obj.attachEvent) { //IE
obj.attachEvent('on' + type, afn);
} else { //Everyone else
obj.addEventListener(type, afn, false);
if (!this._events[id + obj + type + callback])
this._events[id + obj + type + callback] = afn;
else {
return;
}

obj.addEventListener(type, afn, false);

},

/**@
Expand Down Expand Up @@ -222,9 +218,7 @@ module.exports = {
afn = this._events[id + obj + type + callback];

if (afn) {
if (obj.detachEvent) {
obj.detachEvent('on' + type, afn);
} else obj.removeEventListener(type, afn, false);
obj.removeEventListener(type, afn, false);
delete this._events[id + obj + type + callback];
}
},
Expand Down
5 changes: 5 additions & 0 deletions src/graphics/canvas-layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ Crafty.extend({
return;
}

// set properties to initial values -- necessary on a restart
this._dirtyRects = [];
this._changedObjs = [];
this.layerCount = 0;

//create an empty canvas element
var c;
c = document.createElement("canvas");
Expand Down
4 changes: 4 additions & 0 deletions src/graphics/dom-layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ Crafty.extend({
_div: null,

init: function () {
// Set properties to initial values -- necessary on a restart
this._changedObjs = [];
this._dirtyViewport = false;

// Create the div that will contain DOM elements
var div = this._div = document.createElement("div");

Expand Down
14 changes: 10 additions & 4 deletions src/graphics/viewport.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ Crafty.extend({
Crafty.unbind("EnterFrame", enterFrame);
}

Crafty.bind("StopCamera", stopPan);
Crafty._preBind("StopCamera", stopPan);

return function (dx, dy, time, easingFn) {
// Cancel any current camera control
Expand Down Expand Up @@ -248,7 +248,7 @@ Crafty.extend({
}
}

Crafty.bind("StopCamera", stopFollow);
Crafty._preBind("StopCamera", stopFollow);

return function (target, offsetx, offsety) {
if (!target || !target.has('2D'))
Expand Down Expand Up @@ -320,7 +320,7 @@ Crafty.extend({
function stopZoom(){
Crafty.unbind("EnterFrame", enterFrame);
}
Crafty.bind("StopCamera", stopZoom);
Crafty._preBind("StopCamera", stopZoom);

var startingZoom, finalZoom, finalAmount, startingX, finalX, startingY, finalY, easing;

Expand Down Expand Up @@ -528,11 +528,17 @@ Crafty.extend({
init: function (w, h, stage_elem) {
// setters+getters for the viewport
this._defineViewportProperties();

// Set initial values -- necessary on restart
this._x = 0;
this._y = 0;
this._scale = 1;
this.bounds = null;

// If no width or height is defined, the width and height is set to fullscreen
this._width = w || window.innerWidth;
this._height = h || window.innerHeight;


//check if stage exists
if (typeof stage_elem === 'undefined')
stage_elem = "cr-stage";
Expand Down
3 changes: 3 additions & 0 deletions src/graphics/webgl.js
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,9 @@ Crafty.extend({
return;
}

// necessary on restart
this.changed_objects = [];

//create an empty canvas element
var c;
c = document.createElement("canvas");
Expand Down

0 comments on commit 30fe086

Please sign in to comment.