Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mouse.(get|set)_relative_mode() (replaces _sdl2.Window.relative_mouse) #2076

Merged
merged 21 commits into from
Nov 19, 2023

Conversation

yunline
Copy link
Contributor

@yunline yunline commented Mar 30, 2023

Issue: #2073

@yunline yunline changed the title mouse.get/set_relative_mode() mouse.get/set_relative_mode() instead of _sdl2.Window.relative_mouse Mar 30, 2023
@yunline yunline marked this pull request as ready for review March 30, 2023 08:24
@yunline yunline requested a review from a team as a code owner March 30, 2023 08:24
docs/reST/ref/mouse.rst Outdated Show resolved Hide resolved
docs/reST/ref/mouse.rst Show resolved Hide resolved
src_c/mouse.c Outdated Show resolved Hide resolved
@zoldalma999 zoldalma999 added New API This pull request may need extra debate as it adds a new class or function to pygame mouse pygame.mouse labels May 25, 2023
Copy link
Member

@zoldalma999 zoldalma999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@MyreMylar
Copy link
Member

I know this PR is just moving the functions to a more proper place, however my main questions are about how this interacts with the existing functions that cover this functionality, namely:

  • mouse.get_rel()
  • mouse.set_visble()
  • mouse.get_visible()
  • event.set_grab()
  • event.get_grab()

Mostly so we can document them if it is odd, or maybe correct functionality if it does something you would not expect.

I'd guess users would likely expect all except the event.set_/get_grab() pair to maybe work OK with relative mode enabled. Then probably we could document whatever happens when you try to set/unset grab with relative mode enabled as a reminder not do it as the functionality, while distinct, overlaps somewhat. Whether those mouse function do work however is something we'll need to test...

@MyreMylar MyreMylar added this to the 2.4.0 milestone Aug 14, 2023
Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so there are a couple of things that come up because SDL_SetRelativeMouseMode() also hides the cursor and locks the mouse to the window.

As the event grab also locks the key events to the window I think it is fine that pygame.event.get_grab() doesn't return true while the mouse is in relative mode.

However - mouse.get_visible() really should be false while we are in relative mode as the mouse cursor is not visible and this function does nothing else.

Anyway, it would be an easy fix, except these functions just got updated in the main branch for SDL3 so:

  1. Please merge main into this PR.
  2. If still needed (as the functions might have changed) please change line 218 of mouse.get_visible() to something like:
result = (PG_CursorVisible() || SDL_GetRelativeMouseMode());

so that pygame.mouse.get_visible() returns False while in relative mode.

Test program:

import pygame


display_surf = pygame.display.set_mode((1280, 800))
display_surf.fill((255, 255, 255))


pygame.init()

clock = pygame.time.Clock()

running = True

mouse_state_changed = False

while running:
    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            running = False

        if event.type == pygame.KEYDOWN and event.key == pygame.K_r:
            if pygame.mouse.get_relative_mode():
                pygame.mouse.set_relative_mode(False)
            else:
                pygame.mouse.set_relative_mode(True)
            mouse_state_changed = True

        if event.type == pygame.KEYDOWN and event.key == pygame.K_v:
            if pygame.mouse.get_visible():
                pygame.mouse.set_visible(False)
            else:
                pygame.mouse.set_visible(True)
            mouse_state_changed = True

        if event.type == pygame.KEYDOWN and event.key == pygame.K_g:
            if pygame.event.get_grab():
                pygame.event.set_grab(False)
            else:
                pygame.event.set_grab(True)
            mouse_state_changed = True

    if mouse_state_changed:
        print("mouse relative mode:", pygame.mouse.get_relative_mode())
        print("mouse visible:", pygame.mouse.get_visible())
        print("events grabbed:", pygame.event.get_grab(), "\n")
        mouse_state_changed = False

    pygame.display.flip()

    clock.tick(60)

src_c/mouse.c Outdated Show resolved Hide resolved
test/mouse_test.py Outdated Show resolved Hide resolved
test/mouse_test.py Outdated Show resolved Hide resolved
@MyreMylar MyreMylar merged commit 5de705d into pygame-community:main Nov 19, 2023
29 checks passed
Calling :func:`pygame.mouse.set_visible` with argument
``True`` will exit relative mouse mode.

.. ## pygame.mouse.set_relative_mode ##
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I'm late and the PR has been merged already, but this is missing version added tags. Can open a new PR to fix this

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ankith26 ankith26 changed the title mouse.get/set_relative_mode() instead of _sdl2.Window.relative_mouse Add mouse.(get|set)_relative_mode() (replaces _sdl2.Window.relative_mouse) Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mouse pygame.mouse New API This pull request may need extra debate as it adds a new class or function to pygame
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants