Do we really need to wait for the ensembleLoaded event?

[meldckn]

Arguably comparable JS libraries (like p5js, nlp-compromise, Ritajs, Tracery, etc), don't require you to explicitly wait for their library to load before calling any of their functions. It is sufficient to just include their script tags in your main HTML in order from library to code-that-uses-library.

But Ensemble requires the unusual pattern of waiting for a custom event (ensembleLoaded) that is triggered at the end of the standalone ensemble.js file, and then putting any ensemble function calls in the scope of that event handler's callback. You can also wait for the Window load event (but that waits for every resource, like images if you have an image-heavy game, so the custom event is probably better). 

Btw, sometimes your webpage will successfully load with just the well-ordered script tags and no ensembleLoaded event listener. But I found that it would fail to load in the correct order maybe 50% of the time when you refresh the page many times in a row, throwing Uncaught ReferenceError: ensemble is not defined in the failure case. (This was on Chrome on macOS 10.14.5 with the sampleGame.)

Investigate why you need to explicitly wait for ensemble to load with ensembleLoaded or window load event, so that we could potentially remove this odd pattern in the future.

Max's suggestion: Maybe ensemble somehow defers executing itself until after pageload/another page lifecycle event for some reason?

[mkremins]

I may have figured this one out! If you do a bit of script surgery on the compiled ensemble.js file, specifically by changing the penultimate line at the end:

require(["js/ensemble/ensemble"]);

...to this instead (removing the square brackets):

require("js/ensemble/ensemble");

...then the global ensemble module will become available immediately once the <script src="ensemble.js"></script> tag is processed.

As far as I can tell, this is because square brackets around the name of the module you're trying to load in a require call are for some reason taken by requirejs to indicate that it should load the module asynchronously instead of synchronously.

More information here: https://github.com/requirejs/almond/issues/42#issuecomment-9498562

If this is actually the cause, then we need to somehow tweak our build process to produce a version of the standalone compiled ensemble.js script without the offending square brackets in it. Haven't made any attempt to figure out how we might go about doing this yet.

[mkremins]

Commit https://github.com/ensemble-engine/ensemble/commit/fed07bb30823e22bcfc656ad9331817714ff1155 represents my first pass at trying to fix this, and seems to work on my machine. @meldckn, could you confirm it works for you as well?

To test the changes:

• Pull down the latest version of the repo.
• Run grunt deploy.
• Try to load the new built version of Ensemble (created at bin/ensemble.js) with a simple <script src="ensemble.js"></script> tag.

Your scripts should now be able to access the ensemble global immediately after this script tag is run, rather than having to wait for the ensembleLoaded event.

Note that this commit ends up double-require()ing the core js/ensemble/ensemble module at the end of the built library file: once asynchronously (the same async require([]) call that was being inserted automatically before), then synchronously immediately afterward. In other words, the last three lines of the file now look like this:

require(["js/ensemble/ensemble"]);
return require("js/ensemble/ensemble");
}());

I don't think this should cause any actual problems, but it's kind of ugly, and it'd be nice to be able to remove the async require([]) call entirely.

For more information on how this commit was implemented, using the wrap.start and wrap.end build config functionality from almond and r.js:

• https://github.com/requirejs/almond#exporting-a-public-api
• https://github.com/requirejs/r.js/blob/master/build/example.build.js#L470

[meldckn]

That did it! Woop! Thanks so much for looking into that, Max!

(Also, do you think it would be possible to remove RequireJS entirely? Isn't all it's doing concatenating JS files in the order specified by ensemble-config.js?)

[mkremins]

Removing RequireJS entirely is almost certainly possible, but nontrivial. Right now all of the Ensemble JS files are using RequireJS to get access to their dependencies and exporting themselves as RequireJS modules using define. We'd have to work out the dependency order for ourselves (to ensure that the files are concatenated in the correct order), then go through every Ensemble JS file (including core, test and console/tool files) to strip out the define calls and explicitly export these modules as variables instead.

Related to this issue is the problem of stripping out unused or mostly-unused external dependencies from the Ensemble JS files, as discussed in #36. I've started going through and marking which of the functions from these large external libraries we actually use, and in many cases we're not using them for very much – but if we wanted to drop our external dependencies altogether, we'd still have to grab the functions we actually are using and put them in some sort of internal utils file.

[meldckn]

Hmmm got it. Thanks for the thoughtful explanation and new issue! I wonder how modern JS libraries build their standalone files nowadays.

Do you think, as we're making updates generally, that we should strive for backwards-compatibility? Like, should we remove the ensembleLoaded event-triggering? Or keep it and like, for renamed functions for example, keep the old ones around as functions that just call the new/renamed ones? Ensemble's pretty scant current user base might be a good opportunity for a decluttering mindset, but I'm still torn.

If we're keeping it around, seems like this issue can be closed. 🎉

And we'll remove ensembleLoaded from the wiki API docs, README, etc, when we create a new release with this commit included.

[mkremins]

My inclination is to go ahead and remove the ensembleLoaded event entirely now that it's no longer needed. The current userbase is probably as small as it's ever going to be, so I think we should get our breaking changes out of the way ASAP, before it becomes too late.

In this case specifically, the browser-API-reliant code that creates and dispatches the ensembleLoaded event also serves as a minor barrier to the use of Ensemble as a server-side/Node.js module outside of a web browser context.