Skip to content

Commit ef757d1

Browse files
authored
Make spritelist remove() and pop() more preformant (#2624)
* Make SpriteList.pop() O(1) instead of O(n) * Make SpriteList.remove() 3 x faster Because the index buffer and spritelist share the same index we only need to resolve the index once. Removing by value in an array is a lot more expensive than removing by value in a list. * Add complexity info in docstrings
1 parent 68a9c7b commit ef757d1

1 file changed

Lines changed: 30 additions & 6 deletions

File tree

arcade/sprite_list/sprite_list.py

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,8 @@ def clear(self, *, capacity: int | None = None, deep: bool = True) -> None:
600600
self._init_deferred()
601601

602602
def pop(self, index: int = -1) -> SpriteType:
603-
"""Attempt to pop a sprite from the list.
603+
"""
604+
Attempt to pop a sprite from the list.
604605
605606
This works like :external:ref:`popping from <tut-morelists>` a
606607
standard Python :py:class:`list`:
@@ -609,15 +610,34 @@ def pop(self, index: int = -1) -> SpriteType:
609610
#. If no ``index`` is passed, try to pop the last
610611
:py:class:`Sprite` in the list
611612
613+
This is the most efficient way to remove a sprite from the list.
614+
The complexity of this method is ``O(1)``.
615+
612616
Args:
613617
index:
614618
Index of sprite to remove (defaults to ``-1`` for the last item)
615619
"""
616620
if len(self.sprite_list) == 0:
617621
raise IndexError("pop from empty list")
618622

619-
sprite = self.sprite_list[index]
620-
self.remove(sprite)
623+
sprite = self.sprite_list.pop(index)
624+
try:
625+
slot = self.sprite_slot[sprite]
626+
except KeyError:
627+
raise ValueError("Sprite is not in the SpriteList")
628+
629+
sprite.sprite_lists.remove(self)
630+
del self.sprite_slot[sprite]
631+
self._sprite_buffer_free_slots.append(slot)
632+
633+
_ = self._sprite_index_data.pop(index)
634+
self._sprite_index_data.append(0)
635+
self._sprite_index_slots -= 1
636+
self._sprite_index_changed = True
637+
638+
if self.spatial_hash is not None:
639+
self.spatial_hash.remove(sprite)
640+
621641
return sprite
622642

623643
def append(self, sprite: SpriteType) -> None:
@@ -681,6 +701,10 @@ def remove(self, sprite: SpriteType) -> None:
681701
"""
682702
Remove a specific sprite from the list.
683703
704+
Note that this method is ``O(N)`` in complexity and will have
705+
and increased cost the more sprites you have in the list.
706+
A faster option is to use :py:meth:`pop` or :py:meth:`swap`.
707+
684708
Args:
685709
sprite: Item to remove from the list
686710
"""
@@ -689,14 +713,14 @@ def remove(self, sprite: SpriteType) -> None:
689713
except KeyError:
690714
raise ValueError("Sprite is not in the SpriteList")
691715

692-
self.sprite_list.remove(sprite)
716+
index = self.sprite_list.index(sprite)
717+
self.sprite_list.pop(index)
693718
sprite.sprite_lists.remove(self)
694719
del self.sprite_slot[sprite]
695720

696721
self._sprite_buffer_free_slots.append(slot)
697722

698-
# Brutal resize for now. Optimize later
699-
self._sprite_index_data.remove(slot)
723+
self._sprite_index_data.pop(index)
700724
self._sprite_index_data.append(0)
701725
self._sprite_index_slots -= 1
702726
self._sprite_index_changed = True

0 commit comments

Comments
 (0)