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

Added Polygon IHitShape #14840

Merged
merged 4 commits into from Mar 9, 2018

Conversation

Projects
None yet
4 participants
@abcdefg30
Copy link
Member

abcdefg30 commented Feb 22, 2018

#13484 revived.

@Unit158

This comment has been minimized.

Copy link
Contributor

Unit158 commented Feb 22, 2018

Looks sane, and it works. I'm not fond of some of the variable naming, but I couldn't think of anything better so 👍

Tried some edge cases, and it looks like there's some weirdness going on with hit detection. All I did was increase all of the polygon values on the medium tank by an order of magnitude. If this isn't a problem with the code here, then my +1 still stands :)

Yep. Looks like this is part of the impact code (scanning for the actor's center if it lands outside of the impact shape or something?), not the polygon code.

squares[i] = (Points[i] - Points[i - 1]).LengthSquared;
}

static int DistanceSquaredFromLineSegment(int2 c, int2 a, int2 b, int ab2)

This comment has been minimized.

@reaperrr

reaperrr Feb 23, 2018

Contributor

There must definitely be something wrong either in this...

return (ac - ap).LengthSquared;
}

public WDist DistanceFromEdge(WVec v)

This comment has been minimized.

@reaperrr

reaperrr Feb 23, 2018

Contributor

...or this.

Shots that land rather close to an edge, but whose AoE does not cover any of the polygons, deal zero damage.

@Unit158

This comment has been minimized.

Copy link
Contributor

Unit158 commented Mar 2, 2018

I took a look at this a while back. I think there's at least an absolute value missing somewhere.

// c is closest to its unknown orthogonal projection (p) onto the line spanned by b with a as the origin
var ab = b - a;
var ap2 = ac.X * ab.X + ac.Y * ab.Y;
var ap = new int2(ab.X * ap2 / ab2, ab.Y * ap2 / ab2);

This comment has been minimized.

@Unit158

Unit158 Mar 2, 2018

Contributor

ab.X * ap^2 and ab.Y * ap^2 will overflow for larger hitboxes.

This comment has been minimized.

@abcdefg30

abcdefg30 Mar 9, 2018

Author Member

Fixed, thanks.

@abcdefg30 abcdefg30 force-pushed the abcdefg30:polyShape branch from 0fd18d6 to d01ae63 Mar 9, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Mar 9, 2018

👍 after testcase has been removed.

@abcdefg30 abcdefg30 force-pushed the abcdefg30:polyShape branch from d01ae63 to f1ab284 Mar 9, 2018

@abcdefg30

This comment has been minimized.

Copy link
Member Author

abcdefg30 commented Mar 9, 2018

Removed it.

@reaperrr reaperrr merged commit b012fa6 into OpenRA:bleed Mar 9, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Mar 9, 2018

@abcdefg30 abcdefg30 deleted the abcdefg30:polyShape branch Mar 9, 2018

@reaperrr reaperrr referenced this pull request Jul 29, 2018

Closed

Add Polygon HitShape #11362

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.