pos attribute added to MOUSEWHEEL event#3783
Conversation
Position information in the MOUSEWHEEL event was added with SDL 2.0.26. In case pygame was built with an SDL version older than 2.0.26, the `pos` value will be `(0, 0)`.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough📝 Walkthrough🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/reST/ref/event.rst`:
- Around line 163-164: Replace the awkward phrase in the MOUSEWHEEL note so it
clearly ties the fallback to the SDL build: update the sentence referencing the
``MOUSEWHEEL`` event and its ``pos`` attribute to say that if pygame was built
against an older SDL version (pre-2.0.26) the ``pos`` attribute will be ``(0,
0)``; ensure the words “built against an older SDL version (pre-2.0.26)” replace
“was build on a version lower than that” to remove ambiguity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3079e045-217a-48af-978a-6671418afc99
📒 Files selected for processing (2)
docs/reST/ref/event.rstsrc_c/event.c
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/reST/ref/event.rst (1)
163-165:⚠️ Potential issue | 🟡 MinorPolish footnote wording for clarity and grammar.
Please change “was build on a version lower than that” to “was built against an older SDL version” and fix punctuation/spacing after “SDL 2.26.0”.
Suggested diff
-(``5``): Position information was added to ``MOUSEWHEEL`` events in SDL 2.26.0 If -pygame was build on a version lower than that, the ``pos`` attribute is ``(0, 0)``. +(``5``): Position information was added to ``MOUSEWHEEL`` events in SDL 2.26.0. If +pygame was built against an older SDL version, the ``pos`` attribute is ``(0, 0)``.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reST/ref/event.rst` around lines 163 - 165, Update the footnote text labeled "(``5``): Position information was added to ``MOUSEWHEEL`` events in SDL 2.26.0 If pygame was build on a version lower than that, the ``pos`` attribute is ``(0, 0)``." — change “was build on a version lower than that” to “was built against an older SDL version” and fix the punctuation/spacing after “SDL 2.26.0” so it reads “…SDL 2.26.0. If pygame was built against an older SDL version, the ``pos`` attribute is ``(0, 0)``.” Ensure the footnote text with marker (``5``) is updated exactly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/reST/ref/event.rst`:
- Around line 163-165: Update the footnote text labeled "(``5``): Position
information was added to ``MOUSEWHEEL`` events in SDL 2.26.0 If pygame was
build on a version lower than that, the ``pos`` attribute is ``(0, 0)``." —
change “was build on a version lower than that” to “was built against an older
SDL version” and fix the punctuation/spacing after “SDL 2.26.0” so it reads
“…SDL 2.26.0. If pygame was built against an older SDL version, the ``pos``
attribute is ``(0, 0)``.” Ensure the footnote text with marker (``5``) is
updated exactly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8c5c06be-b377-413d-84ae-be0e29ea4a19
📒 Files selected for processing (2)
docs/reST/ref/event.rstsrc_c/event.c
✅ Files skipped from review due to trivial changes (1)
- src_c/event.c
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src_c/event.c (1)
1383-1393: Add regression tests forMOUSEWHEEL.posto lock API behavior.This introduces a new public attribute, but current test context shows no dedicated attribute-level
MOUSEWHEELtest. Please add coverage for both populatedposand the(0, 0)fallback path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src_c/event.c` around lines 1383 - 1393, Add unit tests that verify the new public attribute MOUSEWHEEL.pos returns the expected tuple when the SDL wheel fields are present and also returns (0, 0) when those fields are absent; specifically, create tests that simulate an event with event->wheel.mouse_x / event->wheel.mouseX values and assert MOUSEWHEEL.pos equals that pair (to exercise the pg_tuple_couple_from_values_int branch), and a separate test that simulates the older SDL path where the code hits the fallback branch and assert MOUSEWHEEL.pos == (0, 0). Ensure tests exercise both SDL_VERSION_ATLEAST(3,0,0) and the else fallback behavior (or mock the event fields directly) so the API behavior is locked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src_c/event.c`:
- Around line 1383-1393: Add unit tests that verify the new public attribute
MOUSEWHEEL.pos returns the expected tuple when the SDL wheel fields are present
and also returns (0, 0) when those fields are absent; specifically, create tests
that simulate an event with event->wheel.mouse_x / event->wheel.mouseX values
and assert MOUSEWHEEL.pos equals that pair (to exercise the
pg_tuple_couple_from_values_int branch), and a separate test that simulates the
older SDL path where the code hits the fallback branch and assert MOUSEWHEEL.pos
== (0, 0). Ensure tests exercise both SDL_VERSION_ATLEAST(3,0,0) and the else
fallback behavior (or mock the event fields directly) so the API behavior is
locked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 21975845-ff3d-47bf-a3b5-b63b8c0526d9
📒 Files selected for processing (1)
src_c/event.c
oddbookworm
left a comment
oddbookworm
left a comment
There was a problem hiding this comment.
I have one definitely actionable comment, and one I would like to see justified if behavior is correct, or adjusted if incorrect
Starbuck5
added
New API
damusss
left a comment
damusss
left a comment
There was a problem hiding this comment.
I pointed out a few typos, but other than that, it looks good to me, thank you!
4 participants
Up to now, if the position where a MOUSEWHEEL event happends is required, a user has to either fall back on the additionally created MOUSEBUTTON events, or poll the mouse position manually.
Position information in the MOUSEWHEEL event was added with SDL 2.0.26. In case pygame was built with an SDL version older than 2.0.26, the
posvalue will beNone.