[Stable11.3] Cherry pick asset fixes

pxtblocks/fields/field_animation.ts

@@ -80,7 +80,7 @@ export class FieldAnimationEditor extends FieldAssetEditor<FieldAnimationOptions
             const frames = parseImageArrayString(text, this.params.taggedTemplate);
 
             if (frames && frames.length) {
-                const id = this.sourceBlock_.id;
+                const id = this.temporaryAssetId();
 
                 const newAnimation: pxt.Animation = {
                     internalID: -1,
@@ -97,7 +97,7 @@ export class FieldAnimationEditor extends FieldAssetEditor<FieldAnimationOptions
                 if (asset) return asset;
         }
 
-        const id = this.sourceBlock_.id;
+        const id = this.temporaryAssetId();
         const bitmap = new pxt.sprite.Bitmap(this.params.initWidth, this.params.initHeight).data()
 
         const newAnimation: pxt.Animation = {

pxtblocks/fields/field_asset.ts

@@ -264,7 +264,7 @@ export abstract class FieldAssetEditor<U extends FieldAssetEditorOptions, V exte
     }
 
     protected onFieldEditorHide(fv: pxt.react.FieldEditorView<pxt.Asset>) {
-        const result = fv.getResult();
+        let result = fv.getResult();
         const project = pxt.react.getTilemapProject();
 
         if (this.asset.type === pxt.AssetType.Song) {
@@ -275,15 +275,10 @@ export abstract class FieldAssetEditor<U extends FieldAssetEditorOptions, V exte
             const old = this.getValue();
             if (pxt.assetEquals(this.asset, result)) return;
 
-            const oldId = isTemporaryAsset(this.asset) ? null : this.asset.id;
-            let newId = isTemporaryAsset(result) ? null : result.id;
+            result = pxt.patchTemporaryAsset(this.asset, result, project);
 
-            if (!oldId && newId === this.sourceBlock_.id) {
-                // The temporary assets we create just use the block id as the id; give it something
-                // a little nicer
-                result.id = project.generateNewID(result.type);
-                newId = result.id;
-            }
+            const oldId = isTemporaryAsset(this.asset) ? null : this.asset.id;
+            const newId = isTemporaryAsset(result) ? null : result.id;
 
             this.pendingEdit = true;
 
@@ -552,6 +547,10 @@ export abstract class FieldAssetEditor<U extends FieldAssetEditorOptions, V exte
     protected isFullscreen() {
         return true;
     }
+
+    protected temporaryAssetId() {
+        return this.sourceBlock_.id + "_" + this.name;
+    }
 }
 
 function isTemporaryAsset(asset: pxt.Asset) {

pxtblocks/fields/field_musiceditor.ts

@@ -61,7 +61,7 @@ export class FieldMusicEditor extends FieldAssetEditor<FieldMusicEditorOptions,
 
         const newAsset: pxt.Song = {
             internalID: -1,
-            id: this.sourceBlock_.id,
+            id: this.temporaryAssetId(),
             type: pxt.AssetType.Song,
             meta: {
             },

pxtblocks/fields/field_sprite.ts

@@ -70,7 +70,7 @@ export class FieldSpriteEditor extends FieldAssetEditor<FieldSpriteEditorOptions
 
         const newAsset: pxt.ProjectImage = {
             internalID: -1,
-            id: this.sourceBlock_.id,
+            id: this.temporaryAssetId(),
             type: pxt.AssetType.Image,
             jresData: pxt.sprite.base64EncodeBitmap(data),
             meta: {

pxteditor/monaco-fields/field_musiceditor.ts

@@ -7,19 +7,21 @@ export class MonacoSongEditor extends MonacoReactFieldEditor<pxt.Song> {
     protected isPython: boolean;
     protected isAsset: boolean;
     protected text: string;
+    protected editing: pxt.Asset;
 
     protected textToValue(text: string): pxt.Song {
         this.isPython = text.indexOf("`") === -1
         this.text = text;
 
         const match = pxt.parseAssetTSReference(text);
         if (match) {
-            const { type, name: matchedName } = match;
+            const { name: matchedName } = match;
             const name = matchedName.trim();
             const project = pxt.react.getTilemapProject();
             this.isAsset = true;
             const asset = project.lookupAssetByName(pxt.AssetType.Song, name);
             if (asset) {
+                this.editing = asset;
                 return asset;
             }
             else {
@@ -29,6 +31,8 @@ export class MonacoSongEditor extends MonacoReactFieldEditor<pxt.Song> {
                     newAsset.meta.displayName = name;
                 }
 
+                this.editing = newAsset;
+
                 return newAsset;
             }
         }
@@ -39,18 +43,24 @@ export class MonacoSongEditor extends MonacoReactFieldEditor<pxt.Song> {
             const contents = hexLiteralMatch[1].trim();
 
             if (contents) {
-                return createFakeAsset(pxt.assets.music.decodeSongFromHex(contents));
+                this.editing = createFakeAsset(pxt.assets.music.decodeSongFromHex(contents));
+            }
+            else {
+                this.editing = createFakeAsset(pxt.assets.music.getEmptySong(2));
             }
 
-            return createFakeAsset(pxt.assets.music.getEmptySong(2));
+            return this.editing;
         }
 
         return undefined; // never
     }
 
     protected resultToText(result: pxt.Song): string {
+        const project = pxt.react.getTilemapProject();
+        project.pushUndo();
+
+        result = pxt.patchTemporaryAsset(this.editing, result, project) as pxt.Song;
         if (result.meta?.displayName) {
-            const project = pxt.react.getTilemapProject();
             if (this.isAsset || project.lookupAsset(result.type, result.id)) {
                 result = project.updateAsset(result)
             } else {

pxteditor/monaco-fields/field_sprite.ts

@@ -7,19 +7,21 @@ export class MonacoSpriteEditor extends MonacoReactFieldEditor<pxt.ProjectImage>
     protected isPython: boolean;
     protected isAsset: boolean;
     protected template: string;
+    protected editing: pxt.Asset;
 
     protected textToValue(text: string): pxt.ProjectImage {
         this.isPython = text.indexOf("`") === -1
         this.template = text.startsWith("bmp") ? "bmp" : "img"
 
         const match = pxt.parseAssetTSReference(text);
         if (match) {
-            const { type, name: matchedName } = match;
+            const { name: matchedName } = match;
             const name = matchedName.trim();
             const project = pxt.react.getTilemapProject();
             this.isAsset = true;
             const asset = project.lookupAssetByName(pxt.AssetType.Image, name);
             if (asset) {
+                this.editing = asset;
                 return asset;
             }
             else {
@@ -29,16 +31,23 @@ export class MonacoSpriteEditor extends MonacoReactFieldEditor<pxt.ProjectImage>
                     newAsset.meta.displayName = name;
                 }
 
+                this.editing = newAsset;
+
                 return newAsset;
             }
         }
 
-        return createFakeAsset(pxt.sprite.imageLiteralToBitmap(text, this.template));
+        this.editing = createFakeAsset(pxt.sprite.imageLiteralToBitmap(text, this.template));
+
+        return this.editing;
     }
 
     protected resultToText(result: pxt.ProjectImage): string {
+        const project = pxt.react.getTilemapProject();
+        project.pushUndo();
+        result = pxt.patchTemporaryAsset(this.editing, result, project) as pxt.ProjectImage;
+
         if (result.meta?.displayName) {
-            const project = pxt.react.getTilemapProject();
             if (this.isAsset || project.lookupAsset(result.type, result.id)) {
                 result = project.updateAsset(result)
             } else {

pxteditor/monaco-fields/field_tilemap.ts

@@ -6,13 +6,15 @@ const fieldEditorId = "tilemap-editor";
 export class MonacoTilemapEditor extends MonacoReactFieldEditor<pxt.ProjectTilemap> {
     protected isTilemapLiteral: boolean;
     protected tilemapLiteral: string;
+    protected editing: pxt.Asset;
 
     protected textToValue(text: string): pxt.ProjectTilemap {
         const tm = this.readTilemap(text);
 
         const project = pxt.react.getTilemapProject();
         pxt.sprite.addMissingTilemapTilesAndReferences(project, tm);
 
+        this.editing = tm;
         return tm;
     }
 
@@ -31,7 +33,7 @@ export class MonacoTilemapEditor extends MonacoReactFieldEditor<pxt.ProjectTilem
                     // If the user is still typing, they might try to open the editor on an incomplete tilemap
                 }
                 return null;
-                }
+            }
         }
 
         this.isTilemapLiteral = true;
@@ -71,7 +73,7 @@ export class MonacoTilemapEditor extends MonacoReactFieldEditor<pxt.ProjectTilem
     protected resultToText(asset: pxt.ProjectTilemap): string {
         const project = pxt.react.getTilemapProject();
         project.pushUndo();
-
+        asset = pxt.patchTemporaryAsset(this.editing, asset, project) as pxt.ProjectTilemap;
         pxt.sprite.updateTilemapReferencesFromResult(project, asset);
 
         if (this.isTilemapLiteral) {

pxtlib/spriteutils.ts

@@ -145,7 +145,7 @@ namespace pxt.sprite {
             }
         }
 
-        protected dataLength() {
+        dataLength() {
             return Math.ceil(this.width * this.height / 2);
         }
     }
@@ -179,7 +179,7 @@ namespace pxt.sprite {
             this.buf[index] = value;
         }
 
-        protected dataLength() {
+        dataLength() {
             return this.width * this.height;
         }
     }

pxtlib/tilemap.ts

@@ -154,6 +154,12 @@ namespace pxt {
                 const existing = this.lookupByID(id);
 
                 if (!assetEquals(existing, newValue)) {
+                    if (!validateAsset(newValue) && validateAsset(existing)) {
+                        pxt.warn("Refusing to overwrite asset with invalid version");
+                        pxt.tickEvent("assets.invalidAssetOverwrite", { assetType: newValue.type });
+                        return existing;
+                    }
+
                     this.removeByID(id);
                     asset = this.add(newValue);
                     this.notifyListener(newValue.internalID);
@@ -1709,6 +1715,10 @@ namespace pxt {
 
     export function cloneAsset<U extends Asset>(asset: U): U {
         asset.meta = Object.assign({}, asset.meta);
+        if (asset.meta.temporaryInfo) {
+            asset.meta.temporaryInfo = Object.assign({}, asset.meta.temporaryInfo);
+        }
+
         switch (asset.type) {
             case AssetType.Tile:
             case AssetType.Image:
@@ -1948,6 +1958,69 @@ namespace pxt {
         return getShortIDCore(asset.type, asset.id);
     }
 
+    export function validateAsset(asset: pxt.Asset) {
+        if (!asset) return false;
+
+        switch (asset.type) {
+            case AssetType.Image:
+            case AssetType.Tile:
+                return validateImageAsset(asset as ProjectImage | Tile);
+            case AssetType.Tilemap:
+                return validateTilemap(asset as ProjectTilemap);
+            case AssetType.Animation:
+                return validateAnimation(asset as Animation)
+            case AssetType.Song:
+                return validateSong(asset as Song);
+        }
+    }
+
+    function validateImageAsset(asset: ProjectImage | Tile) {
+        if (!asset.bitmap) return false;
+
+        return validateBitmap(sprite.Bitmap.fromData(asset.bitmap));
+    }
+
+    function validateTilemap(tilemap: ProjectTilemap) {
+        if (
+            !tilemap.data ||
+            !tilemap.data.tilemap ||
+            !tilemap.data.tileset ||
+            !tilemap.data.tileset.tileWidth ||
+            !tilemap.data.tileset.tiles?.length ||
+            !tilemap.data.layers
+        ) {
+            return false;
+        }
+
+        return validateBitmap(sprite.Bitmap.fromData(tilemap.data.layers)) &&
+            validateBitmap(tilemap.data.tilemap);
+    }
+
+    function validateAnimation(animation: Animation) {
+        if (!animation.frames?.length || animation.interval <= 0) {
+            return false;
+        }
+
+        return !animation.frames.some(frame => !validateBitmap(sprite.Bitmap.fromData(frame)));
+    }
+
+    function validateBitmap(bitmap: sprite.Bitmap) {
+        return bitmap.width > 0 &&
+            bitmap.height > 0 &&
+            !Number.isNaN(bitmap.x0) &&
+            !Number.isNaN(bitmap.y0) &&
+            bitmap.data().data.length === bitmap.dataLength();
+    }
+
+    function validateSong(song: Song) {
+        return song.song &&
+            song.song.ticksPerBeat > 0 &&
+            song.song.beatsPerMeasure > 0 &&
+            song.song.measures > 0 &&
+            song.song.beatsPerMinute > 0 &&
+            !!song.song.tracks;
+    }
+
     function getShortIDCore(assetType: pxt.AssetType, id: string, allowNoPrefix = false) {
         let prefix: string;
         switch (assetType) {
@@ -2062,12 +2135,6 @@ namespace pxt {
         }
     }
 
-
-    function set16Bit(buf: Uint8ClampedArray, offset: number, value: number) {
-        buf[offset] = value & 0xff;
-        buf[offset + 1] = (value >> 8) & 0xff;
-    }
-
     function read16Bit(buf: Uint8ClampedArray, offset: number) {
         return buf[offset] | (buf[offset + 1] << 8)
     }
@@ -2087,4 +2154,21 @@ namespace pxt {
             case AssetType.Song: return snapshot.songs;
         }
     }
+
+    export function patchTemporaryAsset(oldValue: pxt.Asset, newValue: pxt.Asset, project: TilemapProject) {
+        if (!oldValue || assetEquals(oldValue, newValue)) return newValue;
+
+        newValue = cloneAsset(newValue);
+        const wasTemporary = !oldValue.meta.displayName;
+        const isTemporary = !newValue.meta.displayName;
+
+        // if we went from being temporary to no longer being temporary,
+        // make sure we replace the junk id with a new value
+        if (wasTemporary && !isTemporary) {
+            newValue.id = project.generateNewID(newValue.type);
+            newValue.internalID = project.getNewInternalId();
+        }
+
+        return newValue;
+    }
 }

webapp/src/components/assetEditor/assetSidebar.tsx

@@ -95,13 +95,17 @@ class AssetSidebarImpl extends React.Component<AssetSidebarProps, AssetSidebarSt
 
         const project = pxt.react.getTilemapProject();
         project.pushUndo();
+        result = pxt.patchTemporaryAsset(this.props.asset, result, project);
+
+        if (result.meta.displayName) {
+            result = project.updateAsset(result);
+        }
+
         if (!this.props.asset.meta?.displayName && result.meta.temporaryInfo) {
             getBlocksEditor().updateTemporaryAsset(result);
             pkg.mainEditorPkg().lookupFile(`this/${pxt.MAIN_BLOCKS}`).setContentAsync(getBlocksEditor().getCurrentSource());
         }
 
-        if (result.meta.displayName) project.updateAsset(result);
-
         this.props.dispatchChangeGalleryView(GalleryView.User);
         this.updateAssets().then(() => simulator.setDirty());
     }