scratchfoundation / scratch-gui

Graphical User Interface for creating and running Scratch 3.0 projects.
https://scratchfoundation.github.io/scratch-gui/develop/
BSD 3-Clause "New" or "Revised" License
4.41k stars 3.5k forks source link

Say blocks appear further a part so the conversation between sprites looks weird #1591

Closed techwonder21 closed 6 years ago

techwonder21 commented 6 years ago

Expected Behavior

Say blocks for each sprite should appear closer to one another https://scratch.mit.edu/projects/56987002/#editor

Actual Behavior

Say blocks appear further a part so the conversation between sprites looks weird say_blocks_far_apart

Steps to Reproduce

  1. Create a project between two sprites that talk in 3.0
  2. Compare that project to something similar in 2.0
  3. Notice that the say blocks in 3.0 are much further a part.
  4. Note: Importing an existing 2.0 project into 3.0 does not show the issue. Only using new sprites in 3.0 shows the issue.

Operating System and Browser

Chrome 64

techwonder21 commented 6 years ago

Using 3.0 sprites in 2.0 works correctly. https://scratch.mit.edu/projects/208667184/

techwonder21 commented 6 years ago

If the say block fix is extensive, is it possible to just fix it for dinosaur1 and duck sprite initially?

hedgehog125 commented 6 years ago

I've also noticed another bug (see the gif). scratch3 say bug I tested this in Scratch 2.0 and it doesn't occur.

thisandagain commented 6 years ago

@cwillisf @paulkaplan Any thoughts on the easiest path to tightening up the offset on these? This is causing some trouble for some of the resource developers.

/cc @carljbowman

BryceLTaylor commented 6 years ago

I think this is the same as issue #1568

paulkaplan commented 6 years ago

@BryceLTaylor yeah same issue.

Did some digging into how Scratch 2 works (src): it uses the bounding box of a thin strip at the top of the sprite, instead of the bounding box of the entire sprite.

I propose we introduce a new method on the drawable & renderer that retrieves this bounding box. We can do that by taking an AABB of only the convex hull points that are in this region.

Then the VM can use this new method (renderer.getBoundsForBubble(drawableID), e.g.) instead of using renderer.getBounds() when positioning the bubbles: https://github.com/LLK/scratch-vm/blob/develop/src/blocks/scratch3_looks.js#L122

@cwillisf seem like a good way to do it? I know filtering the convex hull points isn't a geometrically sound operation, it would be better to either redo the convex hull calculation in the strip or take an intersection with the overall convex hull with the strip, but because the strip is so small, I doubt it could really get into too much trouble...

thisandagain commented 6 years ago

Thanks for the investigation @paulkaplan. This approach seems reasonable. 👍