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

raycast & multiraycast #136

Closed
Emc2356 opened this issue Oct 28, 2022 · 5 comments
Closed

raycast & multiraycast #136

Emc2356 opened this issue Oct 28, 2022 · 5 comments

Comments

@Emc2356
Copy link
Collaborator

Emc2356 commented Oct 28, 2022

i think the API should look something like this:

@overload
def raycast(
    originpos: Coordinate,
    endpos: Coordinate,
    colliders: Sequence[Rect, Circle, Line],
) -> Optional[Tuple[float, float]]: ...
@overload
def raycast(
    originpos: Coordinate,
    angle: float,
    max_dist: float,
    colliders: Sequence[Rect, Circle, Line],
) -> Optional[Tuple[float, float]]: ...


@overload
def multiraycast(
    origin_positions: Sequence[Coordinate],
    end_positions: Sequence[Coordinate],
    colliders: Sequence[Rect, Circle, Line],
) -> Sequence[Optional[Tuple[float, float]]]: ...
@overload
def multiraycast(
    origin_positions: Sequence[Coordinate],
    angle: float,
    max_dist: float,
    colliders: Sequence[Rect, Circle, Line],
) -> Sequence[Optional[Tuple[float, float]]]: ...

it is easy for the user in my opinion

@novialriptide
Copy link
Member

I thought we settled for using directions?

@itzpr3d4t0r
Copy link
Member

itzpr3d4t0r commented Oct 31, 2022

I don't like this as it's sub-optimal to go trough 2 lists just to get the origin and end positions so this

@overload
def multiraycast(
    origin_positions: Sequence[Coordinate],
    end_positions: Sequence[Coordinate],
    colliders: Sequence[Rect, Circle, Line],
) -> Sequence[Optional[Tuple[float, float]]]: ...

Is a No-No for me.

I was thinking of something more like the pygame.Surface.blits() function, in that you pass it a list of containers that represent the surface, where you wanna blit it etc.

Basically i was thinking of a very compact approach that accepts a list of tuples that represent a ray, and that ray can be interpreted in various ways

@overload
def multiraycast(
    rays: Sequence[Ray],
    colliders: Sequence[Rect, Circle, Line],
) -> Sequence[Optional[Tuple[float, float]]]: ...

Where Ray can be either of the following:

  • Tuple (origin_pos, end_pos, max_dist)
  • Tuple (origin_pos, angle, max_dist)
  • Tuple (origin_pos, direction, max_dist)
  • Line

Would also be interesting to have an additional parameter that dictates whether None objects are appended to the return list if no intersection is found, so something like:

@overload
def multiraycast(
    rays: Sequence[Ray],
    colliders: Sequence[Rect, Circle, Line],
    include_miss: bool,
) -> Sequence[Optional[Tuple[float, float]]]: ...

And also, given that the majority of the times people will either use a batch of rays with infinite max_dist or all with the same max_dist, could be an efficiency fix to add another parameter that applies the same max_distance to each ray so that you don't have to specify a max_dist for each ray and we don't need to exract them:

@overload
def multiraycast(
    rays: Sequence[Ray],
    colliders: Sequence[Rect, Circle, Line],
    include_miss: bool,
    global_max_dist: float,
) -> Sequence[Optional[Tuple[float, float]]]: ...

And with that, given that it's possible that someone might want to set global_max_dist and not include_miss, i'd suggest we make this one a kwargs function.

@novialriptide
Copy link
Member

It seems that @itzpr3d4t0r has the right idea about optimizing this even further. I just want to be clear that we will not be creating a new Ray object.

I think adding None to the returned list is a good idea so that users can index the results better, I don't see how else they'll be able to do so.

@novialriptide
Copy link
Member

This also must not be worked on until #127 is fixed and #126 is merged.

@itzpr3d4t0r
Copy link
Member

itzpr3d4t0r commented Oct 31, 2022

I propose to work on this one after #126 is merged, which is pretty close anyway...

@Emc2356 Emc2356 closed this as completed Mar 12, 2023
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

No branches or pull requests

3 participants