Blockly: real time code generation incorrect while dragging?

[gengshenghong]

Problem statement

It can reproduce with the online demo at: https://developers.google.com/blockly/
While dragging a block from toolbox, the real time generation of code is incorrect.

Expected Behavior

N/A

Actual Behavior

Steps to Reproduce

1. open 'https://developers.google.com/blockly/'
2. use the "Try Blockly"
3. switch "Language" to "Python" (also fail with other language in fact...).
4. drag "if do" block in "logic"
5. while dragging, the generated code is as below:

if False:
  pass

if False:
 pass

which is incorrect and confusing to user.

1. stop dragging and put the block in workspace, the generated code is as below:

if False:
  pass

which is correct.

Stack Traces

N/A

Operating System and Browser

Mac OSX, Chrome

Additional Information

N/A

[acbart]

I can confirm this is happening on my machine too (Windows 10, Chrome). While you are dragging, the result of getTopBlocks() has the extra dragged block.

[RoboErikG]

This is most likely caused by insertion markers and a missing a check somewhere to skip them.

[gengshenghong]

Yes, it is caused by insertion markers, and can be fixed as below:

Blockly.Generator.prototype.workspaceToCode = function(workspace) {
  .....
  var blocks = workspace.getTopBlocks(true);
  for (var x = 0, block; block = blocks[x]; x++) {
      if(block.isInsertionMarker_) { // FIX: add this to ignore insertion markers!!!
        continue;
      }

However, it only fixed the issue when dragging a new block from toolbox. It still has issues in other cases: when two block are connected, drag it out, it will also generate wrong code, which is not fixed by above code. Is it a different root cause? Any idea.

By the way, I've tested BlockPy @acbart (nice project), and it works well (split mode). It seems like you are using an old version of Blockly?

[acbart]

Thanks - and yes, we're upgrading to the latest version this summer. Hence all my Issues and Pull Requests :)

[gengshenghong]

I've fixed it with PR. There are two places required to check whether it is insertion marker to avoid incorrect code generation to cover the two cases I mentioned.

Thanks,
Shenghong

[rachel-fenichel]

See #2322 for more discussion of problems related to insertion markers. A better solution is to not generate code while dragging, which you can check with workspace.isDragging().

[NeilFraser]

Whether this is a bug in Blockly core, or just a bug in the online demo, it's still a valid and user-visible bug. Reopening.

(I just found it via weird behaviour observed in Blockly Games.)

[NeilFraser]

If the consensus is that workspaceToCode must not be called when a drag is in progress, then we should throw an error in this case.

Something else to consider: For some applications (such as the live demo mentioned above), it is trivial to handle moments where code is unavailable -- just add a check and don't do anything. For other real-time applications (such as Made-with-Code animations), it's quite a bit trickier to handle an interruption. Ideally workspaceToCode would figure out a way to return the code for the workspace's state immediately prior to the drag. This is what I've had to implement on the application level for Blockly Games Movie. But it's really a detail that Blockly's API should be handling internally.

Bottom line, the current behaviour is never the desired behaviour.

[BeksOmega]

it's quite a bit trickier to handle an interruption. Ideally workspaceToCode would figure out a way to return the code for the workspace's state immediately prior to the drag. This is what I've had to implement on the application level for Blockly Games Movie.

@NeilFraser would you be able to link the changes you made to get this working? I'd be interested in taking a look (just for funsies).

Also do you think that a similar fix could be applied to workspaceToDom (see https://github.com/google/blockly/issues/2322)? Or would a different strategy have to be employed there. I think it'd be great if devs didn't have to worry about these issues.

[NeilFraser]

@BeksOmega The Blockly Games Movie solution isn't general-purpose, it's relying on the fact that we generate code upon every non-UI change to the workspace (real-time code generation). So the latest PR adds a cache; if we are dragging, use the cache, if no drag generate code and save in the cache. That way valid code is always available.

Lines 384 to 389 here is the check, plus the rest of the PR to handle the cache consistently across all games:
https://github.com/google/blockly-games/commit/c4fa71e631d6fc51c5a4c58f82d23b6a9585475d#diff-0ad1ed0ee72ff2f4b4f08c6d7698bedc

[BeksOmega]

I was thinking about this some more, and I'm wondering if the code generation could be fixed by modifying generator.js. I know #2667 tried to this as well, but I think there's a better splot for it.

This proposal takes advantage of some rules of insertion markers:

1. They have at most 1 parent.
2. They have at most 1 child.

So if blockToCode ever runs into an insertion marker, it can just generate the code for the insertion marker's one child, instead of the insertion marker itself. It would work similarly to how disabled blocks are currently handled.

The code might look something like this:

  if (block.isInsertionMarker()) {
    // Skip past this block if it is an insertion marker.
    return opt_thisOnly ? '' : this.blockToCode(block.getChildren()[0]);
  }

@rachel-fenichel do you think a system like that could work? I haven't done a lot with insertion markers or code generation before.

[rachel-fenichel]

@BeksOmega good observation. I think that should work, though I will need to see tests to be certain.

Another option is to return the empty string from workspace to code if mid-drag, but I can't imagine that being good for existing uses. (Admittedly, neither is generating bad code, so maybe it's an acceptable solution if you can't get yours to work.)

[JoshuaLowe1002]

What's the best way to implement a quick fix for those using compressed blockly?

[rachel-fenichel]

@JoshuaLowe1002 Surround your calls to workspaceToCode with a check of workspace.isDragging().

[JoshuaLowe1002]

Thanks! Got it working now

On Thu, Apr 30, 2020 at 6:06 PM Rachel Fenichel [e-mail]
wrote:

@JoshuaLowe1002 https://github.com/JoshuaLowe1002 Surround your calls
to workspaceToCode with a check of workspace.isDragging().

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/google/blockly/issues/2646#issuecomment-621982915,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AGEPK3J5T7IOAL2SUSL3G2LRPGV2BANCNFSM4ID7XDHA
.

--
https://about.me/joshua.lowe?promo=email_sig&utm_source=product&utm_medium=email_sig&utm_campaign=edit_panel&utm_content=thumb
Joshua Lowe
Find out more about me: about.me/joshua.lowe
https://about.me/joshua.lowe?promo=email_sig&utm_source=product&utm_medium=email_sig&utm_campaign=edit_panel&utm_content=thumb