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

feat: Add Ray2 class to be used in raytracing/casting #1788

Merged
merged 21 commits into from
Jul 27, 2022
Merged

Conversation

spydon
Copy link
Member

@spydon spydon commented Jul 8, 2022

Description

Introduces the Ray2 class which later will be used to see whether a ray intersects a hitbox.

Checklist

  • The title of my PR starts with a Conventional Commit prefix (fix:, feat:, docs: etc).
  • I have read the Contributor Guide and followed the process outlined for submitting PRs.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples.

Breaking Change

  • No, this is not a breaking change.

Related Issues

@spydon spydon requested review from st-pasha and a team July 8, 2022 07:07
@spydon spydon mentioned this pull request Jul 8, 2022
9 tasks
packages/flame/lib/src/geometry/ray2.dart Show resolved Hide resolved
packages/flame/lib/src/geometry/ray2.dart Outdated Show resolved Hide resolved
packages/flame/lib/src/geometry/ray2.dart Outdated Show resolved Hide resolved
packages/flame/lib/src/geometry/ray2.dart Outdated Show resolved Hide resolved
packages/flame/lib/src/geometry/ray2.dart Show resolved Hide resolved
packages/flame/lib/src/geometry/ray2.dart Outdated Show resolved Hide resolved
packages/flame/lib/src/geometry/ray2.dart Outdated Show resolved Hide resolved
@st-pasha
Copy link
Contributor

st-pasha commented Jul 8, 2022

Nit: in the title capitalize Ray2

@spydon spydon changed the title feat: Add ray2 class to be used in raytracing/casting feat: Add Ray2 class to be used in raytracing/casting Jul 8, 2022
return t1;
}
return null;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This method should probably need to be tested too. In particular, I'm not sure whether the sign of t1 is correct.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added some simple tests for it, please have a look to see whether they are testing what you were wondering about.

@spydon spydon requested a review from st-pasha July 24, 2022 19:15
@spydon
Copy link
Member Author

spydon commented Jul 25, 2022

Hmm, seems like something is still off with intersectsWithAabb2 from the results I'm getting after rebasing #1785 on this PR.
Possibly the Y sign is flipped somewhere...

@spydon
Copy link
Member Author

spydon commented Jul 25, 2022

Hmm, seems like something is still off with intersectsWithAabb2 from the results I'm getting after rebasing #1785 on this PR. Possibly the Y sign is flipped somewhere...

Nvm me, it was just the direction inverse that wasn't properly updated when a Ray2 was modified.

packages/flame/lib/src/geometry/ray2.dart Outdated Show resolved Hide resolved
packages/flame/lib/src/geometry/ray2.dart Outdated Show resolved Hide resolved
/// [LineSegment] or null if there is no intersection.
///
/// A ray that is parallel and overlapping with the [segment] is considered to
/// not intersect.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we can handle this edge case better? For example, adding if (dot == 0) { ... }.

Or was this a deliberate choice?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would be nice, what should we return in that case though, would t1 still be a valid point on the line segment?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, obviously it can't return t1 since that would be having a division by zero.
I'll try around a bit, if you have a good idea of how it can be done I'm all ears.

Copy link
Member Author

@spydon spydon Jul 26, 2022

Choose a reason for hiding this comment

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

What do you think about the solution that I pushed now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, maybe it makes sense that it doesn't collide with parallel rays, because it will always collide with one of the line segments connecting to it anyways?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it makes sense, esp since the intersection in this case is not a single point but rather a segment, so returning a single point would be wrong.
But we should probably explain this in a comment, because I suspect we'll soon forget what the reason was.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, I added that in #1785

packages/flame/lib/src/extensions/double.dart Outdated Show resolved Hide resolved
final dot = _v2.dot(_v3);

if (dot == 0) {
// ray is parallel to line
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I'm on the phone rn, but I'd do smth like this here:

  1. Let dot2=v1.dot(v3). Then if dot2!=0 there's no intersection and we can return null.
  2. Otherwise, let t1=direction.dot(segment.from-ray.origin) and t2=direction.dot(segment.to-ray.origin). (These can be expressed in terms of v1 and v2). Then if both t1 and t2 positive, return the min of them; if both negative return null; otherwise, return 0 because the Ray's origin is inside the segment.

This ensures that we return a point closest to Ray's origin, and correctly handle the case when ray and segment are on parallel lines.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, maybe it makes sense that it doesn't collide with parallel rays, because it will always collide with one of the line segments connecting to it anyways? I'll remove it again for now and then we can re-add it again later if we choose to.

@spydon spydon merged commit 26196c0 into main Jul 27, 2022
@spydon spydon deleted the spydon/ray2 branch July 27, 2022 15:51
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.

5 participants