Skip to content

Conversation

ktbee
Copy link
Contributor

@ktbee ktbee commented Feb 5, 2019

Resolves

Fixes some of the factors in #403 and depends on #408

Proposed Changes

Add a method for identifying skin type and if the drawable is a bitmap, always use getAABB for its bounding box when calculating its fenced position at the edge of the stage.

Reason for Changes

In Scratch 2, we used getRect to get the aabb, whose implementation seems closer to drawable.getAABB than drawable.getBounds. drawable.getAABB seems closer because Flash's getRect doesn't take into account any strokes on the shape, and drawable.getBounds uses hull points, which would take stroke into account. If the skin is a bitmap, we make sure we always use drawable.getAABB instead of drawable.getFastBounds, which might use either drawable.getBounds or drawable.getAABB.

All this being said, this fix doesn't work for SVG skins, so we're sticking with our previous tactic for getting the aabb for any non-bitmap skins.

Test Coverage

I updated sprite-goes-off-stage.sb2 to include more bitmap sprite examples to test.

@thisandagain
Copy link
Contributor

/cc @kchadha

@thisandagain thisandagain added this to the February 2019 milestone Feb 7, 2019
Copy link
Contributor

@cwillisf cwillisf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome job tracking these issues down!
One quibble about implementation details, but 👍 on the general concept :)

@ktbee ktbee assigned ktbee and unassigned cwillisf Feb 11, 2019
@ktbee ktbee force-pushed the compat-bitmap-position-off-stage branch from dbe1057 to 4abf67e Compare February 11, 2019 16:52
@ktbee ktbee force-pushed the compat-bitmap-position-off-stage branch from 4abf67e to 1f0f899 Compare February 11, 2019 16:57
@ktbee
Copy link
Contributor Author

ktbee commented Feb 11, 2019

Thank you for the suggestion/feedback @cwillisf! I've implemented it in my latest round of changes, let me know what you think.

Copy link
Contributor

@cwillisf cwillisf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@cwillisf cwillisf removed their assignment Feb 13, 2019
@ktbee ktbee merged commit 4bf233e into scratchfoundation:develop Feb 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants