Enhancements for Dialog Navigation/Controls

Lolonois

The dialog navigation could be more intuitive IMHO. I started to modify some things and looking for feedback before optionally filing an PR. Coincidently some other user proposed some changes already. But the efforts are not related.

For my changes:

Streamline the button functions with ES functions (mostly) and in between the scripts runcommand.sh and helpers.sh (A=Enter, B=Tab,X=Space ,Y=Esc ,TL=PageUp ,TR=PageUp).
Making Escape key a one function key, by reducing the timeout ncurses waits for a Escape+<key> combination.
Quitting the runcommand.sh with Esc
Repeating "Cursor events" when a joystick is pushed and held in one position.

While writing this I am considering if the B-Button in the dialogs should not be mapped to Escape (which would match ES's back function).

Thoughts on this / in general?

The proposed changes:
helpers.sh:

--- /home/pi/git/RetroPie-Setup/scriptmodules/helpers.sh	2020-07-12 12:13:08.936792361 +0200
+++ /home/pi/RetroPie-Setup/scriptmodules/helpers.sh	2020-07-12 14:37:39.872418698 +0200
@@ -1078,7 +1078,7 @@
 
     local params=("$@")
     if [[ "${#params[@]}" -eq 0 ]]; then
-        params=(kcub1 kcuf1 kcuu1 kcud1 0x0a 0x20 0x1b)
+        params=(kcub1 kcuf1 kcuu1 kcud1 0x0a 0x09 0x1b 0x20 kpp knp)
     fi
 
     # get the first joystick device (if not already set)

runcommand.sh:

--- /home/pi/git/RetroPie-Setup/scriptmodules/supplementary/runcommand/runcommand.sh	2020-07-12 12:13:08.976792361 +0200
+++ /opt/retropie/supplementary/runcommand/runcommand.sh	2020-07-12 14:35:50.038259127 +0200
@@ -132,7 +132,7 @@
 
         # call joy2key.py: arguments are curses capability names or hex values starting with '0x'
         # see: http://pubs.opengroup.org/onlinepubs/7908799/xcurses/terminfo.html
-        "$ROOTDIR/supplementary/runcommand/joy2key.py" "$JOY2KEY_DEV" kcub1 kcuf1 kcuu1 kcud1 0x0a 0x09
+        "$ROOTDIR/supplementary/runcommand/joy2key.py" "$JOY2KEY_DEV" kcub1 kcuf1 kcuu1 kcud1 0x0a 0x09 0x1b 0x20 kpp knp
         JOY2KEY_PID=$(pgrep -f joy2key.py)
 
     # ensure coherency between on-screen prompts and actual button mapping functionality
@@ -583,6 +583,8 @@
     local cmd
     local choice
 
+    export ESCDELAY=25
+
     local user_menu=0
     [[ -d "$CONFIGDIR/all/runcommand-menu" && -n "$(find "$CONFIGDIR/all/runcommand-menu" -maxdepth 1 -name "*.sh")" ]] && user_menu=1
 
@@ -650,6 +652,10 @@
         fi
         cmd=(dialog --nocancel --menu "System: $SYSTEM\nEmulator: $EMULATOR\nVideo Mode: $temp_mode\nROM: $ROM_BN"  22 76 16 )
         choice=$("${cmd[@]}" "${options[@]}" 2>&1 >/dev/tty)
+        if [[ $choice = '' ]]; then
+            # quit main menu with ESC
+            return 1
+        fi
         case "$choice" in
             1)
                 choose_emulator "emu_sys" "$emu_sys"

joy2key.py:

--- /home/pi/git/RetroPie-Setup/scriptmodules/supplementary/runcommand/joy2key.py	2020-07-12 12:13:08.976792361 +0200
+++ /opt/retropie/supplementary/runcommand/joy2key.py	2020-07-12 15:18:12.048721128 +0200
@@ -92,8 +92,8 @@
     if not js_cfg:
         js_cfg = RETROARCH_CFG
 
-    # getting configs for dpad, buttons A, B, X and Y
-    btn_map = [ 'left', 'right', 'up', 'down', 'a', 'b', 'x', 'y' ]
+    # getting configs for dpad, buttons A, B, X, Y, TL and TR
+    btn_map = [ 'left', 'right', 'up', 'down', 'a', 'b', 'x', 'y', 'l', 'r' ]
     btn_num = {}
     biggest_num = 0
     i = 0
@@ -192,7 +192,7 @@
 
     # ignore init events
     if js_type & JS_EVENT_INIT:
-        return False
+        return 0
 
     hex_chars = ""
 
@@ -215,9 +215,9 @@
     if hex_chars:
         for c in hex_chars:
             fcntl.ioctl(tty_fd, termios.TIOCSTI, c)
-        return True
+        return js_type
 
-    return False
+    return 0
 
 js_fds = []
 tty_fd = []
@@ -263,6 +263,7 @@
             i = 0
             current = time.time()
             js_last = [None] * len(js_fds)
+            js_prev_evt = [None] * len(js_fds)
             for js in js_fds:
                 js_last[i] = current
                 i += 1
@@ -279,12 +280,24 @@
                         button_codes = js_button_codes[fd]
                     else:
                         button_codes = default_button_codes
-                    if process_event(event):
+                    ev_type = process_event(event)
+                    if ev_type:
                         js_last[i] = time.time()
+                        if ev_type == JS_EVENT_AXIS:
+                            # keep axis event
+                            js_prev_evt[i] = event
+                    else:
+                        js_prev_evt[i] = None
+                else:
+                    js_prev_evt[i] = None
             elif event == False:
                 close_fds(js_fds)
                 js_fds = []
                 break
+            elif js_prev_evt[i] and time.time() - js_last[i] > JS_REP:
+                # no real event happened, replay last axis event
+                _ = process_event(js_prev_evt[i])
+                time.sleep(JS_REP)
             i += 1
 
     if time.time() - rescan_time > 2:

PS: Nice, the forum supports patch highlighting :-)

```patch 
<code here>
```

mitu

@Lolonois said in Enhancements for Dialog Navigation/Controls:

Thoughts on this / in general?

I wouldn't introduce any key combo in the dialog boxes, they're meant to be operated simply with a button (any button).
The repeat function would be useful though - one thing that's bugging me currently is scrolling in large lists (hello ESThemes!), which right now is slow. Having a working 'repeat on key/button press' or mapping TR/TL to scroll (PgUp/PgDn) events should speed things up considerably.

keith.f.kelly

@Lolonois Note that 0x0a is not the correct hex code to use for ENTER key; all it does is send a newline, which isn't the same thing. Per my PR, I believe 0x0d is the one you want.

I've puzzled for a while over why it takes the gamepad's ESCAPE key longer to back out of menus -- nice find with the ESCDELAY thing. I can add that to my PR.

I don't see why any button needs to be mapped to the TAB key. TAB doesn't appear to be used anywhere in the menus. Am I missing something?

Overall, yeah, it seems like we've both independently desired the same class of improvement here... when multiple users are independently asking for the same thing, that should be a sign that there's a real problem.

keith.f.kelly

@mitu - The proposed changes do not introduce any "key combos". What are you talking about?

You can see the explanation for the key repeat problem in my other PR here: https://github.com/RetroPie/RetroPie-Setup/pull/3196

keith.f.kelly

@keith-f-kelly said in Enhancements for Dialog Navigation/Controls:

You can see the explanation for the key repeat problem in my other PR here: https://github.com/RetroPie/RetroPie-Setup/pull/3196

@Lolonois - I think rather than wedging your repeat implementation into this already-convoluted code, it would be better to add what you're trying to do on top of PR 3196, as it provides a much cleaner and clearer framework on which to add your implementation. I can take a shot at adding that on top of what I've already done there based on your example, if you like. Ideally the repeat behavior would apply not just to the axes, but also to the L/R buttons (since they are to be used for navigation via pgup/pgdown).

Lolonois

@mitu Thanks. Enter and Space do have the same effect in the text dialog (=confirm something). The only addition would be the Escape (=Quit and return to ES) from the main menu in the runcommand. However, Escape is already in the mainline working in the runcommand submenus when using a physical keyboard and in the Retropie-Setup main dialog, so I assumed it would be consistent to allow quitting the runcommand back to ES with an mapped Escape key.
For the proposed Tab mapping in helpers.sh: Here it is the other way around: Tab is used in runcommand already (mainline) and is currently not in helpers.sh defined.

To sum up: If you have concerns about the Escape function I have no trouble withholding it. For the helpers.sh I can drop the proposed changes also since usually one has a physical keyboard attached when running sudo Retropie-Setup/retropie_setup.sh.

The motivation for the repeat function and page up/dn was fueled by the tedious input movements to get to the last / lower part of a menu list, at least in my setup as I am using classic arcade sticks. With a xbox like controller the input is less an annoyance I assume due to shorter travel paths for actuation.

Lolonois

@keith-f-kelly said in Enhancements for Dialog Navigation/Controls:

I don't see why any button needs to be mapped to the TAB key. TAB doesn't appear to be used anywhere in the menus. Am I missing something?

The physical key Tab is by default mapped to traverse the button selection (in the bottom area) in retropie's dialogs. More generally it is mapped to traverse the input fields/controls of the different widgets available in a dialog.

For 0x0a / 0x0d: I do not stick to 0x0a I just took what was there, maybe it is the cleanest to sent the terminfo's cr instead of 0x0a or 0x0d.