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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/joystick #686

Merged
merged 2 commits into from Sep 23, 2014

Conversation

Projects
None yet
6 participants
@binary1248
Member

binary1248 commented Aug 20, 2014

Fixes #660. Still has to be tested by @mantognini on OS X, and possibly others that have access to a joystick 馃槃.

@binary1248 binary1248 added bug labels Aug 20, 2014

@binary1248 binary1248 self-assigned this Aug 20, 2014

@TankOs

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Aug 28, 2014

Member

How to test? (it builds fine)

Member

TankOs commented Aug 28, 2014

How to test? (it builds fine)

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Aug 28, 2014

Member

Grab a joystick, and if possible a laptop with built-in accelerometer and see if it spews out false-positives 馃槢.

Member

binary1248 commented Aug 28, 2014

Grab a joystick, and if possible a laptop with built-in accelerometer and see if it spews out false-positives 馃槢.

@TankOs

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Aug 28, 2014

Member

No, I mean: Is there a specific test case? I have no idea what exactly to test to help you out with this. Do I just move around the stick to check if it works? ;)

Member

TankOs commented Aug 28, 2014

No, I mean: Is there a specific test case? I have no idea what exactly to test to help you out with this. Do I just move around the stick to check if it works? ;)

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Aug 28, 2014

Member

Well... it should pass that test, so yeah use a joystick and see if everything works. Other than that, as already mentioned in #660, this should fix a memory leak as well, so valgrind will help with that. And finally, it will fix the accelerometer issue in #660. Obviously if you don't have an accelerometer you won't be able to test that part of the patch, but I'm pretty sure there are other volunteers 馃槢.

Member

binary1248 commented Aug 28, 2014

Well... it should pass that test, so yeah use a joystick and see if everything works. Other than that, as already mentioned in #660, this should fix a memory leak as well, so valgrind will help with that. And finally, it will fix the accelerometer issue in #660. Obviously if you don't have an accelerometer you won't be able to test that part of the patch, but I'm pretty sure there are other volunteers 馃槢.

@TankOs

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Aug 28, 2014

Member

I'm creating a joystick example program for SFML (to be hopefully included in the future; there's not a single example using a joystick) to test this. However, sf::Joystick::isConnected() already returns false for every index. I have 2 joysticks (Logitech Rumblepad 2 and Saitek X52 Pro) connected.

With current master's HEAD checked out, it works though. I haven't taken a look in the sources at all yet (no time), but maybe you already have an idea why that might happen.

Member

TankOs commented Aug 28, 2014

I'm creating a joystick example program for SFML (to be hopefully included in the future; there's not a single example using a joystick) to test this. However, sf::Joystick::isConnected() already returns false for every index. I have 2 joysticks (Logitech Rumblepad 2 and Saitek X52 Pro) connected.

With current master's HEAD checked out, it works though. I haven't taken a look in the sources at all yet (no time), but maybe you already have an idea why that might happen.

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Aug 28, 2014

Member

Do you call Joystick::update() before doing anything else?

Member

LaurentGomila commented Aug 28, 2014

Do you call Joystick::update() before doing anything else?

@NoobsArePeople2

This comment has been minimized.

Show comment
Hide comment
@NoobsArePeople2

NoobsArePeople2 Aug 28, 2014

Contributor

@TankOs I have a joystick test app here.

Contributor

NoobsArePeople2 commented Aug 28, 2014

@TankOs I have a joystick test app here.

@TankOs

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs Aug 29, 2014

Member

@LaurentGomila I tried that, yes, it didn't make a difference. Also, it works without update() in master (just like the docs say, i.e. when a window is involved, a call to update() is not sufficient anymore).

@NoobsArePeople2 That's a nice example!

Member

TankOs commented Aug 29, 2014

@LaurentGomila I tried that, yes, it didn't make a difference. Also, it works without update() in master (just like the docs say, i.e. when a window is involved, a call to update() is not sufficient anymore).

@NoobsArePeople2 That's a nice example!

@NoobsArePeople2

This comment has been minimized.

Show comment
Hide comment
@NoobsArePeople2

NoobsArePeople2 Aug 29, 2014

Contributor

@TankOs Thanks! You're welcome to hack on it and add it to the examples if it suits your goals for the example.

Contributor

NoobsArePeople2 commented Aug 29, 2014

@TankOs Thanks! You're welcome to hack on it and add it to the examples if it suits your goals for the example.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Sep 12, 2014

Member

@binary1248 Works fine on OS X :-)

@NoobsArePeople2 very useful app you have there! Thanks for sharing. I've made some modification there in case you're interested.

Member

mantognini commented Sep 12, 2014

@binary1248 Works fine on OS X :-)

@NoobsArePeople2 very useful app you have there! Thanks for sharing. I've made some modification there in case you're interested.

@NoobsArePeople2

This comment has been minimized.

Show comment
Hide comment
@NoobsArePeople2

NoobsArePeople2 Sep 13, 2014

Contributor

@mantognini Sweet! Care to submit a pull request with your changes?

Contributor

NoobsArePeople2 commented Sep 13, 2014

@mantognini Sweet! Care to submit a pull request with your changes?

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Sep 19, 2014

Member

I've added this issue to my merge list, however I'll wait for a few more Unix users to test it. So please do test! 馃槈

Member

eXpl0it3r commented Sep 19, 2014

I've added this issue to my merge list, however I'll wait for a few more Unix users to test it. So please do test! 馃槈

@eXpl0it3r eXpl0it3r merged commit eec9f77 into master Sep 23, 2014

@eXpl0it3r eXpl0it3r added this to the 2.2 milestone Sep 23, 2014

@eXpl0it3r eXpl0it3r deleted the bugfix/joystick branch Sep 23, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment