Skip to content

Ignore sprite visibility when testing for touching color #213

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

Merged
merged 3 commits into from
Dec 11, 2017

Conversation

paulkaplan
Copy link
Contributor

@paulkaplan paulkaplan commented Dec 8, 2017

Resolves

What Github issue does this resolve (please include link)?

Fixes #212

Proposed Changes

Describe what this Pull Request does

Make the draw step of the touching color test ignore sprite visibility. Use the same flag as isStamping but rename for more general use case.

Also return false when there are no candidate sprites to test nearby, instead of returning undefined.

Reason for Changes

Explain why these changes should be made

To bring the touching color behavior in line with scratch 2.

Test Coverage

Please show how you have added tests to cover your changes

Tested the demo project I made in the linked issue. Now it matches scratch 2

image

image

@paulkaplan paulkaplan added this to the Dec 20 milestone Dec 8, 2017
@paulkaplan paulkaplan self-assigned this Dec 8, 2017
@paulkaplan paulkaplan requested a review from cwillisf December 8, 2017 17:30
@paulkaplan
Copy link
Contributor Author

@towerofnix thanks for the demo projects. I verified with the falling cat project and also added a commit to fix the undefined issue. That was a good catch, wouldn't have noticed because of the falsey-ness, but definitely should be false.

sensing-report--2
sensing-report--3

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.

Nice catch! Thanks @towerofnix and @paulkaplan!

There's one comment that could use an update, but other than that this looks great!

@@ -1115,7 +1119,7 @@ class RenderWebGL extends EventEmitter {
/** @todo check if drawable is inside the viewport before anything else */

// Hidden drawables (e.g., by a "hide" block) are not drawn unless stamping

This comment was marked as abuse.

@paulkaplan paulkaplan merged commit a0c3c3f into scratchfoundation:develop Dec 11, 2017
@paulkaplan paulkaplan deleted the touching-invisible branch December 11, 2017 14:16
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.

Touching color should work when sprites are not visible
2 participants