Fix SpriteList pop with negative index, rescale, and lazy preload

CHANGELOG.md

@@ -23,6 +23,9 @@ Arcade [PyPi Release History](https://pypi.org/project/arcade/#history) page.
 - `check_for_collision_with_lists` no longer returns the same sprite more than once when it's in more than one of the lists.
 - `CollisionMethod.SPATIAL` (`method=1`) on a sprite list without a spatial hash now chooses the same way as `AUTO`, checking every sprite in lists of 1500 or fewer. Before, it always used the GPU, which is slow when called many times per frame and needed an open window.
 - Documented that sprites whose hit boxes only touch don't count as colliding, while a point exactly on a hit box's edge does count for `get_sprites_at_point` and `collides_with_point`. Corrected `get_sprites_in_rect`'s docs, which said touching sprites were included.
+- Fixed `SpriteList.pop()` with a negative index other than `-1` (such as `pop(-2)`) drawing the wrong sprites: the removed sprite stayed on screen and another sprite disappeared.
+- Fixed `SpriteList.rescale()` moving the list's center while rescaling, so sprites after the first were scaled around the wrong point.
+- Fixed `SpriteList.preload_textures()` raising `AttributeError` on a lazy sprite list that hadn't been drawn yet. It now preloads into the atlas the list will use.
 
 ### New Features
 - Added `HitBox.get_adjusted_bounds()`, which returns the cached `(left, right, bottom, top)` bounds of the adjusted hit box points.

arcade/sprite_list/sprite_list.py

@@ -219,7 +219,7 @@ class SpriteList(SpriteSequence[SpriteType]):
     #:     # Set global default to nearest filtering (pixelated)
     #:     arcade.SpriteList.DEFAULT_TEXTURE_FILTER = gl.NEAREST, gl.NEAREST
     #:     # Set global default to linear filtering (smooth). This is the default.
-    #:     arcade.SpriteList.DEFAULT_TEXTURE_FILTER = gl.NEAREST, gl.NEAREST
+    #:     arcade.SpriteList.DEFAULT_TEXTURE_FILTER = gl.LINEAR, gl.LINEAR
     DEFAULT_TEXTURE_FILTER: ClassVar[tuple[int, int]] = gl.LINEAR, gl.LINEAR
 
     # Declare `special_hash` as an attribute that implements the abstract
@@ -279,7 +279,7 @@ def __init__(
         self._sprite_color_data = array("B", [0] * self._buf_capacity * 4)
         self._sprite_texture_data = array("f", [0] * self._buf_capacity)
         # Index buffer
-        self._sprite_index_data = array("i", [0] * self._idx_capacity)
+        self._sprite_index_data = array("I", [0] * self._idx_capacity)
 
         self._data: SpriteListData | None = None
 
@@ -604,8 +604,15 @@ def pop(self, index: int = -1) -> SpriteType:
             index:
                 Index of sprite to remove (defaults to ``-1`` for the last item)
         """
-        if len(self.sprite_list) == 0:
+        sprite_count = len(self.sprite_list)
+        if sprite_count == 0:
             raise IndexError("pop from empty list")
+        if not -sprite_count <= index < sprite_count:
+            raise IndexError("pop index out of range")
+        # The index buffer is longer than the list (it has spare capacity at
+        # the end), so a negative index must be made positive before using it.
+        if index < 0:
+            index += sprite_count
 
         sprite = self.sprite_list.pop(index)
         try:
@@ -872,8 +879,12 @@ def _get_center(self) -> tuple[float, float]:
 
     def rescale(self, factor: float) -> None:
         """Rescale all sprites in the list relative to the spritelists center."""
+        if not self.sprite_list:
+            return
+        # Find the center before any sprite moves
+        center = self.center
         for sprite in self.sprite_list:
-            sprite.rescale_relative_to_point(self.center, factor)
+            sprite.rescale_relative_to_point(center, factor)
 
     def move(self, change_x: float, change_y: float) -> None:
         """
@@ -897,12 +908,17 @@ def preload_textures(self, texture_list: Iterable[Texture]) -> None:
         Args:
             texture_list: List of textures.
         """
-        if not self.ctx:
-            raise ValueError("Cannot preload textures before the window is created")
+        atlas = self._atlas
+        if atlas is None:
+            # Not initialized yet (a lazy list, or no window when it was
+            # created). Use the atlas the list will get when it initializes.
+            try:
+                atlas = get_window().ctx.default_atlas
+            except RuntimeError:
+                raise ValueError("Cannot preload textures before the window is created")
 
         for texture in texture_list:
-            # Ugly spacing is a fast workaround for None type checking issues
-            self._atlas.add(texture)  # type: ignore
+            atlas.add(texture)
 
     def write_sprite_buffers_to_gpu(self) -> None:
         """

tests/unit/spritelist/test_spritelist.py

@@ -331,3 +331,93 @@ def test_swap(window):
     sl.draw()
     assert arcade.get_pixel(x=0, y=0, components=4) == arcade.color.RED
     assert sl.sprite_list == sprites[::-1]
+
+
+def _drawn_sprites(spritelist):
+    """The sprites the GPU will draw, in order, read back from the GPU index buffer"""
+    spritelist.draw()
+    count = spritelist._sprite_index_slots
+    gpu_slots = struct.unpack(f"{count}I", spritelist.data.storage_index.read()[: count * 4])
+    sprite_for_slot = {slot: sprite for sprite, slot in spritelist.sprite_slot.items()}
+    return [sprite_for_slot.get(slot) for slot in gpu_slots]
+
+
+@pytest.mark.parametrize("index", [0, 1, 2, 3, -1, -2, -3, -4])
+def test_pop_index(ctx, index):
+    """Popping at any index keeps the GPU draw order in sync with the list"""
+    spritelist = make_named_sprites(4)
+    sprites = list(spritelist)
+    spritelist.draw()
+
+    popped = spritelist.pop(index)
+    expected = sprites.copy()
+    assert popped is expected.pop(index)
+    assert list(spritelist) == expected
+    assert _drawn_sprites(spritelist) == expected
+
+    # A new sprite reuses the popped sprite's buffer slot
+    new_sprite = arcade.SpriteSolidColor(16, 16)
+    spritelist.append(new_sprite)
+    assert _drawn_sprites(spritelist) == expected + [new_sprite]
+
+
+def test_pop_repeatedly(ctx):
+    """Popping from the middle with negative indexes until the list is empty"""
+    spritelist = make_named_sprites(6)
+    expected = list(spritelist)
+    while len(spritelist) > 1:
+        assert spritelist.pop(-2) is expected.pop(-2)
+        assert _drawn_sprites(spritelist) == expected
+    spritelist.pop()
+    assert len(spritelist) == 0
+
+
+@pytest.mark.parametrize("index", [4, -5, 100, -100])
+def test_pop_out_of_range(ctx, index):
+    spritelist = make_named_sprites(4)
+    sprites = list(spritelist)
+    with pytest.raises(IndexError):
+        spritelist.pop(index)
+    # Nothing was removed
+    assert list(spritelist) == sprites
+    assert _drawn_sprites(spritelist) == sprites
+
+
+def test_pop_empty(ctx):
+    with pytest.raises(IndexError):
+        arcade.SpriteList().pop()
+
+
+def test_rescale_around_center(ctx):
+    """All sprites are rescaled around the same center, found before any move"""
+    spritelist = arcade.SpriteList()
+    for x, y in ((0, 0), (100, 0), (50, 60)):
+        spritelist.append(arcade.SpriteSolidColor(10, 10, center_x=x, center_y=y))
+    # The center is (50, 20)
+    spritelist.rescale(2)
+    assert [sprite.position for sprite in spritelist] == [(-50, -20), (150, -20), (50, 100)]
+    assert [sprite.scale for sprite in spritelist] == [(2.0, 2.0)] * 3
+    assert spritelist.center == (50, 20)
+
+    # An empty list does nothing
+    arcade.SpriteList().rescale(2)
+
+
+def test_preload_textures_lazy(ctx):
+    """A lazy list can preload textures before it's initialized"""
+    texture = arcade.load_texture(":resources:images/items/coinGold.png")
+    spritelist = arcade.SpriteList(lazy=True)
+    spritelist.preload_textures([texture])
+    assert ctx.default_atlas.has_texture(texture)
+    # Preloading doesn't initialize the list
+    assert spritelist._initialized is False
+
+
+def test_index_buffer_type(ctx):
+    """The index buffer is always 32 bit unsigned integers"""
+    spritelist = make_named_sprites(3)
+    assert spritelist._sprite_index_data.typecode == "I"
+    spritelist.shuffle()
+    assert spritelist._sprite_index_data.typecode == "I"
+    spritelist.clear()
+    assert spritelist._sprite_index_data.typecode == "I"