diff --git a/src/sprites/rendered-target.js b/src/sprites/rendered-target.js
index b0c1ebfed..495cc0a0f 100644
--- a/src/sprites/rendered-target.js
+++ b/src/sprites/rendered-target.js
@@ -505,18 +505,28 @@ class RenderedTarget extends Target {
return null;
}
- const deletedCostume = this.sprite.deleteCostumeAt(index);
+ const deletedCostume = this.sprite.deleteCostumeAt(index, true); // Update current costume
- if (index === this.currentCostume && index === originalCostumeCount - 1) {
+ this.runtime.requestTargetsUpdate(this);
+ return deletedCostume;
+ }
+
+ /**
+ * Shift current costume index if needed and re-render this target.
+ * This needs to be called on all clones to prevent desync crash. (#2661)
+ * @param {number} index Index of deleted costume
+ * @param {boolean} isLastCostume Whether the deleted costume was the last costume or not
+ */
+ shiftCurrentCostume (index, isLastCostume) {
+ if (index === this.currentCostume && isLastCostume) {
this.setCostume(index - 1);
} else if (index < this.currentCostume) {
this.setCostume(this.currentCostume - 1);
} else {
+ // Index is still the same; however, the costume itself may have changed.
+ // This causes the renderer to re-render.
this.setCostume(this.currentCostume);
}
-
- this.runtime.requestTargetsUpdate(this);
- return deletedCostume;
}
/**
@@ -630,14 +640,15 @@ class RenderedTarget extends Target {
if (newIndex === costumeIndex) return false;
- const currentCostume = this.getCurrentCostume();
+ this.sprite.saveCurrentCostumeName();
const costume = this.sprite.costumes[costumeIndex];
// Use the sprite method for deleting costumes because setCostume is handled manually
this.sprite.deleteCostumeAt(costumeIndex);
this.addCostume(costume, newIndex);
- this.currentCostume = this.getCostumeIndexByName(currentCostume.name);
+ // This sets currentCostume
+ this.sprite.restoreCurrentCostume();
return true;
}
diff --git a/src/sprites/sprite.js b/src/sprites/sprite.js
index a38fa129c..904401b9b 100644
--- a/src/sprites/sprite.js
+++ b/src/sprites/sprite.js
@@ -92,12 +92,43 @@ class Sprite {
}
/**
- * Delete a costume by index.
+ * Delete a costume by index, and optionally update current costume for all clones.
* @param {number} index Costume index to be deleted
+ * @param {boolean} [optUpdateCurrentCostume] Whether or not to update current costume for all clones
* @return {?object} The deleted costume
*/
- deleteCostumeAt (index) {
- return this.costumes.splice(index, 1)[0];
+ deleteCostumeAt (index, optUpdateCurrentCostume) {
+ const isLastCostume = index === this.costumes.length - 1;
+ const deletedCostume = this.costumes.splice(index, 1)[0];
+ if (optUpdateCurrentCostume) {
+ for (const target of this.clones) {
+ target.shiftCurrentCostume(index, isLastCostume);
+ }
+ }
+ return deletedCostume;
+ }
+
+ /**
+ * Save current costume temporarliy during reordering, for all clones.
+ */
+ saveCurrentCostumeName () {
+ for (const target of this.clones) {
+ target._currentCostumeName = target.getCurrentCostume().name;
+ }
+ }
+
+ /**
+ * Restore current costume from saved name.
+ */
+ restoreCurrentCostume () {
+ for (const target of this.clones) {
+ if (typeof target._currentCostumeName === 'undefined') continue;
+ const costumeIndex = this.costumes.findIndex(c => c.name === target._currentCostumeName);
+ delete target._currentCostumeName;
+ if (costumeIndex !== target.currentCostume) {
+ target.setCostume(costumeIndex);
+ }
+ }
}
/**
diff --git a/test/unit/sprites_rendered-target.js b/test/unit/sprites_rendered-target.js
index 681a10d86..914272745 100644
--- a/test/unit/sprites_rendered-target.js
+++ b/test/unit/sprites_rendered-target.js
@@ -119,6 +119,7 @@ test('deleteCostume', t => {
const a = new RenderedTarget(s, r);
const renderer = new FakeRenderer();
a.renderer = renderer;
+ s.clones = [a];
// x* Costume 1 * Costume 2
// Costume 2 => Costume 3
@@ -211,6 +212,70 @@ test('deleteCostume', t => {
t.end();
});
+test('deleteCostume shifts current costume (1/2)', t => {
+ const o1 = {id: 1};
+ const o2 = {id: 2};
+ const o3 = {id: 3};
+
+ const r = new Runtime();
+ const s = new Sprite(null, r);
+ s.costumes = [o1, o2, o3];
+ const renderer = new FakeRenderer();
+
+ const c1 = new RenderedTarget(s, r);
+ c1.renderer = renderer;
+ c1.currentCostume = 0;
+
+ const c2 = new RenderedTarget(s, r);
+ c2.renderer = renderer;
+ c2.currentCostume = 1;
+
+ const c3 = new RenderedTarget(s, r);
+ c3.renderer = renderer;
+ c3.currentCostume = 2;
+
+ s.clones = [c1, c2, c3];
+
+ c1.deleteCostume(0);
+ t.equals(c1.currentCostume, 0);
+ t.equals(c2.currentCostume, 0);
+ t.equals(c3.currentCostume, 1);
+
+ t.end();
+});
+
+test('deleteCostume shifts current costume (2/2)', t => {
+ const o1 = {id: 1};
+ const o2 = {id: 2};
+ const o3 = {id: 3};
+
+ const r = new Runtime();
+ const s = new Sprite(null, r);
+ s.costumes = [o1, o2, o3];
+ const renderer = new FakeRenderer();
+
+ const c1 = new RenderedTarget(s, r);
+ c1.renderer = renderer;
+ c1.currentCostume = 0;
+
+ const c2 = new RenderedTarget(s, r);
+ c2.renderer = renderer;
+ c2.currentCostume = 1;
+
+ const c3 = new RenderedTarget(s, r);
+ c3.renderer = renderer;
+ c3.currentCostume = 2;
+
+ s.clones = [c1, c2, c3];
+
+ c1.deleteCostume(2);
+ t.equals(c1.currentCostume, 0);
+ t.equals(c2.currentCostume, 1);
+ t.equals(c3.currentCostume, 1);
+
+ t.end();
+});
+
test('deleteSound', t => {
const o1 = {id: 1};
const o2 = {id: 2};
@@ -474,12 +539,16 @@ test('#reorderCostume', t => {
const r = new Runtime();
const s = new Sprite(null, r);
s.costumes = [o1, o2, o3, o4, o5];
- const a = new RenderedTarget(s, r);
const renderer = new FakeRenderer();
+ const a = new RenderedTarget(s, r);
a.renderer = renderer;
+ const b = new RenderedTarget(s, r);
+ b.renderer = renderer;
+ s.clones = [a, b];
const resetCostumes = () => {
a.setCostume(0);
+ b.setCostume(0);
s.costumes = [o1, o2, o3, o4, o5];
};
const costumeIds = () => a.sprite.costumes.map(c => c.id);
@@ -487,6 +556,7 @@ test('#reorderCostume', t => {
resetCostumes();
t.deepEquals(costumeIds(), [0, 1, 2, 3, 4]);
t.equals(a.currentCostume, 0);
+ t.equals(b.currentCostume, 0);
// Returns false if the costumes are the same and no change occurred
t.equal(a.reorderCostume(3, 3), false);
@@ -495,15 +565,19 @@ test('#reorderCostume', t => {
// Make sure reordering up and down works and current costume follows
resetCostumes();
+ b.setCostume(4);
t.equal(a.reorderCostume(0, 3), true);
t.deepEquals(costumeIds(), [1, 2, 3, 0, 4]);
t.equals(a.currentCostume, 3); // Index of id=0
+ t.equals(b.currentCostume, 4); // Index of id=4
resetCostumes();
a.setCostume(1);
+ b.setCostume(3);
t.equal(a.reorderCostume(3, 1), true);
t.deepEquals(costumeIds(), [0, 3, 1, 2, 4]);
t.equals(a.currentCostume, 2); // Index of id=1
+ t.equals(b.currentCostume, 1); // Index of id=3
// Out of bounds indices get clamped
resetCostumes();