Shadow improvements (#135)

* Always add `next` to block representation

* Add `shadow` property to inputs, to maintain obscured shadows

* Create obscured shadows in SB2 import

* Add XML import of obscured shadows

* Alias SB2 shadow inputs to block inputs

* Add shadow to inputs on "delete inputs" test

* Add a small test to ensure obscured shadows are preserved

* Add more obscured shadow tests

src/engine/adapter.js

@@ -114,11 +114,16 @@ function domToBlock (blockDOM, blocks, isTopBlock) {
         case 'statement':
             // Recursively generate block structure for input block.
             domToBlock(childBlockNode, blocks, false);
+            if (childShadowNode && childBlockNode != childShadowNode) {
+                // Also generate the shadow block.
+                domToBlock(childShadowNode, blocks, false);
+            }
             // Link this block's input to the child block.
             var inputName = xmlChild.attribs.name;
             block.inputs[inputName] = {
                 name: inputName,
-                block: childBlockNode.attribs.id
+                block: childBlockNode.attribs.id,
+                shadow: childShadowNode ? childShadowNode.attribs.id : null
             };
             break;
         case 'next':

src/engine/blocks.js

@@ -166,6 +166,10 @@ Blocks.prototype.blocklyListen = function (e, isFlyout, opt_runtime) {
         });
         break;
     case 'delete':
+        // Don't accept delete events for shadow blocks being obscured.
+        if (this._blocks[e.blockId].shadow) {
+            return;
+        }
         this.deleteBlock({
             id: e.blockId
         });
@@ -181,9 +185,13 @@ Blocks.prototype.blocklyListen = function (e, isFlyout, opt_runtime) {
  * @param {boolean} opt_isFlyoutBlock Whether the block is in the flyout.
  */
 Blocks.prototype.createBlock = function (block, opt_isFlyoutBlock) {
-    // Create new block
+    // Does the block already exist?
+    // Could happen, e.g., for an unobscured shadow.
+    if (this._blocks.hasOwnProperty(block.id)) {
+        return;
+    }
+    // Create new block.
     this._blocks[block.id] = block;
-
     // Push block id to scripts array.
     // Blocks are added as a top-level stack if they are marked as a top-block
     // (if they were top-level XML in the event) and if they are not
@@ -239,11 +247,11 @@ Blocks.prototype.moveBlock = function (e) {
         this._deleteScript(e.id);
         // Otherwise, try to connect it in its new place.
         if (e.newInput !== undefined) {
-             // Moved to the new parent's input.
+            // Moved to the new parent's input.
-            this._blocks[e.newParent].inputs[e.newInput] = {
+            // Don't obscure the shadow block.
-                name: e.newInput,
+            var newInput = this._blocks[e.newParent].inputs[e.newInput];
-                block: e.id
+            newInput.name = e.newInput;
-            };
+            newInput.block = e.id;
         } else {
             // Moved to the new parent's next connection.
             this._blocks[e.newParent].next = e.id;
@@ -272,6 +280,11 @@ Blocks.prototype.deleteBlock = function (e) {
         if (block.inputs[input].block !== null) {
             this.deleteBlock({id: block.inputs[input].block});
         }
+        // Delete obscured shadow blocks.
+        if (block.inputs[input].shadow !== null &&
+            block.inputs[input].shadow !== block.inputs[input].block) {
+            this.deleteBlock({id: block.inputs[input].shadow});
+        }
     }
 
     // Delete any script starting with this block.
@@ -319,9 +332,16 @@ Blocks.prototype.blockToXML = function (blockId) {
     for (var input in block.inputs) {
         var blockInput = block.inputs[input];
         // Only encode a value tag if the value input is occupied.
-        if (blockInput.block) {
+        if (blockInput.block || blockInput.shadow) {
-            xmlString += '<value name="' + blockInput.name + '">' +
+            xmlString += '<value name="' + blockInput.name + '">';
-                this.blockToXML(blockInput.block) + '</value>';
+            if (blockInput.block) {
+                xmlString += this.blockToXML(blockInput.block);
+            }
+            if (blockInput.shadow && blockInput.shadow != blockInput.block) {
+                // Obscured shadow.
+                xmlString += this.blockToXML(blockInput.shadow);
+            }
+            xmlString += '</value>';
         }
     }
     // Add any fields on this block.

src/import/sb2import.js

@@ -183,6 +183,7 @@ function parseBlock (sb2block) {
         opcode: blockMetadata.opcode, // Converted, e.g. "motion_movesteps".
         inputs: {}, // Inputs to this block and the blocks they point to.
         fields: {}, // Fields on this block and their values.
+        next: null, // Next block.
         shadow: false, // No shadow blocks in an SB2 by default.
         children: [] // Store any generated children, flattened in `flatten`.
     };
@@ -192,13 +193,16 @@ function parseBlock (sb2block) {
     for (var i = 0; i < blockMetadata.argMap.length; i++) {
         var expectedArg = blockMetadata.argMap[i];
         var providedArg = sb2block[i + 1]; // (i = 0 is opcode)
+        // Whether the input is obscuring a shadow.
+        var shadowObscured = false;
         // Positional argument is an input.
         if (expectedArg.type == 'input') {
             // Create a new block and input metadata.
             var inputUid = uid();
             activeBlock.inputs[expectedArg.inputName] = {
                 name: expectedArg.inputName,
-                block: inputUid
+                block: null,
+                shadow: null
             };
             if (typeof providedArg == 'object') {
                 // Block or block list occupies the input.
@@ -210,37 +214,61 @@ function parseBlock (sb2block) {
                     // Single block occupies the input.
                     innerBlocks = [parseBlock(providedArg)];
                 }
-                activeBlock.inputs[expectedArg.inputName] = {
+                // Obscures any shadow.
-                    name: expectedArg.inputName,
+                shadowObscured = true;
-                    block: innerBlocks[0].id
+                activeBlock.inputs[expectedArg.inputName].block = (
-                };
+                    innerBlocks[0].id
+                );
                 activeBlock.children = (
                     activeBlock.children.concat(innerBlocks)
                 );
-            } else if (expectedArg.inputOp) {
+            }
-                // Unoccupied input. Generate a shadow block to occupy it.
+            // Generate a shadow block to occupy the input.
-                var fieldName = expectedArg.inputName;
+            // The shadow block is either visible or obscured.
-                if (expectedArg.inputOp == 'math_number') {
+            if (!expectedArg.inputOp) {
-                    fieldName = 'NUM';
+                // No editable shadow input; e.g., for a boolean.
-                } else if (expectedArg.inputOp == 'text') {
+                continue;
-                    fieldName = 'TEXT';
+            }
-                } else if (expectedArg.inputOp == 'colour_picker') {
+            // Each shadow has a field generated for it automatically.
-                    fieldName = 'COLOR';
+            // Value to be filled in the field.
+            var fieldValue = providedArg;
+            // Shadows' field names match the input name, except for these:
+            var fieldName = expectedArg.inputName;
+            if (expectedArg.inputOp == 'math_number') {
+                fieldName = 'NUM';
+                // Fields are given Scratch 2.0 default values if obscured.
+                if (shadowObscured) {
+                    fieldValue = 10;
                 }
-                var fields = {};
+            } else if (expectedArg.inputOp == 'text') {
-                fields[fieldName] = {
+                fieldName = 'TEXT';
-                    name: fieldName,
+                if (shadowObscured) {
-                    value: providedArg
+                    fieldValue = '';
-                };
+                }
-                activeBlock.children.push({
+            } else if (expectedArg.inputOp == 'colour_picker') {
-                    id: inputUid,
+                fieldName = 'COLOR';
-                    opcode: expectedArg.inputOp,
+                if (shadowObscured) {
-                    inputs: {},
+                    fieldValue = '#990000';
-                    fields: fields,
+                }
-                    next: null,
+            }
-                    topLevel: false,
+            var fields = {};
-                    shadow: true
+            fields[fieldName] = {
-                });
+                name: fieldName,
+                value: fieldValue
+            };
+            activeBlock.children.push({
+                id: inputUid,
+                opcode: expectedArg.inputOp,
+                inputs: {},
+                fields: fields,
+                next: null,
+                topLevel: false,
+                shadow: true
+            });
+            activeBlock.inputs[expectedArg.inputName].shadow = inputUid;
+            // If no block occupying the input, alias the block to the shadow.
+            if (!activeBlock.inputs[expectedArg.inputName].block) {
+                activeBlock.inputs[expectedArg.inputName].block = inputUid;
             }
         } else if (expectedArg.type == 'field') {
             // Add as a field on this block.

test/fixtures/events.json

@@ -59,5 +59,11 @@
         "xml": {
             "outerHTML":  "<block type='operator_equals' id='l^H_{8[DDyDW?m)HIt@b' x='100' y='362'><value name='OPERAND1'><shadow type='text' id='Ud@4y]bc./]uv~te?brb'><field name='TEXT'></field></shadow></value><value name='OPERAND2'><shadow type='text' id='p8[y..,[K;~G,k7]N;08'><field name='TEXT'></field></shadow></value></block>"
         }
+    },
+    "createobscuredshadow": {
+        "name": "block",
+        "xml": {
+            "outerHTML": "<block type='operator_add' id='D;MqidqmaN}Dft)y#Bf`' x='80' y='98'><value name='NUM1'><shadow type='math_number' id='F[IFAdLbq8!q25+Nio@i'><field name='NUM'></field></shadow><block type='sensing_answer' id='D~ZQ|BYb1)xw4)8ziI%.'></block</value><value name='NUM2'><shadow type='math_number' id='|Sjv4!*X6;wj?QaCE{-9'><field name='NUM'></field></shadow></value></block>"
+        }
     }
 }

test/unit/adapter.js

@@ -55,7 +55,9 @@ test('create with branch', function (t) {
     t.equal(result[0].topLevel, true);
     // In branch
     var branchBlockId = result[0].inputs['SUBSTACK']['block'];
+    var branchShadowId = result[0].inputs['SUBSTACK']['shadow'];
     t.type(branchBlockId, 'string');
+    t.equal(branchShadowId, null);
     // Find actual branch block
     var branchBlock = null;
     for (var i = 0; i < result.length; i++) {
@@ -83,6 +85,10 @@ test('create with two branches', function (t) {
     var secondBranchBlockId = result[0].inputs['SUBSTACK2']['block'];
     t.type(firstBranchBlockId, 'string');
     t.type(secondBranchBlockId, 'string');
+    var firstBranchShadowBlockId = result[0].inputs['SUBSTACK']['shadow'];
+    var secondBranchShadowBlockId = result[0].inputs['SUBSTACK2']['shadow'];
+    t.equal(firstBranchShadowBlockId, null);
+    t.equal(secondBranchShadowBlockId, null);
     // Find actual branch blocks
     var firstBranchBlock = null;
     var secondBranchBlock = null;
@@ -142,6 +148,13 @@ test('create with next connection', function (t) {
     t.end();
 });
 
+test('create with obscured shadow', function (t) {
+    var result = adapter(events.createobscuredshadow);
+    t.ok(Array.isArray(result));
+    t.equal(result.length, 4);
+    t.end();
+});
+
 test('create with invalid block xml', function (t) {
     // Entirely invalid block XML
     var result = adapter(events.createinvalid);

test/unit/blocks.js

@@ -130,7 +130,8 @@ test('getBranch', function (t) {
         inputs: {
             SUBSTACK: {
                 name: 'SUBSTACK',
-                block: 'foo2'
+                block: 'foo2',
+                shadow: null
             }
         },
         topLevel: true
@@ -164,11 +165,13 @@ test('getBranch2', function (t) {
         inputs: {
             SUBSTACK: {
                 name: 'SUBSTACK',
-                block: 'foo2'
+                block: 'foo2',
+                shadow: null
             },
             SUBSTACK2: {
                 name: 'SUBSTACK2',
-                block: 'foo3'
+                block: 'foo3',
+                shadow: null
             }
         },
         topLevel: true
@@ -416,11 +419,13 @@ test('delete inputs', function (t) {
         inputs: {
             input1: {
                 name: 'input1',
-                block: 'foo2'
+                block: 'foo2',
+                shadow: 'foo2'
             },
             SUBSTACK: {
                 name: 'SUBSTACK',
-                block: 'foo3'
+                block: 'foo3',
+                shadow: null
             }
         },
         topLevel: true
@@ -433,6 +438,14 @@ test('delete inputs', function (t) {
         inputs: {},
         topLevel: false
     });
+    b.createBlock({
+        id: 'foo5',
+        opcode: 'TEST_OBSCURED_SHADOW',
+        next: null,
+        fields: {},
+        inputs: {},
+        topLevel: false
+    });
     b.createBlock({
         id: 'foo3',
         opcode: 'TEST_BLOCK',
@@ -441,7 +454,8 @@ test('delete inputs', function (t) {
         inputs: {
             subinput: {
                 name: 'subinput',
-                block: 'foo4'
+                block: 'foo4',
+                shadow: 'foo5'
             }
         },
         topLevel: false
@@ -461,6 +475,7 @@ test('delete inputs', function (t) {
     t.type(b._blocks['foo2'], 'undefined');
     t.type(b._blocks['foo3'], 'undefined');
     t.type(b._blocks['foo4'], 'undefined');
+    t.type(b._blocks['foo5'], 'undefined');
     t.equal(b._scripts.indexOf('foo'), -1);
     t.equal(Object.keys(b._blocks).length, 0);
     t.equal(b._scripts.length, 0);