Add CollisionMethod enum for collision list methods - #2901
Merged
Merged
Conversation
check_for_collision_with_list and check_for_collision_with_lists took a bare method number from 0 to 3. Add arcade.CollisionMethod (AUTO, SPATIAL, GPU, SIMPLE) and document the choices on it. It's an IntEnum, so existing code passing numbers keeps working. The two functions duplicated the logic that picks which sprites to check; move it into one helper, _get_sprites_to_check(). Behavior is unchanged, including SPATIAL falling back to the GPU when the list has no spatial hash. Add tests covering which path each method takes (spatial hash, every sprite, or GPU, including the WebGL fallback) for both enum members and plain numbers, through both functions. The same cases pass on the previous code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
check_for_collision_with_listandcheck_for_collision_with_liststook a baremethodnumber from 0 to 3. This adds an enum for it:AUTOSPATIALGPUSIMPLEOn WebGL, anything that would use the GPU checks every sprite instead.
CollisionMethodis anIntEnum, so code passing0–3keeps working, and the enum compares equal to those numbers.Changes
arcade.CollisionMethodinarcade/sprite_list/collision.py, exported fromarcadeandarcade.sprite_list. The choices are now documented on the enum's members, and both functions' docstrings point to it. The API doc generator picks it up automatically (checked withutil/update_quick_index.py).methodparameter is now typedCollisionMethod | int, with defaultCollisionMethod.AUTO.check_for_collision_with_listspreviously had no type hint for it._get_sprites_to_check(), which compares against the enum members.SPATIALfalling back to the GPU when the list has no spatial hash, which is arguably surprising. Changing it belongs in a separate PR.Tests
test_collision_method_values: the members equal 0–3, andCollisionMethod(2) is CollisionMethod.GPU.test_collision_method_paths: 12 cases covering every method, with and without a spatial hash, small and large (> 1500) lists, and the WebGL fallback. Each case runs with the enum member and with the plain number, through both functions, and checks which path is taken (spatial hash, every sprite, or GPU). This also covers the existingTODO: Check that the right collision function is called internally.Performance
The machine was noisy, so I loaded the old version, this version, and a version with the selection logic inlined into one process, and interleaved their timings (best of 400 rounds):
check_for_collision_with_list, 5 spritescheck_for_collision_with_lists, 2 listsThe helper call costs about 15–50 ns per call (1–3%). Inlining it removed that, but I kept the helper to avoid duplicating the selection logic. Happy to inline it if you'd prefer.
🤖 Generated with Claude Code