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

Sprite group collide tweaks #3197

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions buildconfig/stubs/pygame/sprite.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ def spritecollide(
group: AbstractGroup[_TSprite],
dokill: bool,
collided: Optional[Callable[[_TSprite, _TSprite2], Any]] = None,
ignore_self: bool,
) -> List[_TSprite]: ...
def groupcollide(
groupa: AbstractGroup[_TSprite],
Expand All @@ -293,4 +294,5 @@ def spritecollideany(
sprite: _HasRect,
group: AbstractGroup[_TSprite],
collided: Optional[Callable[[_TSprite, _TSprite2], Any]] = None,
ignore_self: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ignore_self: bool,
ignore_self: bool = False,

) -> Optional[_TSprite]: ...
16 changes: 12 additions & 4 deletions docs/reST/ref/sprite.rst
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ Sprites are not thread safe. So lock them yourself if using threads.
.. function:: spritecollide

| :sl:`Find sprites in a group that intersect another sprite.`
| :sg:`spritecollide(sprite, group, dokill, collided = None) -> Sprite_list`
| :sg:`spritecollide(sprite, group, dokill, collided = None, ignore_self = False) -> Sprite_list`

Return a list containing all Sprites in a Group that intersect with another
Sprite. Intersection is determined by comparing the ``Sprite.rect``
Expand All @@ -659,6 +659,10 @@ Sprites are not thread safe. So lock them yourself if using threads.
sprites must have a "rect" value, which is a rectangle of the sprite area,
which will be used to calculate the collision.

The ignore_self argument is a bool. If set to True, the sprite will not
register a collision with itself, even if it is contained within the group
being tested.

collided callables:

::
Expand Down Expand Up @@ -825,21 +829,25 @@ Sprites are not thread safe. So lock them yourself if using threads.
.. function:: spritecollideany

| :sl:`Simple test if a sprite intersects anything in a group.`
| :sg:`spritecollideany(sprite, group, collided = None) -> Sprite` Collision with the returned sprite.
| :sg:`spritecollideany(sprite, group, collided = None) -> None` No collision
| :sg:`spritecollideany(sprite, group, collided = None, ignore_self = False) -> Sprite` Collision with the returned sprite.
| :sg:`spritecollideany(sprite, group, collided = None, ignore_self = False) -> None` No collision

If the sprite collides with any single sprite in the group, a single
sprite from the group is returned. On no collision None is returned.

If you don't need all the features of the ``pygame.sprite.spritecollide()`` function, this
function will be a bit quicker.
function will be a bit quicker..
MyreMylar marked this conversation as resolved.
Show resolved Hide resolved
MyreMylar marked this conversation as resolved.
Show resolved Hide resolved

The collided argument is a callback function used to calculate if two sprites are
colliding. It should take two sprites as values and return a bool value
indicating if they are colliding. If collided is not passed, then all
sprites must have a "rect" value, which is a rectangle of the sprite area,
which will be used to calculate the collision.

The ignore_self argument is a bool. If set to True, the sprite will not
register a collision with itself, even if it is contained within the group
being tested.

.. ## pygame.sprite.spritecollideany ##

.. ## ##
Expand Down
53 changes: 37 additions & 16 deletions src_py/sprite.py
Original file line number Diff line number Diff line change
Expand Up @@ -1664,10 +1664,10 @@ def collide_mask(left, right):
return leftmask.overlap(rightmask, (xoffset, yoffset))


def spritecollide(sprite, group, dokill, collided=None):
def spritecollide(sprite, group, dokill, collided=None, ignore_self=False):
"""find Sprites in a Group that intersect another Sprite

pygame.sprite.spritecollide(sprite, group, dokill, collided=None):
pygame.sprite.spritecollide(sprite, group, dokill, collided=None, ignore_self=False):
return Sprite_list

Return a list containing all Sprites in a Group that intersect with another
Expand All @@ -1683,6 +1683,9 @@ def spritecollide(sprite, group, dokill, collided=None):
sprites must have a "rect" value, which is a rectangle of the sprite area,
which will be used to calculate the collision.

The ignore_self argument is a bool. If set to True, the sprite will not register
a collision with itself, even if it is contained within the group being tested.

"""
# pull the default collision function in as a local variable outside
# the loop as this makes the loop run faster
Expand All @@ -1695,25 +1698,39 @@ def spritecollide(sprite, group, dokill, collided=None):
for group_sprite in group.sprites():
if collided is not None:
if collided(sprite, group_sprite):
group_sprite.kill()
append(group_sprite)
if not ignore_self or sprite != group_sprite:
Comment on lines 1700 to +1701
Copy link
Member

Choose a reason for hiding this comment

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

As indicated below you should probably do the if check for self first and then the collision check as the inner if as the self check will be quick to do and the collision relatively slower.

group_sprite.kill()
append(group_sprite)
else:
if default_sprite_collide_func(group_sprite.rect):
group_sprite.kill()
append(group_sprite)
if not ignore_self or sprite != group_sprite:
Copy link
Member

Choose a reason for hiding this comment

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

Another place to swap the order of the collision check with the self check.

group_sprite.kill()
append(group_sprite)

return crashed

if collided is not None:
if ignore_self:
return [
group_sprite for group_sprite in group if collided(sprite, group_sprite) and sprite != group_sprite
Copy link
Member

@MyreMylar MyreMylar Dec 31, 2024

Choose a reason for hiding this comment

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

Suggested change
group_sprite for group_sprite in group if collided(sprite, group_sprite) and sprite != group_sprite
group_sprite for group_sprite in group if sprite != group_sprite and collided(sprite, group_sprite)

I think it will be ever so slightly faster to do the self check first as short circuit evaluation will cause it to skip the collision check if the left hand side of the and fails and the collision check is generally going to be slower. See:

https://en.wikipedia.org/wiki/Short-circuit_evaluation

]
else:
return [
group_sprite for group_sprite in group if collided(sprite, group_sprite)
]

if ignore_self:
return [
group_sprite for group_sprite in group if collided(sprite, group_sprite)
group_sprite
for group_sprite in group
if default_sprite_collide_func(group_sprite.rect) and sprite != group_sprite
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if default_sprite_collide_func(group_sprite.rect) and sprite != group_sprite
if sprite != group_sprite and default_sprite_collide_func(group_sprite.rect)

]
else:
return [
group_sprite
for group_sprite in group
if default_sprite_collide_func(group_sprite.rect)
]

return [
group_sprite
for group_sprite in group
if default_sprite_collide_func(group_sprite.rect)
]


def groupcollide(groupa, groupb, dokilla, dokillb, collided=None):
Expand Down Expand Up @@ -1752,7 +1769,7 @@ def groupcollide(groupa, groupb, dokilla, dokillb, collided=None):
return crashed


def spritecollideany(sprite, group, collided=None):
def spritecollideany(sprite, group, collided=None, ignore_self=False):
"""finds any sprites in a group that collide with the given sprite

pygame.sprite.spritecollideany(sprite, group): return sprite
Expand All @@ -1770,6 +1787,8 @@ def spritecollideany(sprite, group, collided=None):
sprites must have a "rect" value, which is a rectangle of the sprite area,
which will be used to calculate the collision.

The ignore_self argument is a bool. If set to True, the sprite will not register
a collision with itself, even if it is contained within the group being tested.

"""
# pull the default collision function in as a local variable outside
Expand All @@ -1779,10 +1798,12 @@ def spritecollideany(sprite, group, collided=None):
if collided is not None:
for group_sprite in group:
if collided(sprite, group_sprite):
return group_sprite
if not ignore_self or sprite != group_sprite:
return group_sprite
else:
# Special case old behaviour for speed.
for group_sprite in group:
if default_sprite_collide_func(group_sprite.rect):
return group_sprite
if not ignore_self or sprite != group_sprite:
Comment on lines +1801 to +1807
Copy link
Member

Choose a reason for hiding this comment

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

And again, two more places to do the self check first - then the collision check.

return group_sprite
return None
22 changes: 22 additions & 0 deletions test/sprite_test.py
Copy link
Author

Choose a reason for hiding this comment

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

Line 505: is this an ok way to test against an empty returned list?

Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,28 @@ def test_collide_rect(self):
self.assertFalse(pygame.sprite.collide_rect(self.s1, self.s3))
self.assertFalse(pygame.sprite.collide_rect(self.s3, self.s1))

def test_sprites_ignoring_themselves_for_collisions(self):
# spritecollide(): Test that sprite collides with itself in the same group by default.
self.assertEqual(
sprite.spritecollide(self.s1, self.ag, dokill=False, collided=None),
[self.s1],
)
# spritecollide(): Test that sprite does not collide with itself in the same group when ignore_self is passed.
self.assertEqual(
sprite.spritecollide(
self.s1, self.ag, dokill=False, collided=None, ignore_self=True
),
[],
)
# spritecollideany(): Test that sprite collides with itself in the same group by default.
self.assertEqual(
sprite.spritecollideany(self.s1, self.ag, collided=None), self.s1
)
# spritecollideany(): Test that sprite does not collide with itself in the same group when ignore_self is passed.
self.assertIsNone(
sprite.spritecollideany(self.s1, self.ag, collided=None, ignore_self=True)
)


################################################################################

Expand Down