Removed time-based poll for joysticks and replaced with manual update#1195
Removed time-based poll for joysticks and replaced with manual update#1195JonnyPtn wants to merge 1 commit into
Conversation
|
As @MarioLiebisch pointed out on IRC, this would stop joysticks working if there is no window opened, so I've implemented his suggested changes. The downside is that if you're using joysticks without a window then this won't change anything, but alternatives would probably change the API |
|
|
||
| ConnectionCache connectionCache[sf::Joystick::Count]; | ||
|
|
||
| //if true, will only update when WM_DEVICECHANGE message is received |
There was a problem hiding this comment.
Nitpicking: Should be // If true, ...
| } | ||
| case WM_DEVICECHANGE: | ||
| { | ||
| //some sort of device change has happened, update joystick connections |
There was a problem hiding this comment.
Same nitpicking here: // Some ...
| namespace | ||
| { | ||
| unsigned int windowCount = 0; | ||
| unsigned int handleCount = 0; |
There was a problem hiding this comment.
What's the difference between windowCount and handleCount?
There was a problem hiding this comment.
windowCount only counts windows owned by SFML
There was a problem hiding this comment.
I recommend adding a comment to avoid any future ambiguity.
|
|
||
| if (m_handle) | ||
| { | ||
| if (handleCount++ == 0) |
There was a problem hiding this comment.
I personally am not a big fan of this notation, but maybe others don't have an issue with it.
There was a problem hiding this comment.
I strongly think branching conditions should be side effect free; so yeah, I've an issue with this. 😛
There was a problem hiding this comment.
Me too. One extra line of code vs more headache when reading the code, my choice is made.
|
I've implemented the changes and squashed |
|
Only alternative for all cases would be creating an invisible window just for joystick polling (which would also remove the need for public static members in the implementation). But then again that would create a window after all, which might not be wanted in some odd cases. |
|
Merged in f053871 on master. |
Added updateConnections() to JoystickImpl which is called when WM_DEVICECHANGE message is received, this replaces the old functionality of just polling every controller every 0.5s, in the hope of addressing #1179