Skip to content

Commit 0c91ec4

Browse files
authored
Fix platforms.deleter + add tests for physics behavior properties (pythonarcade#2166)
* Fix PhysicsEnginePlatformer.platforms.deleter by using list.clear * Add tests for clearing rather than overwriting * Update comments in the physics engine tests
1 parent 50014a5 commit 0c91ec4

File tree

2 files changed

+42
-1
lines changed

2 files changed

+42
-1
lines changed

arcade/physics_engines.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ def platforms(self, platforms: Optional[Union[SpriteList, Iterable[SpriteList]]]
387387

388388
@platforms.deleter
389389
def platforms(self):
390-
self._platforms = []
390+
self._platforms.clear()
391391

392392
@property
393393
def walls(self):

tests/unit/physics_engine/test_physics_engine2.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,43 @@
88
OUT_OF_THE_WAY = (250, 250)
99

1010

11+
def check_spritelists_prop_clears_instead_of_overwrites(engine, prop_name: str):
12+
"""Some properties are backed by lists which shouldn't be recreated.
13+
14+
Implementing them this way is helpful for cases when a user has lots
15+
of different SpriteLists added to the physics engine's lists. It
16+
helps by avoiding:
17+
18+
* extra allocations from copying lists (see arcade.utils.Chain)
19+
* Avoid GC thrash from creating & deleting items
20+
"""
21+
def _get_current():
22+
return getattr(engine, f'_{prop_name}')
23+
24+
original_list = _get_current()
25+
26+
# Make sure we don't replace it when adding
27+
setattr(engine, prop_name, [arcade.SpriteList()])
28+
assert _get_current() is original_list
29+
30+
# Ensure setting a property to None doesn't kill it
31+
setattr(engine, prop_name, None)
32+
assert _get_current() is original_list
33+
34+
# Ensure empty iterables don't clobber
35+
setattr(engine, prop_name, [])
36+
assert _get_current() is original_list
37+
setattr(engine, prop_name, ())
38+
assert _get_current() is original_list
39+
setattr(engine, prop_name, (arcade.SpriteList() for _ in range(0)))
40+
41+
# We support the del keyword on these properties for some reason,
42+
# but it should only clear the internal list instead of deleting the
43+
# property.
44+
delattr(engine, prop_name)
45+
assert _get_current() is original_list
46+
47+
1148
def basic_tests(moving_sprite, wall_list, physics_engine):
1249
"""Run basic tests that can be done by both engines."""
1350
wall_sprite_1 = wall_list[0]
@@ -221,6 +258,7 @@ def simple_engine_tests(moving_sprite, wall_list, physics_engine):
221258
else:
222259
assert len(collisions) == 1
223260

261+
check_spritelists_prop_clears_instead_of_overwrites(physics_engine, 'walls')
224262

225263
def platformer_tests(moving_sprite, wall_list, physics_engine):
226264
wall_sprite_1 = wall_list[0]
@@ -289,6 +327,9 @@ def platformer_tests(moving_sprite, wall_list, physics_engine):
289327
collisions = physics_engine.update()
290328
assert moving_sprite.position == (3, -6)
291329

330+
check_spritelists_prop_clears_instead_of_overwrites(physics_engine, 'platforms')
331+
check_spritelists_prop_clears_instead_of_overwrites(physics_engine, 'ladders')
332+
292333

293334
# Temp fix for https://github.com/pythonarcade/arcade/issues/2074
294335
def nocopy_tests(physics_engine):

0 commit comments

Comments
 (0)