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

add before/after preload and setup #6433

Merged
merged 14 commits into from
Oct 4, 2023
18 changes: 18 additions & 0 deletions src/core/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,20 @@ class p5 {
this._events.devicemotion = null;
}

// Function to invoke registered hooks before or after events such as preload, setup, and pre/post draw.
p5.prototype.callRegisteredHooksFor = function (hookName) {
const target = this || p5.prototype;
const context = this._isGlobal ? window : this;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, looks like tests are failing because this is undefined. You might need to make this a method on p5.prototype and then call this.callRegisteredHooksFor(...) as a method instead of a function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your help!

if (target._registeredMethods.hasOwnProperty(hookName)) {
const methods = target._registeredMethods[hookName];
for (const method of methods) {
if (typeof method === 'function') {
method.call(context);
}
}
}
};

this._start = () => {
// Find node if id given
if (this._userNode) {
Expand All @@ -241,6 +255,7 @@ class p5 {

const context = this._isGlobal ? window : this;
if (context.preload) {
this.callRegisteredHooksFor('beforePreload');
// Setup loading screen
// Set loading screen into dom if not present
// Otherwise displays and removes user provided loading screen
Expand Down Expand Up @@ -286,6 +301,7 @@ class p5 {
if (loadingScreen) {
loadingScreen.parentNode.removeChild(loadingScreen);
}
this.callRegisteredHooksFor('afterPreload');
if (!this._setupDone) {
this._lastTargetFrameTime = window.performance.now();
this._lastRealFrameTime = window.performance.now();
Expand Down Expand Up @@ -322,6 +338,7 @@ class p5 {
};

this._setup = () => {
this.callRegisteredHooksFor('beforeSetup');
// Always create a default canvas.
// Later on if the user calls createCanvas, this default one
// will be replaced
Expand Down Expand Up @@ -369,6 +386,7 @@ class p5 {
if (this._accessibleOutputs.grid || this._accessibleOutputs.text) {
this._updateAccsOutput();
}
this.callRegisteredHooksFor('afterSetup');
};

this._draw = () => {
Expand Down
8 changes: 2 additions & 6 deletions src/core/structure.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import p5 from './main';

/**
* Stops p5.js from continuously executing the code within <a href="#/p5/draw">draw()</a>.
* If <a href="#/p5/loop">loop()</a> is called, the code in <a href="#/p5/draw">draw()</a>
Expand Down Expand Up @@ -471,9 +470,6 @@ p5.prototype.redraw = function(n) {
if (typeof context.setup === 'undefined') {
context.scale(context._pixelDensity, context._pixelDensity);
}
const callMethod = f => {
f.call(context);
};
for (let idxRedraw = 0; idxRedraw < numberOfRedraws; idxRedraw++) {
context.resetMatrix();
if (this._accessibleOutputs.grid || this._accessibleOutputs.text) {
Expand All @@ -483,14 +479,14 @@ p5.prototype.redraw = function(n) {
context._renderer._update();
}
context._setProperty('frameCount', context.frameCount + 1);
context._registeredMethods.pre.forEach(callMethod);
this.callRegisteredHooksFor('pre');
this._inUserDraw = true;
try {
context.draw();
} finally {
this._inUserDraw = false;
}
context._registeredMethods.post.forEach(callMethod);
this.callRegisteredHooksFor('post');
}
}
};
Expand Down
87 changes: 86 additions & 1 deletion test/unit/core/main.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { expect } = require('chai');
const { expect, assert } = require('chai');

suite('Core', function() {
suite('p5.prototype.registerMethod', function() {
Expand Down Expand Up @@ -31,6 +31,91 @@ suite('Core', function() {
p5.prototype._registeredMethods.init = originalInit;
}
});
test('should register and call before and after "preload" hooks', function() {
return new Promise(resolve => {
let beforePreloadCalled = false;
let preloadCalled = false;
let afterPreloadCalled = false;

p5.prototype.registerMethod('beforePreload', () => {
beforePreloadCalled = true;
});

p5.prototype.registerMethod('preload', () => {
assert.equal(beforePreloadCalled, true);
preloadCalled = true;
});

p5.prototype.registerMethod('afterPreload', () => {
if(beforePreloadCalled && preloadCalled)
afterPreloadCalled = true;
});

myp5 = new p5(function(sketch) {
sketch.preload = () => {};
sketch.setup = () => {
assert.equal(afterPreloadCalled, true);
};
resolve();
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently I think this will resolve before setup or preload gets run. Does it work to move the resolve into sketch.setup after your assertion?

});
});
test('should register and call before and after "setup" hooks', function() {
return new Promise(resolve => {
let beforeSetupCalled = false;
let setupCalled = false;
let afterSetupCalled = false;

p5.prototype.registerMethod('beforeSetup', () => {
beforeSetupCalled = true;
});

p5.prototype.registerMethod('setup', () => {
assert.equal(beforeSetupCalled, true);
setupCalled = true;
});

p5.prototype.registerMethod('afterSetup', () => {
if(beforeSetupCalled && setupCalled)
afterSetupCalled = true;
});

myp5 = new p5(function(sketch) {
sketch.setup = () => {};
sketch.draw = () => {
assert.equal(afterSetupCalled, true);
resolve();
};
});
});
});
test('should register and call pre and post "draw" hooks', function() {
return new Promise(resolve => {
let preDrawCalled = false;
let drawCalled = false;
let postDrawCalled = false;

p5.prototype.registerMethod('pre', () => {
preDrawCalled = true;
});

p5.prototype.registerMethod('draw', () => {
assert.equal(preDrawCalled, true);
drawCalled = true;
});

p5.prototype.registerMethod('post', () => {
if(preDrawCalled && drawCalled)
postDrawCalled = true;
});

myp5 = new p5(function(sketch) {
sketch.draw = () => {};
});
assert.equal(postDrawCalled, true);
resolve();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting a postDraw assertion is going to be a bit tough. The reason why we can't do assertions outside of setup and draw here is that those actually get run first, before draw is called: when we define draw, we're only defining it, not calling it, and it gets called asynchronously later after preload and setup finish. So if we resolve outside of it, then likely we exit before draw gets called at all.

As an alternative: the first time draw gets called, sketch.frameCount will be 1. So we could do something like this, to make sure it gets run after the first draw call:

sketch.draw = () => {
  if (sketch.frameCount === 2) {
    assert.equal(postDrawCalled, true);
    resolve();
  }
};

});
});
});

suite('new p5() / global mode', function() {
Expand Down
Loading