p5.js icon indicating copy to clipboard operation
p5.js copied to clipboard

`_setupDone` flag not set properly

Open quinton-ashley opened this issue 7 months ago • 4 comments

Most appropriate sub-area of p5.js?

  • [ ] Accessibility
  • [ ] Color
  • [x] Core/Environment/Rendering
  • [ ] Data
  • [ ] DOM
  • [ ] Events
  • [ ] Image
  • [ ] IO
  • [ ] Math
  • [ ] Typography
  • [ ] Utilities
  • [ ] WebGL
  • [ ] Build process
  • [ ] Unit testing
  • [ ] Internationalization
  • [ ] Friendly errors
  • [ ] Other (specify if possible)

p5.js version

1.11.7

Web browser and version

Chrome

Operating system

macOS

Steps to reproduce this

_setupDone flag is not being set to true after setup finishes.

Seems to only be an issue in global mode, not with p5 instances.

Example: https://editor.p5js.org/quinton-ashley/sketches/tJkTywzxu

quinton-ashley avatar May 15 '25 16:05 quinton-ashley

Looks like it works as expected if you access it like this:

function setup() {
  createCanvas(400, 400);
  console.log(_renderer._pInst._setupDone);
}

function draw() {
  console.log(_renderer._pInst._setupDone);
  noLoop();
}

So it seems like it's just that the global _setupDone isn't updated. Which might be ok since it's a piece of internal state?

davepagurek avatar May 16 '25 17:05 davepagurek

If _setupDone isn't properly updated globally, or as a property of the p5 instance itself, maybe it should be removed if not repaired?

Even though it's internal state, I didn't expect it to break so late in p5 v1's development.

quinton-ashley avatar May 18 '25 14:05 quinton-ashley

Hi, thank you for opening this up, @quinton-ashley.

function preload() {
  console.log(_preloadDone);
}

function setup() {
  console.log(_preloadDone);
  noLoop();
}

In the above code, the global _preloadDone flag gets updated to true upon completion of preload().

Given this, and considering multiple users have requested similar access, do you think it would be reasonable to set the _setupDone property globally as well for consistency and developer convenience?

https://github.com/processing/p5.js/blob/146f48a3744c4f1f577d2eedf930172b673007e2/src/core/main.js#L497

maybe here we could add a line context._setupDone = true; since when this._isGlobal is present context is window.

https://github.com/processing/p5.js/blob/146f48a3744c4f1f577d2eedf930172b673007e2/src/core/main.js#L349

What are your thoughts on this?

perminder-17 avatar Jun 09 '25 03:06 perminder-17

Well Dave was implying that p5play shouldn't have been relying on the _setupDone internal flag, and I agree.

Yet I still assumed it would always be correct. I didn't know about it being set in the renderer object.

I think in general it's a good practice to avoid duplicating the same property across multiple objects, to avoid confusion and errors like this one, caused by changing it in one place but not the other.

quinton-ashley avatar Jun 09 '25 03:06 quinton-ashley

@ksen0 pointed out that _setupDone was never set properly globally (fine since it's internal state anyway) but that means my example wasn't good (because the prop is a boolean, p._setupDone and global _setupDone don't reference the same data.)

p5play was actually checking p._setupDone on the p5 instance, but it only stopped working with global mode.

The easy fix was to just set the flag properly in p5play.

In p5 v2 already there's no "private" props exported to the global scope, which is good.

Related discussion: #7929

quinton-ashley avatar Jun 21 '25 00:06 quinton-ashley