From 1a0bf07ea6bb09bc5c1946f3a8c680b30077f200 Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Thu, 18 Oct 2018 12:25:42 +0100 Subject: [PATCH 1/6] Stop using builtinHelper.cache And start just creating assets to hand to the VM. Towards https://github.com/LLK/scratch-vm/issues/1577 --- src/containers/record-modal.jsx | 12 +++++++----- src/lib/file-uploader.js | 11 +++++++---- src/lib/storage.js | 2 +- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/containers/record-modal.jsx b/src/containers/record-modal.jsx index 8a57c03d7..ee0007ab4 100644 --- a/src/containers/record-modal.jsx +++ b/src/containers/record-modal.jsx @@ -80,17 +80,19 @@ class RecordModal extends React.Component { sampleCount: clippedSamples.length }; - // Load the encoded .wav into the storage cache and get resulting - // md5 from storage - const storage = this.props.vm.runtime.storage; - const md5 = storage.builtinHelper.cache( + // Create an asset from the encoded .wav and get resulting md5 + const storage = this.props.vm.storage; + vmSound.asset = storage.createAsset( storage.AssetType.Sound, storage.DataFormat.WAV, new Uint8Array(wavBuffer), + null, + true // generate md5 ); + vmSound.assetId = vmSound.asset.assetId; // update vmSound object with md5 property - vmSound.md5 = `${md5}.${vmSound.dataFormat}`; + vmSound.md5 = `${vmSound.assetId}.${vmSound.dataFormat}`; // The VM will update the sound name to a fresh name // if the following is already taken vmSound.name = 'recording1'; diff --git a/src/lib/file-uploader.js b/src/lib/file-uploader.js index afa4371dd..481685684 100644 --- a/src/lib/file-uploader.js +++ b/src/lib/file-uploader.js @@ -63,17 +63,20 @@ const handleFileUpload = function (fileInput, onload) { * which can be used to look up the data in storage */ const cacheAsset = function (storage, fileName, assetType, dataFormat, data) { - const md5 = storage.builtinHelper.cache( + const asset = storage.createAsset( assetType, dataFormat, - data + data, + null, + true // generate md5 ); return { name: fileName, dataFormat: dataFormat, - md5: `${md5}.${dataFormat}`, - assetId: md5 + asset: asset, + md5: `${asset.assetId}.${dataFormat}`, + assetId: asset.assetId }; }; diff --git a/src/lib/storage.js b/src/lib/storage.js index b0634d9c3..7dc057e39 100644 --- a/src/lib/storage.js +++ b/src/lib/storage.js @@ -55,7 +55,7 @@ class Storage extends ScratchStorage { } cacheDefaultProject () { const defaultProjectAssets = defaultProject(this.translator); - defaultProjectAssets.forEach(asset => this.cache( + defaultProjectAssets.forEach(asset => this.builtinHelper._store( this.AssetType[asset.assetType], this.DataFormat[asset.dataFormat], asset.data, From e331232b0076067f2dd1a8d8745af2c1738c1985 Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Thu, 18 Oct 2018 17:09:25 +0100 Subject: [PATCH 2/6] Retrieve assets from the VM rather than storage --- src/components/asset-panel/selector.jsx | 2 +- src/components/sprite-selector/sprite-list.jsx | 2 +- src/components/target-pane/target-pane.jsx | 4 ++-- src/containers/costume-tab.jsx | 2 +- src/containers/sprite-selector-item.jsx | 7 ++++--- src/containers/stage-selector.jsx | 6 +++--- src/containers/watermark.jsx | 17 +++++++++-------- src/lib/backpack/costume-payload.js | 2 +- src/lib/backpack/sound-payload.js | 2 +- src/lib/backpack/sprite-payload.js | 2 +- src/lib/get-costume-url.js | 16 ++++++++-------- 11 files changed, 32 insertions(+), 30 deletions(-) diff --git a/src/components/asset-panel/selector.jsx b/src/components/asset-panel/selector.jsx index cd015c756..3a8df1475 100644 --- a/src/components/asset-panel/selector.jsx +++ b/src/components/asset-panel/selector.jsx @@ -64,7 +64,7 @@ const Selector = props => { onRemoveSortable={onRemoveSortable} >
{stage.id && ({ name: costume.name, - assetId: costume.assetId, + asset: costume.asset, details: costume.size ? this.formatCostumeDetails(costume.size, costume.bitmapResolution) : null, dragPayload: costume })) : []; diff --git a/src/containers/sprite-selector-item.jsx b/src/containers/sprite-selector-item.jsx index b89b89350..7a1100d0b 100644 --- a/src/containers/sprite-selector-item.jsx +++ b/src/containers/sprite-selector-item.jsx @@ -5,6 +5,7 @@ import {connect} from 'react-redux'; import {setHoveredSprite} from '../reducers/hovered-target'; import {updateAssetDrag} from '../reducers/asset-drag'; +import storage from '../lib/storage'; import {getEventXY} from '../lib/touch-utils'; import VM from 'scratch-vm'; import getCostumeUrl from '../lib/get-costume-url'; @@ -44,9 +45,9 @@ class SpriteSelectorItem extends React.Component { } getCostumeData () { if (this.props.costumeURL) return this.props.costumeURL; - if (!this.props.assetId) return null; + if (!this.props.asset) return null; - return getCostumeUrl(this.props.assetId, this.props.vm); + return getCostumeUrl(this.props.asset); } handleMouseUp () { this.initialOffset = null; @@ -147,7 +148,7 @@ class SpriteSelectorItem extends React.Component { } SpriteSelectorItem.propTypes = { - assetId: PropTypes.string, + asset: PropTypes.instanceOf(storage.Asset), costumeURL: PropTypes.string, dispatchSetHoveredSprite: PropTypes.func.isRequired, dragPayload: PropTypes.shape({ diff --git a/src/containers/stage-selector.jsx b/src/containers/stage-selector.jsx index ad302d51d..e8f0fc9df 100644 --- a/src/containers/stage-selector.jsx +++ b/src/containers/stage-selector.jsx @@ -106,7 +106,7 @@ class StageSelector extends React.Component { } render () { const componentProps = omit(this.props, [ - 'assetId', 'dispatchSetHoveredSprite', 'id', 'intl', 'onActivateTab', 'onSelect']); + 'asset', 'dispatchSetHoveredSprite', 'id', 'intl', 'onActivateTab', 'onSelect']); return ( ({ - url: assetId && state.scratchGui.vm.runtime.storage.get(assetId).encodeDataURI(), +const mapStateToProps = (state, {asset, id}) => ({ + url: asset && asset.encodeDataURI(), vm: state.scratchGui.vm, receivedBlocks: state.scratchGui.hoveredTarget.receivedBlocks && state.scratchGui.hoveredTarget.sprite === id, diff --git a/src/containers/watermark.jsx b/src/containers/watermark.jsx index c14b70ad4..8cc9324b9 100644 --- a/src/containers/watermark.jsx +++ b/src/containers/watermark.jsx @@ -5,6 +5,7 @@ import React from 'react'; import {connect} from 'react-redux'; import VM from 'scratch-vm'; +import storage from '../lib/storage'; import getCostumeUrl from '../lib/get-costume-url'; import WatermarkComponent from '../components/watermark/watermark.jsx'; @@ -20,13 +21,13 @@ class Watermark extends React.Component { } getCostumeData () { - if (!this.props.assetId) return null; + if (!this.props.asset) return null; - return getCostumeUrl(this.props.assetId, this.props.vm); + return getCostumeUrl(this.props.asset); } render () { - const componentProps = omit(this.props, ['assetId', 'vm']); + const componentProps = omit(this.props, ['asset', 'vm']); return ( { const targets = state.scratchGui.targets; const currentTargetId = targets.editingTarget; - let assetId; + let asset; if (currentTargetId) { if (targets.stage.id === currentTargetId) { - assetId = targets.stage.costume.assetId; + asset = targets.stage.costume.asset; } else if (targets.sprites.hasOwnProperty(currentTargetId)) { const currentSprite = targets.sprites[currentTargetId]; - assetId = currentSprite.costume.assetId; + asset = currentSprite.costume.asset; } } return { vm: state.scratchGui.vm, - assetId: assetId + asset: asset }; }; diff --git a/src/lib/backpack/costume-payload.js b/src/lib/backpack/costume-payload.js index 9786ec8d3..c35d4fd62 100644 --- a/src/lib/backpack/costume-payload.js +++ b/src/lib/backpack/costume-payload.js @@ -3,7 +3,7 @@ import storage from '../storage'; const costumePayload = costume => { // TODO is it ok to base64 encode SVGs? What about unicode text inside them? - const assetDataUrl = storage.get(costume.assetId).encodeDataURI(); + const assetDataUrl = costume.asset.encodeDataURI(); const assetDataFormat = costume.dataFormat; const payload = { type: 'costume', diff --git a/src/lib/backpack/sound-payload.js b/src/lib/backpack/sound-payload.js index 7ae573648..924b45196 100644 --- a/src/lib/backpack/sound-payload.js +++ b/src/lib/backpack/sound-payload.js @@ -3,7 +3,7 @@ import soundThumbnail from '!base64-loader!./sound-thumbnail.jpg'; import storage from '../storage'; const soundPayload = sound => { - const assetDataUrl = storage.get(sound.assetId).encodeDataURI(); + const assetDataUrl = sound.asset.encodeDataURI(); const assetDataFormat = sound.dataFormat; const payload = { type: 'sound', diff --git a/src/lib/backpack/sprite-payload.js b/src/lib/backpack/sprite-payload.js index c9dea7f22..941be5633 100644 --- a/src/lib/backpack/sprite-payload.js +++ b/src/lib/backpack/sprite-payload.js @@ -14,7 +14,7 @@ const spritePayload = (sprite, vm) => vm.exportSprite( thumbnail: '' }; - const costumeDataUrl = storage.get(sprite.costume.assetId).encodeDataURI(); + const costumeDataUrl = sprite.costume.asset.encodeDataURI(); return jpegThumbnail(costumeDataUrl).then(thumbnail => { payload.thumbnail = thumbnail.replace('data:image/jpeg;base64,', ''); return payload; diff --git a/src/lib/get-costume-url.js b/src/lib/get-costume-url.js index fb7e7c202..33e5c11fa 100644 --- a/src/lib/get-costume-url.js +++ b/src/lib/get-costume-url.js @@ -1,3 +1,4 @@ +import storage from './storage'; import {SVGRenderer} from 'scratch-svg-renderer'; // Contains 'font-family', but doesn't only contain 'font-family="none"' @@ -7,30 +8,29 @@ const getCostumeUrl = (function () { let cachedAssetId; let cachedUrl; - return function (assetId, vm) { + return function (asset) { - if (cachedAssetId === assetId) { + if (cachedAssetId === asset.assetId) { return cachedUrl; } - cachedAssetId = assetId; + cachedAssetId = asset.assetId; - const storage = vm.runtime.storage; - const asset = storage.get(assetId); // If the SVG refers to fonts, they must be inlined in order to display correctly in the img tag. // Avoid parsing the SVG when possible, since it's expensive. if (asset.assetType === storage.AssetType.ImageVector) { - const svgString = vm.runtime.storage.get(assetId).decodeText(); + const svgString = asset.decodeText(); if (svgString.match(HAS_FONT_REGEXP)) { const svgRenderer = new SVGRenderer(); svgRenderer.loadString(svgString); const svgText = svgRenderer.toString(true /* shouldInjectFonts */); cachedUrl = `data:image/svg+xml;utf8,${encodeURIComponent(svgText)}`; } else { - cachedUrl = vm.runtime.storage.get(assetId).encodeDataURI(); + cachedUrl = asset.encodeDataURI(); } + cachedUrl = asset.encodeDataURI(); } else { - cachedUrl = vm.runtime.storage.get(assetId).encodeDataURI(); + cachedUrl = asset.encodeDataURI(); } return cachedUrl; From 60ca790b6970bcc2b1dc91c88f9859a1d6bdd0ec Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Wed, 24 Oct 2018 12:28:31 +0100 Subject: [PATCH 3/6] Lint --- src/lib/backpack/costume-payload.js | 1 - src/lib/backpack/sound-payload.js | 1 - src/lib/backpack/sprite-payload.js | 1 - 3 files changed, 3 deletions(-) diff --git a/src/lib/backpack/costume-payload.js b/src/lib/backpack/costume-payload.js index c35d4fd62..7b5e699ca 100644 --- a/src/lib/backpack/costume-payload.js +++ b/src/lib/backpack/costume-payload.js @@ -1,5 +1,4 @@ import jpegThumbnail from './jpeg-thumbnail'; -import storage from '../storage'; const costumePayload = costume => { // TODO is it ok to base64 encode SVGs? What about unicode text inside them? diff --git a/src/lib/backpack/sound-payload.js b/src/lib/backpack/sound-payload.js index 924b45196..b1694fcff 100644 --- a/src/lib/backpack/sound-payload.js +++ b/src/lib/backpack/sound-payload.js @@ -1,6 +1,5 @@ // eslint-disable-next-line import/no-unresolved import soundThumbnail from '!base64-loader!./sound-thumbnail.jpg'; -import storage from '../storage'; const soundPayload = sound => { const assetDataUrl = sound.asset.encodeDataURI(); diff --git a/src/lib/backpack/sprite-payload.js b/src/lib/backpack/sprite-payload.js index 941be5633..12fc68c40 100644 --- a/src/lib/backpack/sprite-payload.js +++ b/src/lib/backpack/sprite-payload.js @@ -1,5 +1,4 @@ import jpegThumbnail from './jpeg-thumbnail'; -import storage from '../storage'; const spritePayload = (sprite, vm) => vm.exportSprite( sprite.id, From d52aba0b4ec95750129b5aeab88a6df36f18d052 Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Wed, 24 Oct 2018 13:05:09 +0100 Subject: [PATCH 4/6] Rename cacheAsset to better reflect what it does --- src/lib/file-uploader.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib/file-uploader.js b/src/lib/file-uploader.js index 481685684..f399bc214 100644 --- a/src/lib/file-uploader.js +++ b/src/lib/file-uploader.js @@ -51,7 +51,7 @@ const handleFileUpload = function (fileInput, onload) { */ /** - * Cache an asset (costume, sound) in storage and return an object representation + * Create an asset (costume, sound) with storage and return an object representation * of the asset to track in the VM. * @param {ScratchStorage} storage The storage to cache the asset in * @param {string} fileName The name of the asset @@ -62,7 +62,7 @@ const handleFileUpload = function (fileInput, onload) { * @return {VMAsset} An object representing this asset and relevant information * which can be used to look up the data in storage */ -const cacheAsset = function (storage, fileName, assetType, dataFormat, data) { +const createVMAsset = function (storage, fileName, assetType, dataFormat, data) { const asset = storage.createAsset( assetType, dataFormat, @@ -117,7 +117,7 @@ const costumeUpload = function (fileData, fileType, costumeName, storage, handle const bitmapAdapter = new BitmapAdapter(); const addCostumeFromBuffer = function (dataBuffer) { - const vmCostume = cacheAsset( + const vmCostume = createVMAsset( storage, costumeName, assetType, @@ -173,7 +173,7 @@ const soundUpload = function (fileData, fileType, soundName, storage, handleSoun return; } - const vmSound = cacheAsset( + const vmSound = createVMAsset( storage, soundName, storage.AssetType.Sound, From 5be07124a33107a64231cf60c241927a6e460028 Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Thu, 25 Oct 2018 10:07:14 +0100 Subject: [PATCH 5/6] Fix merge error Good catch @kchadha! --- src/lib/get-costume-url.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/get-costume-url.js b/src/lib/get-costume-url.js index 33e5c11fa..2ecb196c2 100644 --- a/src/lib/get-costume-url.js +++ b/src/lib/get-costume-url.js @@ -28,7 +28,6 @@ const getCostumeUrl = (function () { } else { cachedUrl = asset.encodeDataURI(); } - cachedUrl = asset.encodeDataURI(); } else { cachedUrl = asset.encodeDataURI(); } From 67722bb94eb40151486f19313e8c7baaa65937c0 Mon Sep 17 00:00:00 2001 From: Ray Schamp Date: Thu, 25 Oct 2018 11:02:11 +0100 Subject: [PATCH 6/6] Update scratch-vm This version is needed to work with the other storage changes made to GUI --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index fa1e798a4..64c945fde 100644 --- a/package.json +++ b/package.json @@ -105,7 +105,7 @@ "scratch-render": "0.1.0-prerelease.20181024220305", "scratch-storage": "1.1.0", "scratch-svg-renderer": "0.2.0-prerelease.20181024192149", - "scratch-vm": "0.2.0-prerelease.20181023192708", + "scratch-vm": "0.2.0-prerelease.20181025092837", "selenium-webdriver": "3.6.0", "startaudiocontext": "1.2.1", "style-loader": "^0.23.0",