diff --git a/package.json b/package.json index ff1f895fe..d51b27e2e 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "scratch-audio": "latest", "scratch-blocks": "latest", "scratch-render": "latest", - "scratch-storage": "^0.1.0", + "scratch-storage": "^0.2.0", "script-loader": "0.7.0", "socket.io-client": "1.7.3", "stats.js": "^0.17.0", diff --git a/src/blocks/scratch3_control.js b/src/blocks/scratch3_control.js index f3b53baa2..975dfeb81 100644 --- a/src/blocks/scratch3_control.js +++ b/src/blocks/scratch3_control.js @@ -1,5 +1,4 @@ const Cast = require('../util/cast'); -const Timer = require('../util/timer'); class Scratch3ControlBlocks { constructor (runtime) { @@ -73,18 +72,13 @@ class Scratch3ControlBlocks { util.startBranch(1, true); } - wait (args, util) { - if (!util.stackFrame.timer) { - util.stackFrame.timer = new Timer(); - util.stackFrame.timer.start(); - util.yield(); - this.runtime.requestRedraw(); - } else { - const duration = Math.max(0, 1000 * Cast.toNumber(args.DURATION)); - if (util.stackFrame.timer.timeElapsed() < duration) { - util.yield(); - } - } + wait (args) { + const duration = Math.max(0, 1000 * Cast.toNumber(args.DURATION)); + return new Promise(resolve => { + setTimeout(() => { + resolve(); + }, duration); + }); } if (args, util) { diff --git a/src/blocks/scratch3_wedo2.js b/src/blocks/scratch3_wedo2.js new file mode 100644 index 000000000..d79ef9096 --- /dev/null +++ b/src/blocks/scratch3_wedo2.js @@ -0,0 +1,700 @@ +const color = require('../util/color'); +const log = require('../util/log'); + +/** + * Manage power, direction, and timers for one WeDo 2.0 motor. + */ +class WeDo2Motor { + /** + * Construct a WeDo2Motor instance. + * @param {WeDo2} parent - the WeDo 2.0 device which owns this motor. + * @param {int} index - the zero-based index of this motor on its parent device. + */ + constructor (parent, index) { + /** + * The WeDo 2.0 device which owns this motor. + * @type {WeDo2} + * @private + */ + this._parent = parent; + + /** + * The zero-based index of this motor on its parent device. + * @type {int} + * @private + */ + this._index = index; + + /** + * This motor's current direction: 1 for "this way" or -1 for "that way" + * @type {number} + * @private + */ + this._direction = 1; + + /** + * This motor's current power level, in the range [0,100]. + * @type {number} + * @private + */ + this._power = 100; + + /** + * Is this motor currently moving? + * @type {boolean} + * @private + */ + this._isOn = false; + + /** + * If the motor has been turned on or is actively braking for a specific duration, this is the timeout ID for + * the end-of-action handler. Cancel this when changing plans. + * @type {Object} + * @private + */ + this._pendingTimeoutId = null; + + this.startBraking = this.startBraking.bind(this); + this.setMotorOff = this.setMotorOff.bind(this); + } + + /** + * @return {number} - the duration of active braking after a call to startBraking(). Afterward, turn the motor off. + * @constructor + */ + static get BRAKE_TIME_MS () { + return 1000; + } + + /** + * @return {int} - this motor's current direction: 1 for "this way" or -1 for "that way" + */ + get direction () { + return this._direction; + } + + /** + * @param {int} value - this motor's new direction: 1 for "this way" or -1 for "that way" + */ + set direction (value) { + if (value < 0) { + this._direction = -1; + } else { + this._direction = 1; + } + } + + /** + * @return {int} - this motor's current power level, in the range [0,100]. + */ + get power () { + return this._power; + } + + /** + * @param {int} value - this motor's new power level, in the range [0,100]. + */ + set power (value) { + this._power = Math.max(0, Math.min(value, 100)); + } + + /** + * @return {boolean} - true if this motor is currently moving, false if this motor is off or braking. + */ + get isOn () { + return this._isOn; + } + + /** + * Turn this motor on indefinitely. + */ + setMotorOn () { + this._parent._send('motorOn', {motorIndex: this._index, power: this._direction * this._power}); + this._isOn = true; + this._clearTimeout(); + } + + /** + * Turn this motor on for a specific duration. + * @param {number} milliseconds - run the motor for this long. + */ + setMotorOnFor (milliseconds) { + milliseconds = Math.max(0, milliseconds); + this.setMotorOn(); + this._setNewTimeout(this.startBraking, milliseconds); + } + + /** + * Start active braking on this motor. After a short time, the motor will turn off. + */ + startBraking () { + this._parent._send('motorBrake', {motorIndex: this._index}); + this._isOn = false; + this._setNewTimeout(this.setMotorOff, WeDo2Motor.BRAKE_TIME_MS); + } + + /** + * Turn this motor off. + */ + setMotorOff () { + this._parent._send('motorOff', {motorIndex: this._index}); + this._isOn = false; + } + + /** + * Clear the motor action timeout, if any. Safe to call even when there is no pending timeout. + * @private + */ + _clearTimeout () { + if (this._pendingTimeoutId !== null) { + clearTimeout(this._pendingTimeoutId); + this._pendingTimeoutId = null; + } + } + + /** + * Set a new motor action timeout, after clearing an existing one if necessary. + * @param {Function} callback - to be called at the end of the timeout. + * @param {int} delay - wait this many milliseconds before calling the callback. + * @private + */ + _setNewTimeout (callback, delay) { + this._clearTimeout(); + const timeoutID = setTimeout(() => { + if (this._pendingTimeoutId === timeoutID) { + this._pendingTimeoutId = null; + } + callback(); + }, delay); + this._pendingTimeoutId = timeoutID; + } +} + +/** + * Manage communication with a WeDo 2.0 device over a Device Manager client socket. + */ +class WeDo2 { + + /** + * @return {string} - the type of Device Manager device socket that this class will handle. + */ + static get DEVICE_TYPE () { + return 'wedo2'; + } + + /** + * Construct a WeDo2 communication object. + * @param {Socket} socket - the socket for a WeDo 2.0 device, as provided by a Device Manager client. + */ + constructor (socket) { + /** + * The socket-IO socket used to communicate with the Device Manager about this device. + * @type {Socket} + * @private + */ + this._socket = socket; + + /** + * The motors which this WeDo 2.0 could possibly have. + * @type {[WeDo2Motor]} + * @private + */ + this._motors = [new WeDo2Motor(this, 0), new WeDo2Motor(this, 1)]; + + /** + * The most recently received value for each sensor. + * @type {Object.<string, number>} + * @private + */ + this._sensors = { + tiltX: 0, + tiltY: 0, + distance: 0 + }; + + this._onSensorChanged = this._onSensorChanged.bind(this); + this._onDisconnect = this._onDisconnect.bind(this); + + this._connectEvents(); + } + + /** + * Manually dispose of this object. + */ + dispose () { + this._disconnectEvents(); + } + + /** + * @return {number} - the latest value received for the tilt sensor's tilt about the X axis. + */ + get tiltX () { + return this._sensors.tiltX; + } + + /** + * @return {number} - the latest value received for the tilt sensor's tilt about the Y axis. + */ + get tiltY () { + return this._sensors.tiltY; + } + + /** + * @return {number} - the latest value received from the distance sensor. + */ + get distance () { + return this._sensors.distance; + } + + /** + * Access a particular motor on this device. + * @param {int} index - the zero-based index of the desired motor. + * @return {WeDo2Motor} - the WeDo2Motor instance, if any, at that index. + */ + motor (index) { + return this._motors[index]; + } + + /** + * Set the WeDo 2.0 hub's LED to a specific color. + * @param {int} rgb - a 24-bit RGB color in 0xRRGGBB format. + */ + setLED (rgb) { + this._send('setLED', {rgb}); + } + + /** + * Play a tone from the WeDo 2.0 hub for a specific amount of time. + * @param {int} tone - the pitch of the tone, in Hz. + * @param {int} milliseconds - the duration of the note, in milliseconds. + */ + playTone (tone, milliseconds) { + this._send('playTone', {tone, ms: milliseconds}); + } + + /** + * Stop the tone playing from the WeDo 2.0 hub, if any. + */ + stopTone () { + this._send('stopTone'); + } + + /** + * Attach event handlers to the device socket. + * @private + */ + _connectEvents () { + this._socket.on('sensorChanged', this._onSensorChanged); + this._socket.on('deviceWasClosed', this._onDisconnect); + this._socket.on('disconnect', this._onDisconnect); + } + + /** + * Detach event handlers from the device socket. + * @private + */ + _disconnectEvents () { + this._socket.off('sensorChanged', this._onSensorChanged); + this._socket.off('deviceWasClosed', this._onDisconnect); + this._socket.off('disconnect', this._onDisconnect); + } + + /** + * Store the sensor value from an incoming 'sensorChanged' event. + * @param {object} event - the 'sensorChanged' event. + * @property {string} sensorName - the name of the sensor which changed. + * @property {number} sensorValue - the new value of the sensor. + * @private + */ + _onSensorChanged (event) { + this._sensors[event.sensorName] = event.sensorValue; + } + + /** + * React to device disconnection. May be called more than once. + * @private + */ + _onDisconnect () { + this._disconnectEvents(); + } + + /** + * Send a message to the device socket. + * @param {string} message - the name of the message, such as 'playTone'. + * @param {object} [details] - optional additional details for the message, such as tone duration and pitch. + * @private + */ + _send (message, details) { + this._socket.emit(message, details); + } +} + +/** + * Enum for motor specification. + * @readonly + * @enum {string} + */ +const MotorID = { + DEFAULT: 'motor', + A: 'motor A', + B: 'motor B', + ALL: 'all motors' +}; + +/** + * Enum for motor direction specification. + * @readonly + * @enum {string} + */ +const MotorDirection = { + FORWARD: 'this way', + BACKWARD: 'that way', + REVERSE: 'reverse' +}; + +/** + * Enum for tilt sensor direction. + * @readonly + * @enum {string} + */ +const TiltDirection = { + UP: 'up', + DOWN: 'down', + LEFT: 'left', + RIGHT: 'right', + ANY: 'any' +}; + +/** + * Scratch 3.0 blocks to interact with a LEGO WeDo 2.0 device. + */ +class Scratch3WeDo2Blocks { + + /** + * @return {string} - the name of this extension. + */ + static get EXTENSION_NAME () { + return 'wedo2'; + } + + /** + * @return {number} - the tilt sensor counts as "tilted" if its tilt angle meets or exceeds this threshold. + */ + static get TILT_THRESHOLD () { + return 15; + } + + /** + * Construct a set of WeDo 2.0 blocks. + * @param {Runtime} runtime - the Scratch 3.0 runtime. + */ + constructor (runtime) { + /** + * The Scratch 3.0 runtime. + * @type {Runtime} + */ + this.runtime = runtime; + + this.runtime.HACK_WeDo2Blocks = this; + } + + /** + * Use the Device Manager client to attempt to connect to a WeDo 2.0 device. + */ + connect () { + if (this._device || this._finder) { + return; + } + const deviceManager = this.runtime.ioDevices.deviceManager; + const finder = this._finder = + deviceManager.searchAndConnect(Scratch3WeDo2Blocks.EXTENSION_NAME, WeDo2.DEVICE_TYPE); + this._finder.promise.then( + socket => { + if (this._finder === finder) { + this._finder = null; + this._device = new WeDo2(socket); + } else { + log.warn('Ignoring success from stale WeDo 2.0 connection attempt'); + } + }, + reason => { + if (this._finder === finder) { + this._finder = null; + log.warn(`WeDo 2.0 connection failed: ${reason}`); + } else { + log.warn('Ignoring failure from stale WeDo 2.0 connection attempt'); + } + }); + } + + /** + * Retrieve the block primitives implemented by this package. + * @return {object.<string, Function>} Mapping of opcode to Function. + */ + getPrimitives () { + return { + wedo2_motorOnFor: this.motorOnFor, + wedo2_motorOn: this.motorOn, + wedo2_motorOff: this.motorOff, + wedo2_startMotorPower: this.startMotorPower, + wedo2_setMotorDirection: this.setMotorDirection, + wedo2_setLightHue: this.setLightHue, + wedo2_playNoteFor: this.playNoteFor, + wedo2_whenDistance: this.whenDistance, + wedo2_whenTilted: this.whenTilted, + wedo2_getDistance: this.getDistance, + wedo2_isTilted: this.isTilted, + wedo2_getTiltAngle: this.getTiltAngle + }; + } + + /** + * Turn specified motor(s) on for a specified duration. + * @param {object} args - the block's arguments. + * @property {MotorID} MOTOR_ID - the motor(s) to activate. + * @property {int} DURATION - the amount of time to run the motors. + * @return {Promise} - a promise which will resolve at the end of the duration. + */ + motorOnFor (args) { + const durationMS = args.DURATION * 1000; + return new Promise(resolve => { + this._forEachMotor(args.MOTOR_ID, motorIndex => { + this._device.motor(motorIndex).setMotorOnFor(durationMS); + }); + + // Ensure this block runs for a fixed amount of time even when no device is connected. + setTimeout(resolve, durationMS); + }); + } + + /** + * Turn specified motor(s) on indefinitely. + * @param {object} args - the block's arguments. + * @property {MotorID} MOTOR_ID - the motor(s) to activate. + */ + motorOn (args) { + this._forEachMotor(args.MOTOR_ID, motorIndex => { + this._device.motor(motorIndex).setMotorOn(); + }); + } + + /** + * Turn specified motor(s) off. + * @param {object} args - the block's arguments. + * @property {MotorID} MOTOR_ID - the motor(s) to deactivate. + */ + motorOff (args) { + this._forEachMotor(args.MOTOR_ID, motorIndex => { + this._device.motor(motorIndex).setMotorOff(); + }); + } + + /** + * Turn specified motor(s) off. + * @param {object} args - the block's arguments. + * @property {MotorID} MOTOR_ID - the motor(s) to be affected. + * @property {int} POWER - the new power level for the motor(s). + */ + startMotorPower (args) { + this._forEachMotor(args.MOTOR_ID, motorIndex => { + const motor = this._device.motor(motorIndex); + motor.power = args.POWER; + motor.setMotorOn(); + }); + } + + /** + * Set the direction of rotation for specified motor(s). + * If the direction is 'reverse' the motor(s) will be reversed individually. + * @param {object} args - the block's arguments. + * @property {MotorID} MOTOR_ID - the motor(s) to be affected. + * @property {MotorDirection} DIRECTION - the new direction for the motor(s). + */ + setMotorDirection (args) { + this._forEachMotor(args.MOTOR_ID, motorIndex => { + const motor = this._device.motor(motorIndex); + switch (args.DIRECTION) { + case MotorDirection.FORWARD: + motor.direction = 1; + break; + case MotorDirection.BACKWARD: + motor.direction = -1; + break; + case MotorDirection.REVERSE: + motor.direction = -motor.direction; + break; + default: + log.warn(`Unknown motor direction in setMotorDirection: ${args.DIRECTION}`); + break; + } + }); + } + + /** + * Set the LED's hue. + * @param {object} args - the block's arguments. + * @property {number} HUE - the hue to set, in the range [0,100]. + */ + setLightHue (args) { + // Convert from [0,100] to [0,360] + const hue = args.HUE * 360 / 100; + + const rgbObject = color.hsvToRgb({h: hue, s: 1, v: 1}); + + const rgbDecimal = color.rgbToDecimal(rgbObject); + + this._device.setLED(rgbDecimal); + } + + /** + * Make the WeDo 2.0 hub play a MIDI note for the specified duration. + * @param {object} args - the block's arguments. + * @property {number} NOTE - the MIDI note to play. + * @property {number} DURATION - the duration of the note, in seconds. + * @return {Promise} - a promise which will resolve at the end of the duration. + */ + playNoteFor (args) { + return new Promise(resolve => { + const durationMS = args.DURATION * 1000; + const tone = this._noteToTone(args.NOTE); + this._device.playTone(tone, durationMS); + + // Ensure this block runs for a fixed amount of time even when no device is connected. + setTimeout(resolve, durationMS); + }); + } + + /** + * Compare the distance sensor's value to a reference. + * @param {object} args - the block's arguments. + * @property {string} OP - the comparison operation: '<' or '>'. + * @property {number} REFERENCE - the value to compare against. + * @return {boolean} - the result of the comparison, or false on error. + */ + whenDistance (args) { + switch (args.OP) { + case '<': + return this._device.distance < args.REFERENCE; + case '>': + return this._device.distance > args.REFERENCE; + default: + log.warn(`Unknown comparison operator in whenDistance: ${args.OP}`); + return false; + } + } + + /** + * Test whether the tilt sensor is currently tilted. + * @param {object} args - the block's arguments. + * @property {TiltDirection} DIRECTION - the tilt direction to test (up, down, left, right, or any). + * @return {boolean} - true if the tilt sensor is tilted past a threshold in the specified direction. + */ + whenTilted (args) { + return this._isTilted(args.DIRECTION); + } + + /** + * @return {number} - the distance sensor's value, scaled to the [0,100] range. + */ + getDistance () { + return this._device.distance * 10; + } + + /** + * Test whether the tilt sensor is currently tilted. + * @param {object} args - the block's arguments. + * @property {TiltDirection} DIRECTION - the tilt direction to test (up, down, left, right, or any). + * @return {boolean} - true if the tilt sensor is tilted past a threshold in the specified direction. + */ + isTilted (args) { + return this._isTilted(args.DIRECTION); + } + + /** + * @param {object} args - the block's arguments. + * @property {TiltDirection} DIRECTION - the direction (up, down, left, right) to check. + * @return {number} - the tilt sensor's angle in the specified direction. + * Note that getTiltAngle(up) = -getTiltAngle(down) and getTiltAngle(left) = -getTiltAngle(right). + */ + getTiltAngle (args) { + return this._getTiltAngle(args.DIRECTION); + } + + /** + * Test whether the tilt sensor is currently tilted. + * @param {TiltDirection} direction - the tilt direction to test (up, down, left, right, or any). + * @return {boolean} - true if the tilt sensor is tilted past a threshold in the specified direction. + * @private + */ + _isTilted (direction) { + switch (direction) { + case TiltDirection.ANY: + return (Math.abs(this._device.tiltX) >= Scratch3WeDo2Blocks.TILT_THRESHOLD) || + (Math.abs(this._device.tiltY) >= Scratch3WeDo2Blocks.TILT_THRESHOLD); + default: + return this._getTiltAngle(direction) >= Scratch3WeDo2Blocks.TILT_THRESHOLD; + } + } + + /** + * @param {TiltDirection} direction - the direction (up, down, left, right) to check. + * @return {number} - the tilt sensor's angle in the specified direction. + * Note that getTiltAngle(up) = -getTiltAngle(down) and getTiltAngle(left) = -getTiltAngle(right). + * @private + */ + _getTiltAngle (direction) { + switch (direction) { + case TiltDirection.UP: + return -this._device.tiltY; + case TiltDirection.DOWN: + return this._device.tiltY; + case TiltDirection.LEFT: + return -this._device.tiltX; + case TiltDirection.RIGHT: + return this._device.tiltX; + default: + log.warn(`Unknown tilt direction in _getTiltAngle: ${direction}`); + } + } + + /** + * Call a callback for each motor indexed by the provided motor ID. + * @param {MotorID} motorID - the ID specifier. + * @param {Function} callback - the function to call with the numeric motor index for each motor. + * @private + */ + _forEachMotor (motorID, callback) { + let motors; + switch (motorID) { + case MotorID.A: + motors = [0]; + break; + case MotorID.B: + motors = [1]; + break; + case MotorID.ALL: + case MotorID.DEFAULT: + motors = [0, 1]; + break; + default: + log.warn(`Invalid motor ID: ${motorID}`); + motors = []; + break; + } + for (const index of motors) { + callback(index); + } + } + + /** + * @param {number} midiNote - the MIDI note value to convert. + * @return {number} - the frequency, in Hz, corresponding to that MIDI note value. + * @private + */ + _noteToTone (midiNote) { + // Note that MIDI note 69 is A4, 440 Hz + return 440 * Math.pow(2, (midiNote - 69) / 12); + } +} + +module.exports = Scratch3WeDo2Blocks; diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 7756ceb14..80a404002 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -20,7 +20,8 @@ const defaultBlockPackages = { scratch3_sound: require('../blocks/scratch3_sound'), scratch3_sensing: require('../blocks/scratch3_sensing'), scratch3_data: require('../blocks/scratch3_data'), - scratch3_procedures: require('../blocks/scratch3_procedures') + scratch3_procedures: require('../blocks/scratch3_procedures'), + scratch3_wedo2: require('../blocks/scratch3_wedo2') }; /** diff --git a/src/import/load-costume.js b/src/import/load-costume.js index a9323398d..54962ea1f 100644 --- a/src/import/load-costume.js +++ b/src/import/load-costume.js @@ -1,3 +1,4 @@ +const StringUtil = require('../util/string-util'); const log = require('../util/log'); /** @@ -19,17 +20,17 @@ const loadCostume = function (md5ext, costume, runtime) { } const AssetType = runtime.storage.AssetType; - const idParts = md5ext.split('.'); + const idParts = StringUtil.splitFirst(md5ext, '.'); const md5 = idParts[0]; - const ext = idParts[1].toUpperCase(); - const assetType = (ext === 'SVG') ? AssetType.ImageVector : AssetType.ImageBitmap; + const ext = idParts[1].toLowerCase(); + const assetType = (ext === 'svg') ? AssetType.ImageVector : AssetType.ImageBitmap; const rotationCenter = [ costume.rotationCenterX / costume.bitmapResolution, costume.rotationCenterY / costume.bitmapResolution ]; - let promise = runtime.storage.load(assetType, md5).then(costumeAsset => { + let promise = runtime.storage.load(assetType, md5, ext).then(costumeAsset => { costume.assetId = costumeAsset.assetId; costume.assetType = assetType; return costumeAsset; diff --git a/src/import/load-sound.js b/src/import/load-sound.js index a308e1516..80b595987 100644 --- a/src/import/load-sound.js +++ b/src/import/load-sound.js @@ -1,3 +1,4 @@ +const StringUtil = require('../util/string-util'); const log = require('../util/log'); /** @@ -17,9 +18,10 @@ const loadSound = function (sound, runtime) { log.error('No audio engine present; cannot load sound asset: ', sound.md5); return Promise.resolve(sound); } - const idParts = sound.md5.split('.'); + const idParts = StringUtil.splitFirst(sound.md5, '.'); const md5 = idParts[0]; - return runtime.storage.load(runtime.storage.AssetType.Sound, md5) + const ext = idParts[1].toLowerCase(); + return runtime.storage.load(runtime.storage.AssetType.Sound, md5, ext) .then(soundAsset => { sound.assetId = soundAsset.assetId; sound.assetType = runtime.storage.AssetType.Sound; diff --git a/src/playground/playground.js b/src/playground/playground.js index c1b655a58..814bfb628 100644 --- a/src/playground/playground.js +++ b/src/playground/playground.js @@ -34,7 +34,7 @@ const getAssetUrl = function (asset) { 'internalapi/asset/', asset.assetId, '.', - asset.assetType.runtimeFormat, + asset.dataFormat, '/get/' ]; return assetUrlParts.join(''); diff --git a/src/serialization/sb3.js b/src/serialization/sb3.js index 161dbd2f1..816b1af19 100644 --- a/src/serialization/sb3.js +++ b/src/serialization/sb3.js @@ -75,7 +75,7 @@ const parseScratchObject = function (object, runtime) { rotationCenterX: costumeSource.rotationCenterX, rotationCenterY: costumeSource.rotationCenterY }; - const costumeMd5 = `${costumeSource.assetId}.${costumeSource.assetType.runtimeFormat}`; + const costumeMd5 = `${costumeSource.assetId}.${costumeSource.dataFormat}`; return loadCostume(costumeMd5, costume, runtime); }); // Sounds from JSON diff --git a/src/sprites/rendered-target.js b/src/sprites/rendered-target.js index 0bc5cff6e..2fb0ddf9e 100644 --- a/src/sprites/rendered-target.js +++ b/src/sprites/rendered-target.js @@ -832,6 +832,8 @@ class RenderedTarget extends Target { */ dispose () { this.runtime.changeCloneCounter(-1); + this.runtime.stopForTarget(this); + this.sprite.removeClone(this); if (this.renderer && this.drawableID !== null) { this.renderer.destroyDrawable(this.drawableID); if (this.visible) { diff --git a/src/sprites/sprite.js b/src/sprites/sprite.js index 3a0402a16..293a1f265 100644 --- a/src/sprites/sprite.js +++ b/src/sprites/sprite.js @@ -58,6 +58,18 @@ class Sprite { } return newClone; } + + /** + * Disconnect a clone from this sprite. The clone is unmodified. + * In particular, the clone's dispose() method is not called. + * @param {!RenderedTarget} clone - the clone to be removed. + */ + removeClone (clone) { + const cloneIndex = this.clones.indexOf(clone); + if (cloneIndex >= 0) { + this.clones.splice(cloneIndex, 1); + } + } } module.exports = Sprite; diff --git a/src/util/string-util.js b/src/util/string-util.js index 0ee712548..48dde0277 100644 --- a/src/util/string-util.js +++ b/src/util/string-util.js @@ -12,6 +12,30 @@ class StringUtil { while (existingNames.indexOf(name + i) >= 0) i++; return name + i; } + + /** + * Split a string on the first occurrence of a split character. + * @param {string} text - the string to split. + * @param {string} separator - split the text on this character. + * @returns {[string, string]} - the two parts of the split string, or [text, null] if no split character found. + * @example + * // returns ['foo', 'tar.gz'] + * splitFirst('foo.tar.gz', '.'); + * @example + * // returns ['foo', null] + * splitFirst('foo', '.'); + * @example + * // returns ['foo', ''] + * splitFirst('foo.', '.'); + */ + static splitFirst (text, separator) { + const index = text.indexOf(separator); + if (index >= 0) { + return [text.substring(0, index), text.substring(index + 1)]; + } else { + return [text, null]; + } + } } module.exports = StringUtil; diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 19a4a8dd4..12e6e7a77 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -364,6 +364,8 @@ class VirtualMachine extends EventEmitter { */ deleteSprite (targetId) { const target = this.runtime.getTargetById(targetId); + const targetIndexBeforeDelete = this.runtime.targets.map(t => t.id).indexOf(target.id); + if (target) { if (!target.isSprite()) { throw new Error('Cannot delete non-sprite targets.'); @@ -379,7 +381,8 @@ class VirtualMachine extends EventEmitter { this.runtime.disposeTarget(sprite.clones[i]); // Ensure editing target is switched if we are deleting it. if (clone === currentEditingTarget) { - this.setEditingTarget(this.runtime.targets[0].id); + const nextTargetIndex = Math.min(this.runtime.targets.length - 1, targetIndexBeforeDelete); + this.setEditingTarget(this.runtime.targets[nextTargetIndex].id); } } // Sprite object should be deleted by GC. diff --git a/test/fixtures/attach-test-storage.js b/test/fixtures/attach-test-storage.js index 3e75f5967..bb0cc3a08 100644 --- a/test/fixtures/attach-test-storage.js +++ b/test/fixtures/attach-test-storage.js @@ -26,7 +26,7 @@ const getAssetUrl = function (asset) { 'internalapi/asset/', asset.assetId, '.', - asset.assetType.runtimeFormat, + asset.dataFormat, '/get/' ]; return assetUrlParts.join(''); diff --git a/test/fixtures/clone-cleanup.sb2 b/test/fixtures/clone-cleanup.sb2 new file mode 100644 index 000000000..61baeb7f2 Binary files /dev/null and b/test/fixtures/clone-cleanup.sb2 differ diff --git a/test/integration/clone-cleanup.js b/test/integration/clone-cleanup.js new file mode 100644 index 000000000..bfecd6d77 --- /dev/null +++ b/test/integration/clone-cleanup.js @@ -0,0 +1,96 @@ +const path = require('path'); +const test = require('tap').test; +const attachTestStorage = require('../fixtures/attach-test-storage'); +const extract = require('../fixtures/extract'); +const VirtualMachine = require('../../src/index'); + +const projectUri = path.resolve(__dirname, '../fixtures/clone-cleanup.sb2'); +const project = extract(projectUri); + +test('clone-cleanup', t => { + const vm = new VirtualMachine(); + attachTestStorage(vm); + + /** + * Track which step of the project is currently under test. + * @type {number} + */ + let testStep = -1; + + /** + * We test using setInterval; track the interval ID here so we can cancel it. + * @type {object} + */ + let testInterval = null; + + const verifyCounts = (expectedClones, extraThreads) => { + // stage plus one sprite, plus clones + t.strictEqual(vm.runtime.targets.length, 2 + expectedClones, + `target count at step ${testStep}`); + + // the stage should never have any clones + t.strictEqual(vm.runtime.targets[0].sprite.clones.length, 1, + `stage clone count at step ${testStep}`); + + // check sprite clone count (+1 for original) + t.strictEqual(vm.runtime.targets[1].sprite.clones.length, 1 + expectedClones, + `sprite clone count at step ${testStep}`); + + // thread count isn't directly tied to clone count since threads can end + t.strictEqual(vm.runtime.threads.length, extraThreads + (2 * expectedClones), + `thread count at step ${testStep}`); + }; + + const testNextStep = () => { + ++testStep; + switch (testStep) { + case 0: + // Project has started, main thread running, no clones yet + verifyCounts(0, 1); + break; + + case 1: + // 10 clones have been created, main thread still running + verifyCounts(10, 1); + break; + + case 2: + // The first batch of clones has deleted themselves; main thread still running + verifyCounts(0, 1); + break; + + case 3: + // The second batch of clones has been created and the main thread has ended + verifyCounts(10, 0); + break; + + case 4: + // The second batch of clones has deleted themselves; everything is finished + verifyCounts(0, 0); + + clearInterval(testInterval); + t.end(); + process.nextTick(process.exit); + break; + } + }; + + // Start VM, load project, and run + t.doesNotThrow(() => { + vm.start(); + vm.clear(); + vm.setCompatibilityMode(false); + vm.setTurboMode(false); + vm.loadProject(project).then(() => { + + // Verify initial state: no clones, nothing running ("step -1") + verifyCounts(0, 0); + + vm.greenFlag(); + + // Every second, advance the testing step + testInterval = setInterval(testNextStep, 1000); + }); + }); + +}); diff --git a/test/unit/util_string.js b/test/unit/util_string.js index 741336258..aca20f9a4 100644 --- a/test/unit/util_string.js +++ b/test/unit/util_string.js @@ -1,6 +1,14 @@ const test = require('tap').test; const StringUtil = require('../../src/util/string-util'); +test('splitFirst', t => { + t.deepEqual(StringUtil.splitFirst('asdf.1234', '.'), ['asdf', '1234']); + t.deepEqual(StringUtil.splitFirst('asdf.', '.'), ['asdf', '']); + t.deepEqual(StringUtil.splitFirst('.1234', '.'), ['', '1234']); + t.deepEqual(StringUtil.splitFirst('foo', '.'), ['foo', null]); + t.end(); +}); + test('withoutTrailingDigits', t => { t.strictEqual(StringUtil.withoutTrailingDigits('boeing747'), 'boeing'); t.strictEqual(StringUtil.withoutTrailingDigits('boeing747 '), 'boeing747 ');