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

ButtonManager: Fix handling of empty device id. #8420

Open
wants to merge 1 commit into
base: master
from

Conversation

@phcoder
Copy link

phcoder commented Oct 21, 2019

Device id is "" on ChromeOS when using gamepad

Copy link
Contributor

CookiePLMonster left a comment

I can see formatting is broken so lint will fail, but I'll run the build regardless to check if it compiles fine.

Source/Android/jni/ButtonManager.cpp Outdated Show resolved Hide resolved
}
else if (std::string::npos != value.find("Button"))
{
hasbind = true;
type = BIND_BUTTON;
sscanf(value.c_str(), "Device '%127[^\']'-Button %d", dev, &bindnum);
if (std::string::npos != value.find("Device ''"))

This comment has been minimized.

Copy link
@CookiePLMonster
Device id is "" on ChromeOS when using gamepad
@phcoder phcoder force-pushed the phcoder:devid branch from 3191fe1 to a905794 Oct 21, 2019
@@ -586,13 +586,19 @@ void Init(const std::string& gameId)
{
hasbind = true;
type = BIND_AXIS;
sscanf(value.c_str(), "Device '%127[^\']'-Axis %d%c", dev, &bindnum, &modifier);
if (std::string::npos != value.find("Device ''"))

This comment has been minimized.

Copy link
@JosJuice

JosJuice Oct 21, 2019

Contributor

Is there any particular reason why you're using find instead of StringBeginsWith (available in Common/StringUtil.h)?

This comment has been minimized.

Copy link
@phcoder

phcoder Oct 21, 2019

Author

Changed. I will have to retest the patch

Copy link
Member

jordan-woyak left a comment

This solution seems odd. I'd have to dig deeper into the code but I feel like it would be cleaner to just overwrite an empty device name with "Device" (or anything) where the device is created.

@phcoder

This comment has been minimized.

Copy link
Author

phcoder commented Oct 21, 2019

This solution seems odd. I'd have to dig deeper into the code but I feel like it would be cleaner to just overwrite an empty device name with "Device" (or anything) where the device is created.

I see it as a problem with sscanf and just fixing it as near as possible to the cause. We can't change sscanf but we can add this fix on top of it.
But I don't really care which solution to use, I just want Dolphin to work well on Chromebook

@leoetlino leoetlino added the WIP label Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.