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

make sprite collisions register events with all tiles touched #920

Merged
merged 3 commits into from Aug 14, 2019

Conversation

jwunderl
Copy link
Member

@jwunderl jwunderl commented Aug 9, 2019

Currently, only the top most / left most wall that a sprite is touching has it's collision event trigger, which feels weird as the sprite is hitting multiple tiles but only counting collisions for one of them seemingly arbitrarily.

This defers the wall collision handling until after each tile the sprite would be touching has been checked, and then runs the collision events for each of them, instead of backing out after the first one.

fixes microsoft/pxt-arcade#1185 (except the sub issue of a request for non-bounding box based tile collisions, which I don't think will be feasible for now at least.)

2019-08-08 22 01 45 (example is slightly modified from the issue this fixes)

Screen Shot 2019-08-08 at 10 26 30 PM

worth a note this will cause a minor change in the block above's behavior (will return the rightmost collided tile instead of the leftmost), but that behavior wasn't defined / generally understood before anyways. I think that one might be good to change in the future, maybe just to be an option on the is hitting wall block that takes a parameter for the type of wall (e.g so you can check if it's hitting a lava tile); I don't think there's really anything valuable you can do with the block currently (without going to ts), and haven't seen it used (that I remember).

@jwunderl jwunderl requested a review from riknoll Aug 9, 2019
@jwunderl jwunderl requested a review from shakao Aug 12, 2019
@@ -732,10 +732,9 @@ class Sprite extends sprites.BaseSprite {
}

registerObstacle(direction: CollisionDirection, other: sprites.Obstacle) {
if (other == undefined) return;
Copy link
Member

@riknoll riknoll Aug 13, 2019

Choose a reason for hiding this comment

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

why this?

Copy link
Member Author

@jwunderl jwunderl Aug 13, 2019

Choose a reason for hiding this comment

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

Ahh I should have mentioned this one, TileMap.getObstacle can no longer return undefined after the physics interpolation changes, so this is now a redundant check. I just noticed that it while moving stuff around above.

Copy link
Member

@riknoll riknoll Aug 13, 2019

Choose a reason for hiding this comment

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

Cool! Just checking

@jwunderl jwunderl merged commit 5cd8843 into master Aug 14, 2019
3 checks passed
@jwunderl jwunderl deleted the hitAllTiles branch Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants