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.

Sign up for GitHub

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

Jump to bottom

Add warning about XButton1 and 2 on X11. #2038

Closed
wants to merge 1 commit into from
Closed

Add warning about XButton1 and 2 on X11. #2038

wants to merge 1 commit into from

Conversation

elijahfhopp
Copy link
Contributor

Description

Adds a warning about the two XButtons not being polled on Linux. This closes a task for 2.6.0.

@codecov
Copy link

codecov bot commented Mar 8, 2022 •
edited

Codecov Report

❗ No coverage uploaded for pull request base (2.6.x@28279c0). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             2.6.x   #2038   +/-   ##
=======================================
  Coverage         ?   8.93%           
=======================================
  Files            ?     185           
  Lines            ?   15856           
  Branches         ?    4174           
=======================================
  Hits             ?    1416           
  Misses           ?   14308           
  Partials         ?     132

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28279c0...085395d. Read the comment docs.

@kimci86
Copy link
Contributor

kimci86 commented Mar 8, 2022

The linked forum post tells that XButton1 and XButton2 are not detected with isButtonPressed but work well with sf::Event. Only the real-time polling is not available, so a warning would maybe better fit in isButtonPressed documentation.

@elijahfhopp
Copy link
Contributor Author

Ah. I should have read more carefully. I need to target the 2.6.x branch, too.

@eXpl0it3r eXpl0it3r added this to Discussion in SFML 2.6.0 via automation Mar 10, 2022
@eXpl0it3r eXpl0it3r added this to the 2.6 milestone Mar 10, 2022
@eXpl0it3r eXpl0it3r moved this from Discussion to WIP in SFML 2.6.0 Mar 10, 2022
@elijahfhopp
Copy link
Contributor Author

elijahfhopp commented Mar 15, 2022 •
edited

There is no chance I will get around to this before next week. 😬 someone welcome to close and replace.

@eXpl0it3r eXpl0it3r changed the base branch from master to 2.6.x April 19, 2022 06:41
@eXpl0it3r
Copy link
Member

I've rebased your commit onto 2.6.x and updated the target branch 🙂

@eXpl0it3r eXpl0it3r moved this from WIP to Review & Testing in SFML 2.6.0 Apr 19, 2022
SFML 2.6.0 automation moved this from Review & Testing to Ready Apr 19, 2022
eXpl0it3r
eXpl0it3r requested changes Apr 19, 2022
View reviewed changes
Copy link
Member

@eXpl0it3r eXpl0it3r left a comment

Choose a reason for hiding this comment

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

Just got reminded of @kimci86's comment about the original report...

include/SFML/Window/Mouse.hpp
XButton1, //!< The first extra mouse button
XButton2, //!< The second extra mouse button
XButton1, //!< The first extra mouse button (not available on Linux)
XButton2, //!< The second extra mouse button (not available on Linux)
Copy link
Member

Choose a reason for hiding this comment

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

As pointed out by @kimci86, these buttons are available through events, so I guess we might not want to have this here

include/SFML/Window/Mouse.hpp
@@ -172,6 +172,8 @@ class SFML_WINDOW_API Mouse
/// sf::Mouse::setPosition(sf::Vector2i(100, 200), window);
/// \endcode
///
/// \warning sf::Mouse::XButton1 and sf::Mouse::XButton2 are not available on Linux.
///
Copy link
Member

Choose a reason for hiding this comment

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

As pointed out by @kimci86, these buttons are available through events, so we might want to move this warning to isButtonPressed instead

SFML 2.6.0 automation moved this from Ready to Review & Testing Apr 19, 2022
@vittorioromeo
Copy link
Member

@mrzeusthecoder: are you interested in continuing work on this PR?

@elijahfhopp
Copy link
Contributor Author

No, sorry about this all. Life has been crazy!

SFML 2.6.0 automation moved this from Review & Testing to Done Jun 18, 2022
@eXpl0it3r eXpl0it3r moved this from Done to WIP in SFML 2.6.0 Jun 22, 2022
@kimci86 kimci86 mentioned this pull request Nov 13, 2022
Merged
@eXpl0it3r eXpl0it3r moved this from WIP to Done in SFML 2.6.0 Dec 19, 2022
@dogunbound dogunbound mentioned this pull request May 17, 2023
Merged
@ChrisThrasher ChrisThrasher mentioned this pull request Jun 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eXpl0it3r eXpl0it3r eXpl0it3r requested changes

Assignees
No one assigned
Labels
feature m:doc p:linux s:superseded
Projects
No open projects
SFML 2.6.0
  
Done
Milestone
2.6
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@elijahfhopp @kimci86 @eXpl0it3r @vittorioromeo