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

Allow Sprite "sprite-vs-group" collide methods to ignore the calling sprite (it shouldn't hit itself!) #3193

Open
celeritydesign opened this issue Oct 26, 2024 · 9 comments
Assignees

Comments

@celeritydesign
Copy link

celeritydesign commented Oct 26, 2024

I would suggest some of the Sprite "group" collide methods be extended to allow avoiding checking collisions on a sprite with itself, even if the sprite exists within the group being checked against.

My example use-case is that I have four player ships, and a bunch of asteroids, that get added to a group "all_shootable_objects" during init() When I randomly spawn at the start of a level, I use :

if pygame.sprite.spritecollideany(self, all_shootable_objects, pygame.sprite.collide_rect_ratio(2)):
...to test I'm not spawning on top of another object. Obviously this always reports a collision, as my current object is colliding with itself.

My workaround is to temporarily remove the current object from "all_shootable_objects" and then put it back in afterwards.
That seems a bit clunky though, and I think new users would be pretty confused to always get collisions in a similar scenario.

Maybe adding a new arg, "ignore_self" and default it to False to maintain current behaviour,
e.g. spritecollideany(sprite, group, collided = None, ignore_self = False)

Suggested functions that would benefit (not sure if I've missed others):

  • pygame.sprite.spritecollideany()
  • pygame.sprite.spritecollide()

To put it another way, is there any time you would ever want to register a collision against yourself when checking against a group? It seems counter-intuitive to me, so if there are reasons, maybe adding a note to the documentation for new users would be useful?
Eg: “note, if your sprite is in the same group you are checking against, it will report as colliding with itself! You will need to remove it prior to your check to avoid this.”

Am I missing something obvious, or is this worth considering?

@oddbookworm
Copy link
Member

This seems like a reasonable change to me

@oddbookworm
Copy link
Member

@celeritydesign said in discord that they're going to work on it

@bigwhoopgames
Copy link

I wouldn't mind this as I do it anyways. I usually have an exclusions set or list I have to check to remove false positives. I would love to have an optional iterable object I can pass that includes things to exclude. It would be more flexible.

The other thing I would love to have these functions support is the ability to check collision on sets of objects as well as lists.

@bilhox
Copy link
Contributor

bilhox commented Oct 30, 2024

While this feature is fairly reasonable, are we sure it'll not break any code ?

@celeritydesign
Copy link
Author

celeritydesign commented Oct 30, 2024

(Edited as I was getting confused on what board I was on :-) )

While this feature is fairly reasonable, are we sure it'll not break any code ?

Not sure how, in the current implementation? It’s an extra optional argument, and if it’s not passed, behaviour remains unchanged to how it is now.

It sounds like there is a preference to make it more complex than this PR though, by changing the optional argument to be an exclusion list. This still wouldn’t have any backwards compatibility issues, other than potentially slowing down spritecollideany() a tiny amount.

@bigwhoopgames - can we discuss your use cases for passing in (potentially) multiple sprites to exclude? And if that is to be added, should we change what I’ve done so far, or add a new function to keep it separate, such as spritecollideanywithexclusions() ?

@celeritydesign
Copy link
Author

For clarity, the corresponding PR for this issue is : #3197

@aatle
Copy link
Contributor

aatle commented Oct 31, 2024

Even though the behavior remains unchanged, there will likely be performance implications because it adds an extra condition check per sprite for most cases.
Note that the current old sprite collide implementations look like they can probably be optimized or restructured, I might look at this later.

@bigwhoopgames
Copy link

If there isn't room for an exclusion list or something of that nature then maybe adding a single sprite.collideiterable function would suffice. You can then pass it any iterable and presumable there would be a few keywords it could handle like "key" and "exclude".

It would also be nice to have a uniform function definition where rect.collideiterable, mask.collideiterable and sprite.collideiterable would be the same format.

I feel like I'm getting a bit off topic here but it just seems like there is an opportunity here. But for now, I mostly use sprite.collidegroup for UI related checks and overlaps, so it is probably just fine to have an exclude self only.

@JiffyRob
Copy link
Contributor

If performance of old code is an issue, then why not structure the code like this?

collidable = sprite_group_param
if do_not_collide_with_self:
    collidable = collidable.copy() 
    collidable.remove(self_sprite)
# do whatever the code does now but iterate through the collidable variable

That way we only have an added variable assignment and if statement, which should be plenty negligible. The more expensive finagling is only done if the variable is true.

If the above loss in performance is too much, then we can also just make a separate pygame.sprite.spritecollidenot_but_not_with_self() (somebody else can name it better) function and leave the old one unchanged.

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

No branches or pull requests

6 participants