Sprite group collide tweaks

[celeritydesign]

As per initial discussion at: #3193

This PR expands two Sprite collide functions, pygame.sprite.spritecollideany() and pygame.sprite.spritecollide() to allow an optional arg, ignore_self that will disregard the sprite being tested, with itself.

Without this arg, both functions report collisions with the sprite's own rect, if that sprite belongs to the group being tested.
My feeling is that this could be counter-intuitive, especially to new users.
For example, if someone sets up a group "all_ships" and wanted to see if any of them are hitting each other, the result would always be a hit. Even if only one ship was being drawn to the screen, the above functions would report that the ship had hit something (it is hitting itself!).

[it is obviously fairly easy to work around the above situation, for example you could remove a sprite from a group prior to checking it and then add it back in - this PR doesn't dispute this, it is more about making things easier for new users]

With the new "ignore_self" arg, no collisions would be reported unless the sprite were actually overlapping a different sprite within the group, which I would suggest is what a new user would expect.

On the discord, there was talk of expanding this functionality to add an 'exclusion' list of sprites. This PR does not currently do this. My own feeling is that this perhaps adds too much complexity, especially to spritecollideany() which I think of as a "quick and cheap" way of doing initial collision detection, prior to more accurate checks such as using masks. However, I could obviously change the functionality to work like that if that is the consensus.

Finally, please note this is my first PR to the project, so apologies if I have done anything incorrectly (for example, I've done three commits, sorry!).
I have read the wiki contribution articles, run ruff on my changes and tested (more details below). I did not know how to regenerate the HTML documentation locally, but I've updated \docs\reST\ref\sprite.rst - is that all that is required?

--

Testing this PR, 1 of 2: Unit Tests

I updated sprite_text.py and added a new function to test both functions, both with default behaviour, and the new arg. I do this twice for each function, once with a callback collide function, and one with it omitted.

Testing this PR, 2 of 2: Messy visual test

I also cobbled together this test script (code below), which toggles between ignore_self = False and ignore_self = True by pressing the space bar. Move the box around using cursor keys. I use visual indicators to show how many sprites each box is hitting. (note orange is using a rectratio scale bigger than the sprite, so you turn orange before you overlap)

ignore_self = False - every box thinks it is hitting one sprite (themselves!). Orange and Red colours show spritecollideany() hits, black and yellow inner squares denote 1 hit each returned by two calls to spritecollide()

ignore_self = True - here we are actually overlapping some of the boxes. You can see the two top boxes are only reporting one hit (one black inner box, and one yellow one), and the bottom box is reporting two hits from each call.

(if this is confusing, I wrote it late at night! - look at the code and move the box around yourself and it will make sense :-) )

Test code:

import pygame

pygame.init()
screen = pygame.display.set_mode((600, 600))
clock = pygame.time.Clock()
running = True
dt = 0
all_squares = pygame.sprite.Group()
sprite_size = 50
ignore_self = False


def set_window_caption():
    if ignore_self:
        pygame.display.set_caption(
            "Sprites won't trigger collisions with themselves in group 'all_squares'"
        )
    else:
        pygame.display.set_caption("Sprites will hit themselves in group 'all_squares'")


class Square(pygame.sprite.Sprite):
    def __init__(self, pos, colour="blue"):
        pygame.sprite.Sprite.__init__(self, all_squares)
        self.colour = colour
        self.image = pygame.Surface((sprite_size, sprite_size))
        pygame.draw.rect(self.image, "blue", self.image.get_rect(), 0)
        self.rect = self.image.get_rect()
        self.rect.x, self.rect.y = pos[0], pos[1]

    def update(self):
        global ignore_self
        pygame.draw.rect(self.image, self.colour, self.image.get_rect(), 0)

        #test spritecollideany() with passed collided function, squares will turn orange if they are
        #within twice the size of the sprite's rect
        if pygame.sprite.spritecollideany(
            self, all_squares, pygame.sprite.collide_rect_ratio(2), ignore_self
        ):
            pygame.draw.rect(self.image, "orange", self.image.get_rect(), 0)

        #test spritecollideany() with no collided function, squares will get an inner red square if they
        #overlap another sprite
        if pygame.sprite.spritecollideany(self, all_squares, None, ignore_self):
            pygame.draw.rect(
                self.image, "red", (5, 5, sprite_size - 10, sprite_size - 10), 0
            )

        #test spritecollide() with passed collided function,
        #it will overlay a yellow square on itself for each sprite it hits
        hits = pygame.sprite.spritecollide(
            self, all_squares, None, pygame.sprite.collide_rect_ratio(1), ignore_self
        )

        for i, _ in enumerate(hits):
            pygame.draw.rect(self.image, "yellow", (8 * (i + 1), 40, 5, 5), 0)

        #test spritecollide() with no collided function, it will overlay a black square on itself for each sprite it hits
        hits = pygame.sprite.spritecollide(self, all_squares, None, None, ignore_self)

        for i, _ in enumerate(hits):
            pygame.draw.rect(self.image, "black", (8 * (i + 1), 20, 5, 5), 0)


player_square = Square((screen.get_width() / 2, screen.get_height() / 2), "green")
other_square = Square((screen.get_width() / 3, screen.get_height() / 5))
another_square = Square((screen.get_width() / 3 + 70, screen.get_height() / 5))

player_pos = pygame.Vector2(screen.get_width() / 2, screen.get_height() / 2)
set_window_caption()

while running:
    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            running = False
        elif event.type == pygame.KEYDOWN:
            if event.key == pygame.K_SPACE:
                ignore_self = not ignore_self
                set_window_caption()

    screen.fill("gray")

    keys = pygame.key.get_pressed()
    if keys[pygame.K_UP]:
        player_pos.y -= 300 * dt
    if keys[pygame.K_DOWN]:
        player_pos.y += 300 * dt
    if keys[pygame.K_LEFT]:
        player_pos.x -= 300 * dt
    if keys[pygame.K_RIGHT]:
        player_pos.x += 300 * dt

    #move the player
    player_square.rect.x = player_pos.x
    player_square.rect.y = player_pos.y

    all_squares.update()
    all_squares.draw(screen)

    pygame.display.flip()

    dt = clock.tick(60) / 1000

pygame.quit()

[celeritydesign]

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

[ankith26]

I've not thought about the API details yet, this review is just based on my initial look.

Thanks for contributing, anyways! 🎉

[MyreMylar]

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

[MyreMylar]

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)

[MyreMylar]

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.

[MyreMylar]

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

[MyreMylar]

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

[MyreMylar]

This seems fine, I like that it is optional as no doubt some collision functions are already checking this.

Only change I would probably make - as indicated in the suggestions - is to do the quick check for equality with self before running the collision function in each place where you have added it. Just because saving a collision check is probably worth saving a whole bunch of self checks - though the results may vary depending on the expensiveness of the collision function. It may be worth a bit of performance testing with different collision functions and different numbers of other collidables to see if it has any impact. I would expect a large one in a case with say 5 other collidable sprites and a quite complicated collision function like the pixel perfect mask based one, but a different result with a hundred collidable sprites and a basic rectangle to rectangle collision function.

[aatle]

If this feature is to be implemented, it should integrate #3209 which would significantly simplify the implementation for spritecollide: it could be as simple as an if ignore_self: collide_sprites.remove(self) .

[aatle]

Suggested change

	ignore_self: bool,
	) -> List[_TSprite]: ...
	ignore_self: bool = False,
	) -> list[_TSprite]: ...

[aatle]

Suggested change

	ignore_self: bool,
	ignore_self: bool = False,