Cache sounds before running user code #243
Conversation
|
|
||
| if (this.imageCache[file]) { | ||
| return loaded(); | ||
| } |
There was a problem hiding this comment.
changed so that this.imageCache[file] = img and if (this.imageCache[file]) { use the same file
create loadImage/loadSound methods which return promises, use $.when to wait for all promises to resolve before calling the callback
… dry-ed out the regex code some more, don't run tests if the validate string is empty
| this.results.tests = testResults; | ||
| this.phoneHome(); | ||
| }.bind(this)); | ||
| } |
There was a problem hiding this comment.
This isn't necessary for resource caching. It should've been included in the scrubber performance pull request. I'm including it here because I don't want to forget about.
…match both getImage() and getSound() with a single regex to create a single array of resource records which are then loaded by loadResource(record) which multiplexes loadImage and loadSound, went back to caching <img> tags instead of PImage instances.
|
|
||
| var img = document.createElement("img"); | ||
| img.onload = function() { | ||
| this.cache[filename + ".png"] = img; |
There was a problem hiding this comment.
Reverted to caching elements. Investigating whether PImages can be cached or not shouldn't hold up a bug fix for loading sounds.
|
This should be good to go. |
| return this.getSound(filename); | ||
| break; | ||
| default: | ||
| throw "we can't load '" + type + "' resources yet"; |
There was a problem hiding this comment.
Please wrap in $._() for internationalization.
There was a problem hiding this comment.
A user should never see this. All of the resources that are matched are either "sounds" or "images"... I was thinking of just doing break; for the default case. Is there a way to log errors that users shouldn't see but might be useful for us back to a server?
There was a problem hiding this comment.
Ah, okay. We do automatically send JS errors off to Sentry though we dont necessarily use that information yet.
There was a problem hiding this comment.
I had a quick search through live-editor and it doesn't look like we're using Sentry anywhere. I'm going to get rid of the throw and put a TODO there instead and track it with a Github issue for add error tracking via Sentry.
There was a problem hiding this comment.
Oh, I think we're likely hooking in globally on KA through window.onerror or such. Anyway, a break is fine. I'll find out about issues from students and ZenDesk tickets.
…ource cache instance, removed a "throw" which will never happen
|
Is this PR ready to go? I can merge/release it this week if so. |
|
I believe this is good to go. |
Cache sounds before running user code
Attempt to address #223.