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

Updating controller mappings to latest SDL version #1594

Merged
4 commits merged into from Jan 9, 2018

Conversation

Projects
None yet
4 participants
@Thunderforge
Contributor

Thunderforge commented Jan 2, 2018

Originally #1591, which started to have Git issues

Fixes #3708. I can now use my Xbox One Bluetooth controller on a Mac with SDL 2.0.6+.

Note that this will use different mappings depending on if the linked SDL version is <= 2.0.4, 2.0.5, or >= 2.0.6. We kept this behavior because Ubuntu Xenial uses 2.0.4 and we didn't want to lock that out. Some controllers in newer mappings, like the Xbox One Bluetooth controller, are not present in older mappings.

@Thunderforge Thunderforge changed the title from Updating controller mappings to latest version to Updating controller mappings to latest SL version Jan 2, 2018

@Thunderforge Thunderforge changed the title from Updating controller mappings to latest SL version to Updating controller mappings to latest SDL version Jan 2, 2018

SDL_GetVersion(&linkedSdlVersion);
std::string controllerFileName;
if (linkedSdlVersion.major == 2 && linkedSdlVersion.minor == 0 && linkedSdlVersion.patch <= 4) {
controllerFileName = "gamecontrollerdb_204.txt";

This comment has been minimized.

@Mingun

Mingun Jan 2, 2018

If you already do such checks, maybe to make more generic solution: get the current SDL version, make a name of the appropriate file and to try to load it and if it is absent, use default file name? It will allow to avoid the same problems in the future.

@Mingun

Mingun Jan 2, 2018

If you already do such checks, maybe to make more generic solution: get the current SDL version, make a name of the appropriate file and to try to load it and if it is absent, use default file name? It will allow to avoid the same problems in the future.

This comment has been minimized.

@Thunderforge

Thunderforge Jan 2, 2018

Contributor

While I like your suggestion, I have some concerns of how practical that will be.

SDL 2.0.6 and 2.0.7 have the same format for controller mappings. Hopefully that means we are out of the woods for controller formats and can drop the old ones in the future, maybe even returning to one file. But if 2.0.8 comes out and changes the format again, then we'll have to do some logic to have one file for both 2.0.6 and 2.0.7. If that were to happen, I'm not sure we'd be saving much space.

Also, it's been a long time since I worked with files in C++ and all I remember with that was that it was a real pain. Knowing that the format is the same for two consecutive versions as described above, do you still recommend going that route? If so, could you give me a pointer on how to go about doing it?

@Thunderforge

Thunderforge Jan 2, 2018

Contributor

While I like your suggestion, I have some concerns of how practical that will be.

SDL 2.0.6 and 2.0.7 have the same format for controller mappings. Hopefully that means we are out of the woods for controller formats and can drop the old ones in the future, maybe even returning to one file. But if 2.0.8 comes out and changes the format again, then we'll have to do some logic to have one file for both 2.0.6 and 2.0.7. If that were to happen, I'm not sure we'd be saving much space.

Also, it's been a long time since I worked with files in C++ and all I remember with that was that it was a real pain. Knowing that the format is the same for two consecutive versions as described above, do you still recommend going that route? If so, could you give me a pointer on how to go about doing it?

This comment has been minimized.

@Mingun

Mingun Jan 3, 2018

No, I don't know what format of this files, but judging from the fact that this PR doesn't bring anything, as for it parsing, seemed to me that SDL parse this file itself. Search in github has also not given any hints that this file parsed by OpenMW. If this is so, then I don't see the reasons for concern though I don't insist.

@Mingun

Mingun Jan 3, 2018

No, I don't know what format of this files, but judging from the fact that this PR doesn't bring anything, as for it parsing, seemed to me that SDL parse this file itself. Search in github has also not given any hints that this file parsed by OpenMW. If this is so, then I don't see the reasons for concern though I don't insist.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 2, 2018

Could you remove the 'controllers' subdirectory? I think it's confusing that there is such subdirectory in the source repo, but none in the build or install folder.

ghost commented Jan 2, 2018

Could you remove the 'controllers' subdirectory? I think it's confusing that there is such subdirectory in the source repo, but none in the build or install folder.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 2, 2018

I don't have a controller, can anyone test theirs still work in this PR?

ghost commented Jan 2, 2018

I don't have a controller, can anyone test theirs still work in this PR?

@Thunderforge

This comment has been minimized.

Show comment
Hide comment
@Thunderforge

Thunderforge Jan 3, 2018

Contributor

Could you remove the 'controllers' subdirectory? I think it's confusing that there is such subdirectory in the source repo, but none in the build or install folder.

Done. One advantage of removing the controllers directory like this is that when we eventually drop SDL 2.0.4 and 2.0.5 support, we'll be back down to one file, and then a controllers subdirectory wouldn't make sense for just one file.

I don't have a controller, can anyone test theirs still work in this PR?

If you wanted a controller to test stuff like this with, I would seriously buy one for you :-)

Contributor

Thunderforge commented Jan 3, 2018

Could you remove the 'controllers' subdirectory? I think it's confusing that there is such subdirectory in the source repo, but none in the build or install folder.

Done. One advantage of removing the controllers directory like this is that when we eventually drop SDL 2.0.4 and 2.0.5 support, we'll be back down to one file, and then a controllers subdirectory wouldn't make sense for just one file.

I don't have a controller, can anyone test theirs still work in this PR?

If you wanted a controller to test stuff like this with, I would seriously buy one for you :-)

@psi29a

This comment has been minimized.

Show comment
Hide comment
@psi29a

psi29a Jan 3, 2018

Member

The Steam Controller is pretty slick, or so I've heard.

Member

psi29a commented Jan 3, 2018

The Steam Controller is pretty slick, or so I've heard.

@AnyOldName3

This comment has been minimized.

Show comment
Hide comment
@AnyOldName3

AnyOldName3 Jan 8, 2018

Member

It depends. It's great sometimes and is worse than a regular XInput one in other situations. In most of the situations where it beats a standard XInput controller, a mouse would beat the steam controller. There's also the problem of the massive rigmarole to set it up per application through Steam. All that said, it's my default controller when I decide to use a controller. Oh, also, it sips batteries so slowly that it makes my other wireless controllers look terrible.

Member

AnyOldName3 commented Jan 8, 2018

It depends. It's great sometimes and is worse than a regular XInput one in other situations. In most of the situations where it beats a standard XInput controller, a mouse would beat the steam controller. There's also the problem of the massive rigmarole to set it up per application through Steam. All that said, it's my default controller when I decide to use a controller. Oh, also, it sips batteries so slowly that it makes my other wireless controllers look terrible.

@Thunderforge

This comment has been minimized.

Show comment
Hide comment
@Thunderforge

Thunderforge Jan 8, 2018

Contributor

@scrawl I posted this topic on the OpenMW forums requesting testers. One person reports that their DualShock 4 works with this PR.

Contributor

Thunderforge commented Jan 8, 2018

@scrawl I posted this topic on the OpenMW forums requesting testers. One person reports that their DualShock 4 works with this PR.

@ghost ghost merged commit bd072b1 into OpenMW:master Jan 9, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

ghost pushed a commit that referenced this pull request Jan 9, 2018

scrawl

This issue was closed.

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