There's no way to tell when an SVG skin is loaded

[adroitwhiz]

Expected Behavior

There should be some method by which API consumers like scratch-vm can create an SVG skin and run some other code only after the SVG skin is loaded.

Actual Behavior

Since an SVG skin's rendering is done asynchronously, there's a delay between when createSVGSkin is called and the SVG skin is actually renderable.

Right now, createSVGSkin returns the skin's ID immediately. It would be helpful to have a method (called awaitSVGSkin or similar) which creates an SVG skin and returns a Promise<skinID>, to ensure that the SVG skin is actually loaded.

Currently, vm.loadProject doesn't wait for SVG skins to load, which could cause potential test flakiness if tests are run before the skins are loaded. If an asynchronous SVG skin loading API is added, this could easily be fixed on the VM side by replacing this one call site.

[BryceLTaylor]

@adroitwhiz Is this blocking other work? Are there issues that you could link to for that other work?

[adroitwhiz]

I ran into this when working on #537-- as far as I can tell, the current test setup works by coincidence: it waits for a certain element to appear, which just so happens to take long enough that all skins are loaded.

In #537, I've added a workaround to the test code, but it's kind of kludgey.

This isn't blocking anything in particular, but if you're willing to accept a PR (and help decide on the nicest/least weird API design), I'd be glad to submit one.

[adroitwhiz]

The hardest part of this is just figuring out the nicest API surface:

• createBitmapSkin and createSVGSkin can both easily be made asynchronous, but createPenSkin and especially createTextSkin should remain synchronous
• Only SVG skins load asynchronously-- the other three types of skins can be created synchronously
• Having a mix of create[...]Skin methods, some of which return a skinId, and the rest of which return a Promise<skinId>, would likely cause developer confusion
• Outside of the test playground, there's only one place in the public Scratch codebase that calls createSVGSkin currently, so if it was switched to a new, differently named asynchronous method (awaitSVGSkin), createSVGSkin would become dead code
  • But removing createSVGSkin would create an inconsistent API surface