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

Fix Gamepad crash when platform doesn't support the amount #4677

Merged
merged 1 commit into from
Mar 30, 2016

Conversation

harry-cpp
Copy link
Member

Instead of throwing an exception we should just return an empty state, this also makes the style of this more XNAish.

http://community.monogame.net/t/monogame-3-5-gamepad-getstate/7288

@harry-cpp harry-cpp mentioned this pull request Mar 26, 2016
@tomspilman
Copy link
Member

Hopefully no one depends on the exception to detect the maximum supported gamepad. At least we added GamePad.MaximumGamePadCount for that.

@harry-cpp
Copy link
Member Author

Hopefully no one depends on the exception to detect the maximum supported gamepad. At least we added GamePad.MaximumGamePadCount for that.

Xna didn't call a crash when the gamepad was not supported(Windows Phone), but instead just returned an empty state, right?

@tomspilman
Copy link
Member

but instead just returned an empty state, right?

Well... it did return a gamepad state, but just for the back button detection.

Don't know what happened if you asked for the second gamepad.

@harry-cpp
Copy link
Member Author

Don't know what happened if you asked for the second gamepad.

From googling around I am 95% sure it returned an empty state.

@harry-cpp
Copy link
Member Author

@dellis1972 @KonajuGames What do you think on this?

@tomspilman
Copy link
Member

So I did a little research on this before merging it.

Under XNA a disconnected gamepad does not throw any exception. It just returns a disconnected state.

If you are using XNA on WP7 GamePad.GetState(PlayerIndex.Two) also does not throw an exception. It just reports it as disconnected. In fact on WP7 you can call GamePad.GetState((PlayerIndex)100) and it just returns a disconnected state.

The only case where GamePad.GetState() throws an exception is on the PC when you pass an invalid enum value. So GamePad.GetState((PlayerIndex)100) on the PC throws and exception about an invalid enum value.

So my conclusions from this:

This PR is fine... we shouldn't throw exceptions from the indexed versions of the GamePad.GetState/GetCapabilities/SetVibration when referencing more gamepads than supported on the platform. I will merge this shortly.

However we should consider following up this PR with another for the enum versions of these functions which validates that a correct PlayerIndex enum is passed it and throw an argument exception otherwise. The enum versions should be strict and behave like XNA on the desktop.

@tomspilman tomspilman merged commit 9b400b9 into MonoGame:develop Mar 30, 2016
@tomspilman
Copy link
Member

Thanks @cra0zy !

@harry-cpp harry-cpp deleted the fixgamepad branch March 30, 2016 10:01
@tomspilman tomspilman added this to the 3.6 Release milestone Mar 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants