Revert "Revert "Point to S3 DefaultSprites""

Reverts code-dot-org/code-dot-org#47231, which reverted this: https://github.com/code-dot-org/code-dot-org/pull/42835

Merging this caused issues in GameLab: Slack Discussion - (https://codedotorg.slack.com/archives/C1B3PNDL7/p1657765673240019) Turns out, this is not true! https://github.com/code-dot-org/code-dot-org/pull/42835#discussion_r831615599 It's only true on 'fresh' projects, but is not true in GameLab used in the the curriculum.

Error: The error that caused the need to revert. In setInitialAnimationList in animationList.js, we have a migration to convert any defaultSprites that use the v3 animation api to use the v1 animation api. Originally added here: https://github.com/code-dot-org/code-dot-org/pull/35243. This was only intended to be used for SpriteLab projects and animations. Previously in this step, we were comparing any starter animations in Gamelab against the defaultSprites.json list (as the variable spritesForV3Migration) - this had no overlap, so was a no-op execution. However, in the original version of this change we replaced defaultSprites.json with a call to S3. I did not add an S3 call for Gamelab projects since defaultSprites.json is the default list for SpriteLab. However! This meant that the start animations in Gamelab were being compared against an empty array (instead of defaultSprites.json). Because we were expecting a JSON object, calling propsByKey on the empty array, was throwing an error that prevented the page from loading, as outlined in the Slack thread.

The Solution: Since the migration is intended for SpriteLab and compares against the default list of sprites for SpriteLab, I added a check for whether the current project isSpriteLab before progressing with the migration. Now for GameLab, we just skip this migration step and avoid the error.

Working in New GL project:

Working in GL curriculum:

Working in new SL project:

Working in SL curriculum:

Recovery path - Default SpriteLab request to S3 was blocked - Error recorded and backup sprites used.

@mikeharv, I'm going to start chipping away at these fixes, but just to clarify - as a general rule, we now want to call sprites as animations, right?

Sounds good @epeach! I think we should "animation" whenever we're talking about [image+metadata] together. We can further categorize animations into "sprite costumes" and "backgrounds" in Sprite Lab, but should try to only talk about "sprites" when we mean the JS objects created in P5Lab (usually created in Sprite Lab by the student's Blockly code).

Sounds good @epeach! I think we should "animation" whenever we're talking about [image+metadata] together. We can further categorize animations into "sprite costumes" and "backgrounds" in Sprite Lab, but should try to only talk about "sprites" when we mean the JS objects created in P5Lab (usually created in Sprite Lab by the student's Blockly code).

This is great clarification - thank you! Can you post this breakdown in the star-labs Slack channel so we can publicize and practice this vocabulary clarification?

Sounds good @epeach! I think we should "animation" whenever we're talking about [image+metadata] together. We can further categorize animations into "sprite costumes" and "backgrounds" in Sprite Lab, but should try to only talk about "sprites" when we mean the JS objects created in P5Lab (usually created in Sprite Lab by the student's Blockly code).

This is great clarification - thank you! Can you post this breakdown in the star-labs Slack channel so we can publicize and practice this vocabulary clarification?

+1, I definitely don't understand the distinctions here :) Would love a bit more detail in Slack if you have time at some point!

@mikeharv PTAL!

Sounds good @epeach! I think we should "animation" whenever we're talking about [image+metadata] together. We can further categorize animations into "sprite costumes" and "backgrounds" in Sprite Lab, but should try to only talk about "sprites" when we mean the JS objects created in P5Lab (usually created in Sprite Lab by the student's Blockly code).

This is great clarification - thank you! Can you post this breakdown in the star-labs Slack channel so we can publicize and practice this vocabulary clarification?

+1, I definitely don't understand the distinctions here :) Would love a bit more detail in Slack if you have time at some point!

I have also asked Mike about the differences between sprites and animations and I have a much better understanding after our conversation. What do you think of doing a demo of this tool including Mike's UI warning for photo uploads for the next monthly demo? Then Mike could clarify the differences between sprites vs animation for folks.

Woo hoo! Pulled and tested once again and it looks like it's working great! Thanks for doing the rename change as well.

@bencodeorg - would you give a second look at this? I added DCDO flags just in case and it's my first attempt at those. Thanks!

@bencodeorg - would you give a second look at this? I added DCDO flags just in case and it's my first attempt at those. Thanks!

Looks right to me! The goal here is to have the new behavior turned on when this deploys, and have the option to turn it off, yeah? Were you able to hand test flipping the flag locally and seeing the old behavior?

@bencodeorg - Yup! I want to default to using this change and have a back-up plan just in case. I tested locally. In the screenshots, I edited the local defaultSprites.json to be just 3 sprites to easily distinguish when I'm using the new updates (all the animations) and when I switch DCDO to use the old defaultSprites.json (just three animations).

@bencodeorg - Yup! I want to default to using this change and have a back-up plan just in case. I tested locally. In the screenshots, I edited the local defaultSprites.json to be just 3 sprites to easily distinguish when I'm using the new updates (all the animations) and when I switch DCDO to use the old defaultSprites.json (just three animations).

Beautiful! Sounds good to me from a DCDO perspective.