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

Android: Reimplement input binding #4410

Merged
merged 2 commits into from Nov 27, 2016
Merged

Android: Reimplement input binding #4410

merged 2 commits into from Nov 27, 2016

Conversation

Hydr8gon
Copy link
Member

@Hydr8gon Hydr8gon commented Oct 30, 2016

I fixed up the old stuff to make it work with the new settings UI. Functionally, everything is as it was before, plus a new clear button to remove bindings. I changed the strings to match the desktop UI, which was also convenient because it lead to less strings. :)

Here's a screenshot just because:
screenshot oct 30 2016 7-54-09 pm
(^ When you map a real Wiimote to an emulated one because you don't have a DolphinBar :P)


This change is Reviewable


sl.add(new CheckBoxSetting(SettingsFile.KEY_GCADAPTER_RUMBLE + gcPadNumber, SettingsFile.SECTION_CORE, R.string.gc_adapter_rumble, R.string.gc_adapter_rumble_description, false, rumble));
sl.add(new CheckBoxSetting(SettingsFile.KEY_GCADAPTER_BONGOS + gcPadNumber, SettingsFile.SECTION_CORE, R.string.gc_adapter_bongos, R.string.gc_adapter_bongos_description, false, bongos));
private void addExtensionSubSettings(ArrayList<SettingsItem> sl, int wiimoteNumber, int extentionType)

This comment was marked as off-topic.

@degasus
Copy link
Member

degasus commented Oct 31, 2016

@sigmabeta Do you want to help me with reviewing?

@sigmabeta
Copy link
Contributor

I can take a look later tonight.

SettingsFile.SECTION_BINDINGS, SettingsFile.KEY_WIIMOTE_EXTENSION + i, "0"));
String extension;

switch (extensionValue)

This comment was marked as off-topic.

{
Setting rumble = mSettings.get(SettingsFile.SECTION_CORE).getSetting(SettingsFile.KEY_GCADAPTER_RUMBLE + gcPadNumber);
Setting bongos = mSettings.get(SettingsFile.SECTION_CORE).getSetting(SettingsFile.KEY_GCADAPTER_BONGOS + gcPadNumber);
Setting extension = mSettings.get(SettingsFile.SECTION_BINDINGS).getSetting(SettingsFile.KEY_WIIMOTE_EXTENSION + (wiimoteNumber - 3));

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

Setting bindDPadLeft = mSettings.get(SettingsFile.SECTION_BINDINGS).getSetting(SettingsFile.KEY_WIIBIND_DPAD_LEFT + wiimoteNumber);
Setting bindDPadRight = mSettings.get(SettingsFile.SECTION_BINDINGS).getSetting(SettingsFile.KEY_WIIBIND_DPAD_RIGHT + wiimoteNumber);

sl.add(new SingleChoiceSetting(SettingsFile.KEY_WIIMOTE_EXTENSION + (wiimoteNumber - 3), SettingsFile.SECTION_BINDINGS, R.string.wiimote_extensions, R.string.wiimote_extensions_descrip, R.array.wiimoteExtensionsEntries, R.array.wiimoteExtensionsValues, 0, extension));

This comment was marked as off-topic.

@Hydr8gon
Copy link
Member Author

Rebased and removed the hacky stuff, everything should be working fine now.

@Savbyn
Copy link

Savbyn commented Nov 26, 2016

I have discovered a problem. If you bind the controls the first time everything works, but if you bind them again you have to bind every control again or the controls that you did not rebind do not function. There should also be a way to unbind the controls with a controller because currently you either need a mouse or a touchscreen to do so. A good way to accomplish this would be for the user to hold down the selection to unbind. Thanks for your hard work on dolphin!

@MayImilae
Copy link
Contributor

@SeannyM We really need more for the progress report! Does it need review? Is this ready for merging?

@Hydr8gon
Copy link
Member Author

@Brotanium good catch, I fixed the issue. Regarding unbinding via controller, it's something I can look into sometime in the future; at some point I'd like to design a better UI for this anyways, because right now using the same UI as the rest of the settings means there is a lot of scrolling.

@MaJoRoesch if nobody else has any issues with this, it's good to go.

@Helios747
Copy link
Contributor

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/dialogs/MotionAlertDialog.java, line 26 at r2 (raw file):

{
	// The selected input preference
	private final InputBindingSetting item;

Can we name this something else? 'item' isn't particularly descriptive of what this is for even if the code it's used in is simple.


Comments from Reviewable

@Hydr8gon
Copy link
Member Author

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/dialogs/MotionAlertDialog.java, line 26 at r2 (raw file):

Previously, Helios747 (Anthony) wrote… > Can we name this something else? 'item' isn't particularly descriptive of what this is for even if the code it's used in is simple.
I agree "item" isn't a very fitting name for this, I only named it that because they are called "item" in onClickListeners. Though I guess it's more fitting in that situation, because the setting is the item you are clicking on. I'll change it.

Comments from Reviewable

Also update MotionAlertDialog to work with the new setting, and remove the
old InputBindingPreference.
@Helios747
Copy link
Contributor

:lgtm:


Reviewed 3 of 16 files at r1, 11 of 12 files at r2, 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


Comments from Reviewable

@Helios747 Helios747 merged commit 6c275d4 into dolphin-emu:master Nov 27, 2016
@MayImilae
Copy link
Contributor

MayImilae commented Nov 27, 2016

@SeannyM Tested. I ran into a critical issue. The controller presses are still being picked up by the UI even when in game! When I press B on the controller in order to press a button in game, it backs out of the game and back to the UI. I can't use my controller at all!

Device - LETV phone that runs super slowly but without errors
Controller - 8bitdo bluetooth controller

@MayImilae
Copy link
Contributor

Aaaand it's already merged. Ok, well. One last comment. This is probably outside the scope of this PR, but there isn't any way to put in L/R analog. Many games use them as separate buttons (rogue leader, super mario sunshine), or require pressing analog before digital will be accepted (metroid prime). So it's important for it to have mappable analog L/R separate from the digital L/R.

@Hydr8gon
Copy link
Member Author

Hydr8gon commented Nov 27, 2016

@MaJoRoesch regarding the UI input issue, I can't reproduce it on my end. Can you check to make sure the bindings are saved? IIRC, someone else had the same issue and it turned out they were using the 'X' to close the settings, which actually exits without saving. To save the settings, you have to back out of the settings fully without using the 'X'. If this isn't the problem, my only guess would be maybe focus isn't being given to the emulation activity?

@MayImilae
Copy link
Contributor

MayImilae commented Nov 27, 2016

I went back to check at the time, and it was still bound. And I just rechecked it, and the settings were all still there, and it said the settings were saved to ini. And I literally turned on the phone, and ran Dolphin, so there is absolutely nothing else going on!

EDIT: Me and HdkR did some testing, and it seems to be the 8bitdo controller. Apparently it's sending the B button and the back command at the same time. So the configuration detects the button, and the emulator detects the back command. Yuck.

@Hydr8gon
Copy link
Member Author

Perhaps we should prevent the back button from exiting emulation, and instead put an exit button in the dropdown menu? Or at least an option to disable the back button. This would also help prevent accidental exits.

@sigmabeta
Copy link
Contributor

Make sure to only do this on touchscreen devices though.

@Savbyn
Copy link

Savbyn commented Nov 27, 2016

So I tested again to see if I could get multiplayer working and the unbinding problem persists making multiplayer impossible since it unbinds when setting another controller. I then tried to rebind a single controller and all the other controls unbinding when done. Another thing to note a new Panic Alert occurs now which says "Panic Alert: IOS HLE: Couldn't get an absolute path; the root directory will be returned. This will most likely lead to failures.". I am not sure if this is related to anything done but I was just wondering. Thanks.

@JosJuice
Copy link
Member

@Brotanium I broke IOS HLE in 5.0-1339 except on Windows, so many Wii games don't work right now. A fix is in progress.

@Hydr8gon
Copy link
Member Author

@Brotanium I just tested, and I can map multiple controllers at once without a problem. I'm not sure why it wouldn't be working for you. You're getting the toast message saying the settings are saved, right? It should show up after you leave the settings and return to the game list.

@Savbyn
Copy link

Savbyn commented Nov 27, 2016 via email

@Hydr8gon Hydr8gon deleted the android-inputbind branch January 10, 2017 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
9 participants