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

Fix Grid collisions with Hitbox #589

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Fix Grid collisions with Hitbox #589

wants to merge 2 commits into from

Conversation

SunShiranui
Copy link

I've noticed some unexpected behaviour when using Grid - hitboxes colliding with a wrong offset. This change seems to fix the issue, so I thought I'd send it over for review.

It looks like collideMask might have a similar mistake - but I haven't tested it.

I've noticed some unexpected behaviour when using Grid - hitboxes colliding with a wrong offset. This change seems to fix the issue, so I thought I'd send it over for review.

It looks like collideMask might have a similar mistake - but I haven't tested it.
@bendmorris
Copy link
Member

I don't doubt there's an issue here, but the original makes more sense to me than the patch: the original is (mask X - entity X) - (other mask X - other entity X) which gives you the difference in compensated positions; the new one flips it to (mask X - entity X) + other mask X + other entity X.

@SunShiranui
Copy link
Author

Isn't the original equivalent to:
(otherEntityX - otherMaskX) - (EntityX + MaskX)?

If we want the difference in positions, shouldn't we be doing this instead:
(otherEntityX + otherMaskX) - (EntityX + MaskX)

I apologize if there's something I'm missing.

@bendmorris
Copy link
Member

I think you're right about what we want, I need to look more closely at this logic. However:

_rect.x = other._parent.x - other._x - _parent.x + _x;

is the same as (just moving them around):

_rect.x = _x - _parent.x - other._x + other._parent.x;

which is the same as:

_rect.x = (_x - _parent.x) - (other._x - other._parent.x);

So it's consistent now, even if it's doing the wrong thing, and flipping a sign is probably not the right fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants