Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mutator regressions #4448

Closed
NeilFraser opened this issue Nov 14, 2020 · 3 comments
Closed

Mutator regressions #4448

NeilFraser opened this issue Nov 14, 2020 · 3 comments
Labels
component: mutators issue: bug Describes why the code or behaviour is wrong type: regression

Comments

@NeilFraser
Copy link
Contributor

There are two separate regressions in mutators. Here's a video:
https://youtu.be/kg68406XE5c

First issue is that if there are several items in the stack, and n are dragged away, during the drag n-1 inputs are removed from the mutated block. Presumably this is the insertion marker leaking.

The second issue is that if a new item in the stack is added, and a new input appears on the mutated block, there isn't a bump for a non-connected block. This leads to a visual syntax error.

@NeilFraser NeilFraser added issue: bug Describes why the code or behaviour is wrong type: regression issue: triage Issues awaiting triage by a Blockly team member component: mutators labels Nov 14, 2020
@BeksOmega
Copy link
Collaborator

With regard to not bumping, this was done on purpose to avoid #4175 While not bumping in the case you've documented may be a bad behavior, I don't think it should considered a regression because bumping wasn't happening until #3468 introduced it. I do think not bumping is a problem, but I personally have no idea to fix it without digging up other issues.

@NeilFraser
Copy link
Contributor Author

Mutators used to bump:
https://youtu.be/h5Gnou6BIH8
You can play with it here:
https://2-dot-blockly-games.appspot.com/movie?level=10
That site is a permanent snapshot of Blockly Games from Jun 2016.

@BeksOmega
Copy link
Collaborator

Ok I stand corrected! So we need rendered to be false so that #4175 doesn't occur (as stated above). But having rendered be false causes this issue to occur, because in #3783 I removed the timeout bump neighbors call. My reasoning then was that bumps should be triggered by the individual block modification functions (eg appendDummyInput()) not by the mutator, because if you're modifying blocks without using the mutator UI you still want bumps to occur, and the bumpNeighbours call done by the mutator would only hide bugs. I still stand by the reasoning, but that reasoning also relies on rendered being true, which again causes #4175. I think there might be a way to get everything to work if rendered was expanded into more properties, but I don't want to touch that hehe.

Put up a reversion to the bad PR here.

Thank you for correcting me and providing the permanent snap shot :D That really helped me track down the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: mutators issue: bug Describes why the code or behaviour is wrong type: regression
Projects
None yet
Development

No branches or pull requests

4 participants