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 DualShockUDP not adding/removing devices correctly #9418

Merged
merged 1 commit into from Jan 5, 2021

Conversation

Filoppi
Copy link
Contributor

@Filoppi Filoppi commented Jan 2, 2021

-If adding 2 devices with the same name, their unique identifier wouldn't be increased, causing a conflict.
-Removing a device wouldn't actually remove it from the internal devices list because the local list of devices had already been updated when iterating it.
-It was possible to remove devices belonging to other input sources by adding a device with the same name and then removing it.

@iwubcode

This comment has been minimized.

@Filoppi
Copy link
Contributor Author

Filoppi commented Jan 2, 2021

Huh, didn't ever imagine someone would use a duplicate device name. That's certainly not intended and probably should be enforced. But these changes look good.

I thought about enforcing it but that's not necessary given that two devices can have the exact same name under the same source, they will be distinguished by ID.

@iwubcode

This comment has been minimized.

@Filoppi
Copy link
Contributor Author

Filoppi commented Jan 2, 2021

I thought about enforcing it but that's not necessary given that two devices can have the exact same name under the same source, they will be distinguished by ID.

Yeah, not a problem from a software standpoint, more from a user standpoint. Imagine you have two devices named the same thing and come back later. How do you distinguish them? I guess you have to look at the order.

More importantly, if I'm remembering right, profiles are based on device name. I wonder what would happen if it was applied. Maybe both would get it?

But like I said, it was never intended in my design, just an oversight :(

Well of course the first thing I tried to do was adding two devices with the same name to break it :).
Profiles seem to save the whole device path (e.g. DInput/0/Wireless Controller), when restoring it, if the device is not present, it will just add it back to the list and mark it as disconnected, all you got to do now is select a new default device (of the same type) and it would work, so not really a problem.
If I want to have two devices with the same name, it's my problem really, especially because if you attach two Xbox controllers, they will also have the same name, but a different ID.
I've got a PR incoming to make the input editing clearer to users, as you need a PhD to understand it as of now.

@iwubcode
Copy link
Contributor

iwubcode commented Jan 2, 2021

@Filoppi - sorry, I had just woken up when I looked at this initially and was thinking of the path not the actual device name. You're right, there's no reason we can't allow the same name.

This LGTM code wise (I'll test it later)

@AdmiralCurtiss
Copy link
Contributor

Commits still need to be squashed.

@Filoppi
Copy link
Contributor Author

Filoppi commented Jan 3, 2021

Commits still need to be squashed.

Fixed

-If adding 2 devices with the same name, they their unique id wouldn't be increased, causing a conflict.
-Removing a device wouldn't actually remove it from the internal devices list because the list of devices had already been updated when going through it.
-It was possible to remove devices belonging to other sources by adding a device with the same name and then removing it.
@Filoppi
Copy link
Contributor Author

Filoppi commented Jan 3, 2021

Fixed lint code check

@leoetlino leoetlino merged commit ee25f03 into dolphin-emu:master Jan 5, 2021
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants