Merge pull request #1711 from kchadha/import-cloud-var-info

Import cloud var info and enforce cloud var limit

src/engine/runtime.js

@@ -71,6 +71,54 @@ const ArgumentTypeMap = (() => {
     return map;
 })();
 
+/**
+ * A pair of functions used to manage the cloud variable limit,
+ * to be used when adding (or attempting to add) or removing a cloud variable.
+ * @typedef {object} CloudDataManager
+ * @property {function} canAddCloudVariable A function to call to check that
+ * a cloud variable can be added.
+ * @property {function} addCloudVariable A function to call to track a new
+ * cloud variable on the runtime.
+ * @property {function} removeCloudVariable A function to call when
+ * removing an existing cloud variable.
+ * @property {function} hasCloudVariables A function to call to check that
+ * the runtime has any cloud variables.
+ */
+
+/**
+ * Creates and manages cloud variable limit in a project,
+ * and returns two functions to be used to add a new
+ * cloud variable (while checking that it can be added)
+ * and remove an existing cloud variable.
+ * These are to be called whenever attempting to create or delete
+ * a cloud variable.
+ * @return {CloudDataManager} The functions to be used when adding or removing a
+ * cloud variable.
+ */
+const cloudDataManager = () => {
+    const limit = 8;
+    let count = 0;
+
+    const canAddCloudVariable = () => count < limit;
+
+    const addCloudVariable = () => {
+        count++;
+    };
+
+    const removeCloudVariable = () => {
+        count--;
+    };
+
+    const hasCloudVariables = () => count > 0;
+
+    return {
+        canAddCloudVariable,
+        addCloudVariable,
+        removeCloudVariable,
+        hasCloudVariables
+    };
+};
+
 /**
  * Predefined "Converted block info" for a separator between blocks in a block category
  * @type {ConvertedBlockInfo}
@@ -283,6 +331,38 @@ class Runtime extends EventEmitter {
          * @type {Profiler}
          */
         this.profiler = null;
+
+        const newCloudDataManager = cloudDataManager();
+
+        /**
+         * Check wether the runtime has any cloud data.
+         * @type {function}
+         * @return {boolean} Whether or not the runtime currently has any
+         * cloud variables.
+         */
+        this.hasCloudData = newCloudDataManager.hasCloudVariables;
+
+        /**
+         * A function which checks whether a new cloud variable can be added
+         * to the runtime.
+         * @type {function}
+         * @return {boolean} Whether or not a new cloud variable can be added
+         * to the runtime.
+         */
+        this.canAddCloudVariable = newCloudDataManager.canAddCloudVariable;
+
+        /**
+         * A function that tracks a new cloud variable in the runtime,
+         * updating the cloud variable limit.
+         */
+        this.addCloudVariable = newCloudDataManager.addCloudVariable;
+
+        /**
+         * A function which updates the runtime's cloud variable limit
+         * when removing a cloud variable.
+         * @type {function}
+         */
+        this.removeCloudVariable = newCloudDataManager.removeCloudVariable;
     }
 
     /**
@@ -1394,6 +1474,14 @@ class Runtime extends EventEmitter {
         this._monitorState = OrderedMap({});
         // @todo clear out extensions? turboMode? etc.
         this.ioDevices.cloud.clear();
+
+        // Reset runtime cloud data info
+        const newCloudDataManager = cloudDataManager();
+        this.hasCloudData = newCloudDataManager.hasCloudVariables;
+        this.canAddCloudVariable = newCloudDataManager.canAddCloudVariable;
+        this.addCloudVariable = newCloudDataManager.addCloudVariable;
+        this.removeCloudVariable = newCloudDataManager.removeCloudVariable;
+
     }
 
     /**

src/serialization/sb2.js

@@ -470,12 +470,18 @@ const parseScratchObject = function (object, runtime, extensions, topLevel, zip)
     if (object.hasOwnProperty('variables')) {
         for (let j = 0; j < object.variables.length; j++) {
             const variable = object.variables[j];
+            // A variable is a cloud variable if:
+            // - the project says it's a cloud variable, and
+            // - it's a stage variable, and
+            // - the runtime can support another cloud variable
+            const isCloud = variable.isPersistent && topLevel && runtime.canAddCloudVariable();
             const newVariable = new Variable(
                 getVariableId(variable.name, Variable.SCALAR_TYPE),
                 variable.name,
                 Variable.SCALAR_TYPE,
-                variable.isPersistent
+                isCloud
             );
+            if (isCloud) runtime.addCloudVariable();
             newVariable.value = variable.value;
             target.variables[newVariable.id] = newVariable;
         }

src/serialization/sb3.js

@@ -393,7 +393,7 @@ const serializeVariables = function (variables) {
         // otherwise should be a scalar type
         obj.variables[varId] = [v.name, v.value];
         // only scalar vars have the potential to be cloud vars
-        if (v.isPersistent) obj.variables[varId].push(true);
+        if (v.isCloud) obj.variables[varId].push(true);
     }
     return obj;
 };
@@ -900,12 +900,19 @@ const parseScratchObject = function (object, runtime, extensions, zip) {
     if (object.hasOwnProperty('variables')) {
         for (const varId in object.variables) {
             const variable = object.variables[varId];
+            // A variable is a cloud variable if:
+            // - the project says it's a cloud variable, and
+            // - it's a stage variable, and
+            // - the runtime can support another cloud variable
+            const isCloud = (variable.length === 3) && variable[2] &&
+                object.isStage && runtime.canAddCloudVariable();
             const newVariable = new Variable(
                 varId, // var id is the index of the variable desc array in the variables obj
                 variable[0], // name of the variable
                 Variable.SCALAR_TYPE, // type of the variable
-                (variable.length === 3) ? variable[2] : false // isPersistent/isCloud
+                isCloud
             );
+            if (isCloud) runtime.addCloudVariable();
             newVariable.value = variable[1];
             target.variables[newVariable.id] = newVariable;
         }

test/integration/cloud_variables_sb2.js

@@ -0,0 +1,152 @@
+const path = require('path');
+const test = require('tap').test;
+const makeTestStorage = require('../fixtures/make-test-storage');
+const readFileToBuffer = require('../fixtures/readProjectFile').readFileToBuffer;
+const VirtualMachine = require('../../src/index');
+
+const cloudVarSimpleUri = path.resolve(__dirname, '../fixtures/cloud_variables_simple.sb2');
+const cloudVarLimitUri = path.resolve(__dirname, '../fixtures/cloud_variables_limit.sb2');
+const cloudVarExceededLimitUri = path.resolve(__dirname, '../fixtures/cloud_variables_exceeded_limit.sb2');
+const cloudVarLocalUri = path.resolve(__dirname, '../fixtures/cloud_variables_local.sb2');
+
+const cloudVarSimple = readFileToBuffer(cloudVarSimpleUri);
+const cloudVarLimit = readFileToBuffer(cloudVarLimitUri);
+const cloudVarExceededLimit = readFileToBuffer(cloudVarExceededLimitUri);
+const cloudVarLocal = readFileToBuffer(cloudVarLocalUri);
+
+test('importing an sb2 project with cloud variables', t => {
+    const vm = new VirtualMachine();
+    vm.attachStorage(makeTestStorage());
+
+    // Start VM, load project, and run
+    vm.start();
+    vm.clear();
+    vm.setCompatibilityMode(false);
+    vm.setTurboMode(false);
+    vm.loadProject(cloudVarSimple).then(() => {
+        t.equal(vm.runtime.hasCloudData(), true);
+
+        const stage = vm.runtime.targets[0];
+        const stageVars = Object.values(stage.variables);
+        t.equal(stageVars.length, 1);
+
+        const variable = stageVars[0];
+        t.equal(variable.name, '☁ firstCloud');
+        t.equal(Number(variable.value), 100); // Though scratch 2 requires
+        // cloud variables to be numbers, this is something that happens
+        // when the message is being sent to the server rather than on the client
+        t.equal(variable.isCloud, true);
+
+        t.end();
+    });
+});
+
+test('importing an sb2 project with cloud variables at the limit for a project', t => {
+    const vm = new VirtualMachine();
+    vm.attachStorage(makeTestStorage());
+
+    // Start VM, load project, and run
+    vm.start();
+    vm.clear();
+    vm.setCompatibilityMode(false);
+    vm.setTurboMode(false);
+    vm.loadProject(cloudVarLimit).then(() => {
+        t.equal(vm.runtime.hasCloudData(), true);
+
+        const stage = vm.runtime.targets[0];
+        const stageVars = Object.values(stage.variables);
+
+        t.equal(stageVars.length, 8);
+        // All of the 8 stage variables should be cloud variables
+        t.equal(stageVars.filter(v => v.isCloud).length, 8);
+
+        t.end();
+    });
+});
+
+test('importing an sb2 project with cloud variables exceeding the limit for a project', t => {
+    // This tests a hacked project where additional cloud variables exceeding
+    // the project limit have been added.
+    const vm = new VirtualMachine();
+    vm.attachStorage(makeTestStorage());
+
+    // Start VM, load project, and run
+    vm.start();
+    vm.clear();
+    vm.setCompatibilityMode(false);
+    vm.setTurboMode(false);
+    vm.loadProject(cloudVarExceededLimit).then(() => {
+        t.equal(vm.runtime.hasCloudData(), true);
+
+        const stage = vm.runtime.targets[0];
+        const stageVars = Object.values(stage.variables);
+
+        t.equal(stageVars.length, 15);
+        // Only 8 of the variables should have the isCloud flag set to true
+        t.equal(stageVars.filter(v => v.isCloud).length, 8);
+
+        t.end();
+    });
+});
+
+test('importing one project after the other resets cloud variable limit', t => {
+    const vm = new VirtualMachine();
+    vm.attachStorage(makeTestStorage());
+
+    // Start VM, load project, and run
+    vm.start();
+    vm.clear();
+    vm.setCompatibilityMode(false);
+    vm.setTurboMode(false);
+    vm.loadProject(cloudVarExceededLimit).then(() => {
+        t.equal(vm.runtime.canAddCloudVariable(), false);
+
+        vm.loadProject(cloudVarSimple).then(() => {
+            const stage = vm.runtime.targets[0];
+            const stageVars = Object.values(stage.variables);
+            t.equal(stageVars.length, 1);
+
+            const variable = stageVars[0];
+            t.equal(variable.name, '☁ firstCloud');
+            t.equal(Number(variable.value), 100); // Though scratch 2 requires
+            // cloud variables to be numbers, this is something that happens
+            // when the message is being sent to the server rather than on the client
+            t.equal(variable.isCloud, true);
+
+            t.equal(vm.runtime.canAddCloudVariable(), true);
+
+            t.end();
+        });
+    });
+});
+
+test('local cloud variables get imported as regular variables', t => {
+    // This tests a hacked project where a sprite-local variable is
+    // has the cloud variable flag set.
+    const vm = new VirtualMachine();
+    vm.attachStorage(makeTestStorage());
+
+    // Start VM, load project, and run
+    vm.start();
+    vm.clear();
+    vm.setCompatibilityMode(false);
+    vm.setTurboMode(false);
+    vm.loadProject(cloudVarLocal).then(() => {
+        t.equal(vm.runtime.hasCloudData(), false);
+
+        const stage = vm.runtime.targets[0];
+        const stageVars = Object.values(stage.variables);
+
+        t.equal(stageVars.length, 0);
+
+        const sprite = vm.runtime.targets[1];
+        const spriteVars = Object.values(sprite.variables);
+
+        t.equal(spriteVars.length, 1);
+        t.equal(spriteVars[0].isCloud, false);
+
+        t.end();
+
+        process.nextTick(process.exit); // This is needed because this is the end of the last test in this file!!!
+    });
+});

test/integration/cloud_variables_sb3.js

@@ -0,0 +1,148 @@
+const path = require('path');
+const test = require('tap').test;
+const makeTestStorage = require('../fixtures/make-test-storage');
+const readFileToBuffer = require('../fixtures/readProjectFile').readFileToBuffer;
+const VirtualMachine = require('../../src/index');
+
+const cloudVarSimpleUri = path.resolve(__dirname, '../fixtures/cloud_variables_simple.sb3');
+const cloudVarLimitUri = path.resolve(__dirname, '../fixtures/cloud_variables_limit.sb3');
+const cloudVarExceededLimitUri = path.resolve(__dirname, '../fixtures/cloud_variables_exceeded_limit.sb3');
+const cloudVarLocalUri = path.resolve(__dirname, '../fixtures/cloud_variables_local.sb3');
+
+const cloudVarSimple = readFileToBuffer(cloudVarSimpleUri);
+const cloudVarLimit = readFileToBuffer(cloudVarLimitUri);
+const cloudVarExceededLimit = readFileToBuffer(cloudVarExceededLimitUri);
+const cloudVarLocal = readFileToBuffer(cloudVarLocalUri);
+
+test('importing an sb3 project with cloud variables', t => {
+    const vm = new VirtualMachine();
+    vm.attachStorage(makeTestStorage());
+
+    // Start VM, load project, and run
+    vm.start();
+    vm.clear();
+    vm.setCompatibilityMode(false);
+    vm.setTurboMode(false);
+    vm.loadProject(cloudVarSimple).then(() => {
+        t.equal(vm.runtime.hasCloudData(), true);
+
+        const stage = vm.runtime.targets[0];
+        const stageVars = Object.values(stage.variables);
+        t.equal(stageVars.length, 1);
+
+        const variable = stageVars[0];
+        t.equal(variable.name, '☁ firstCloud');
+        t.equal(Number(variable.value), 100);
+        t.equal(variable.isCloud, true);
+
+        t.end();
+    });
+});
+
+test('importing an sb3 project with cloud variables at the limit for a project', t => {
+    const vm = new VirtualMachine();
+    vm.attachStorage(makeTestStorage());
+
+    // Start VM, load project, and run
+    vm.start();
+    vm.clear();
+    vm.setCompatibilityMode(false);
+    vm.setTurboMode(false);
+    vm.loadProject(cloudVarLimit).then(() => {
+        t.equal(vm.runtime.hasCloudData(), true);
+
+        const stage = vm.runtime.targets[0];
+        const stageVars = Object.values(stage.variables);
+
+        t.equal(stageVars.length, 8);
+        // All of the 8 stage variables should be cloud variables
+        t.equal(stageVars.filter(v => v.isCloud).length, 8);
+
+        t.end();
+    });
+});
+
+test('importing an sb3 project with cloud variables exceeding the limit for a project', t => {
+    // This tests a hacked project where additional cloud variables exceeding
+    // the project limit have been added.
+    const vm = new VirtualMachine();
+    vm.attachStorage(makeTestStorage());
+
+    // Start VM, load project, and run
+    vm.start();
+    vm.clear();
+    vm.setCompatibilityMode(false);
+    vm.setTurboMode(false);
+    vm.loadProject(cloudVarExceededLimit).then(() => {
+        t.equal(vm.runtime.hasCloudData(), true);
+
+        const stage = vm.runtime.targets[0];
+        const stageVars = Object.values(stage.variables);
+
+        t.equal(stageVars.length, 15);
+        // Only 8 of the variables should have the isCloud flag set to true
+        t.equal(stageVars.filter(v => v.isCloud).length, 8);
+
+        t.end();
+    });
+});
+
+test('importing one project after the other resets cloud variable limit', t => {
+    const vm = new VirtualMachine();
+    vm.attachStorage(makeTestStorage());
+
+    // Start VM, load project, and run
+    vm.start();
+    vm.clear();
+    vm.setCompatibilityMode(false);
+    vm.setTurboMode(false);
+    vm.loadProject(cloudVarExceededLimit).then(() => {
+        t.equal(vm.runtime.canAddCloudVariable(), false);
+
+        vm.loadProject(cloudVarSimple).then(() => {
+            const stage = vm.runtime.targets[0];
+            const stageVars = Object.values(stage.variables);
+            t.equal(stageVars.length, 1);
+
+            const variable = stageVars[0];
+            t.equal(variable.name, '☁ firstCloud');
+            t.equal(Number(variable.value), 100);
+            t.equal(variable.isCloud, true);
+
+            t.equal(vm.runtime.canAddCloudVariable(), true);
+
+            t.end();
+        });
+    });
+});
+
+test('local cloud variables get imported as regular variables', t => {
+    // This tests a hacked project where a sprite-local variable is
+    // has the cloud variable flag set.
+    const vm = new VirtualMachine();
+    vm.attachStorage(makeTestStorage());
+
+    // Start VM, load project, and run
+    vm.start();
+    vm.clear();
+    vm.setCompatibilityMode(false);
+    vm.setTurboMode(false);
+    vm.loadProject(cloudVarLocal).then(() => {
+        t.equal(vm.runtime.hasCloudData(), false);
+
+        const stage = vm.runtime.targets[0];
+        const stageVars = Object.values(stage.variables);
+
+        t.equal(stageVars.length, 0);
+
+        const sprite = vm.runtime.targets[1];
+        const spriteVars = Object.values(sprite.variables);
+
+        t.equal(spriteVars.length, 1);
+        t.equal(spriteVars[0].isCloud, false);
+
+        t.end();
+
+        process.nextTick(process.exit); // This is needed because this is the end of the last test in this file!!!
+    });
+});

test/unit/engine_runtime.js

@@ -11,6 +11,13 @@ test('spec', t => {
 
     t.type(Runtime, 'function');
     t.type(r, 'object');
+
+    // Test types of cloud data managing functions
+    t.type(r.hasCloudData, 'function');
+    t.type(r.canAddCloudVariable, 'function');
+    t.type(r.addCloudVariable, 'function');
+    t.type(r.removeCloudVariable, 'function');
+
     t.ok(r instanceof Runtime);
 
     t.end();
@@ -127,3 +134,49 @@ test('Project loaded emits runtime event', t => {
         t.end();
     });
 });
+
+test('Cloud variable limit allows only 8 cloud variables', t => {
+    // This is a test of just the cloud variable limit mechanism
+    // The functions being tested below need to be used when
+    // creating and deleting cloud variables in the runtime.
+
+    const rt = new Runtime();
+
+    t.equal(rt.hasCloudData(), false);
+
+    for (let i = 0; i < 8; i++) {
+        t.equal(rt.canAddCloudVariable(), true);
+        rt.addCloudVariable();
+        // Adding a cloud variable should change the
+        // result of the hasCloudData check
+        t.equal(rt.hasCloudData(), true);
+    }
+
+
+    // We should be at the cloud variable limit now
+    t.equal(rt.canAddCloudVariable(), false);
+
+    // Removing a cloud variable should allow the addition of exactly one more
+    // when we are at the cloud variable limit
+    rt.removeCloudVariable();
+
+    t.equal(rt.canAddCloudVariable(), true);
+    rt.addCloudVariable();
+    t.equal(rt.canAddCloudVariable(), false);
+
+    // Disposing of the runtime should reset the cloud variable limitations
+    rt.dispose();
+    t.equal(rt.hasCloudData(), false);
+
+    for (let i = 0; i < 8; i++) {
+        t.equal(rt.canAddCloudVariable(), true);
+        rt.addCloudVariable();
+        t.equal(rt.hasCloudData(), true);
+    }
+
+    // We should be at the cloud variable limit now
+    t.equal(rt.canAddCloudVariable(), false);
+
+    t.end();
+
+});