Fix sooperlooper per-loop bindings #511
Conversation
Added ctrl:loop controllers for all per-loop bindable controllers (as
previously defined, there may benefit more).
But also simple 'ctrl' controllers to always handle the selected loop.
The mode of the binding to be made is still changed with
selected_loop_cc_binding (on the last page of controls).
It is indicated on per-loop bindable controls with (loop number) added
to their name.
This is updated whenever the loop selection is changed or when
selected_loop_cc_binding is set to True.
The earlier implementation was incorrect in multiple aspects:
1. It bound ctrl:loopnumbers only, and it only worked when the loop was
also already the selected one.
This check is now no longer made.
2. Checks for current state checked the selected loop, instead of the
targeted loop. The targeted loop (chan) is now set and checked.
2. Selecting a loop with another method than going to the loop
selection page and back to another controller page did not work
because the UI did not refresh the actual controllers.
Fixed this by calling
self.state_manager.send_cuia("refresh_screen", ["control"])
(I did not find a more direct way)
| return processor.controllers_dict | ||
|
|
||
| def adjust_controller_bindings(self): |
There was a problem hiding this comment.
I am not sure we should be adding more UI specific code into this backend engine class. We have been seperating UI from backend code. The ctrl screens present a challenge so have not yet been worked on.
There was a problem hiding this comment.
It is effectively the same as from the old code from line 849—854, with the added condition to set the non-loop-bound controller when selected_loop_cc_binding was True. Since this code has to run in two occasions (when toggling the option and when changing the selected loop), I turned it into a method.
| except IndexError: | ||
| return | ||
| processor.refresh_controllers() | ||
| self.state_manager.send_cuia("refresh_screen", ["control"]) |
There was a problem hiding this comment.
This is a UI specific call but, maybe this is a reasonable way to handle the challenge of backend changes impacting UI. I wonder whether the selected_loop_cc_binding code might be able to move to a UI class, triggered by this event?
There was a problem hiding this comment.
In my opinion, it is more of a UI change than a back-end change: something effecting what controllers are shown and, more importantly, what to bind to.
Without this, you have to manually refresh the screen.
There was a problem hiding this comment.
Thinking about this some more, I actually do not exactly follow what you mean. I do not see an event being fired, and I would not know how to add this logic to a new UI class.
The screen refresh is necessary for the new binding logic to work.
Something that probably went unnoticed before single_pedal mode was added to the per-loop binding (which is on the same screen as the loop selectors). For the other controls, you had to switch screens to select a loop and then get back to the screen with the desired controller. Which already does the desired refreshing, and thus the proper binding.
Adding the loop number to the visible name is just frills, and more of a result of me trying to get feedback while trying to get the binding logic right.
I'd really like to get this merged in in some way, so what's the way forward now?
Besides ctrl:loop controllers for all per-loop bindable controllers (as previously defined, there may benefit more), also add simple, unnumbered, 'ctrl' controllers to always handle the selected loop.
The mode of the binding to be made is still changed with selected_loop_cc_binding (on the last page of controls).
It is indicated on per-loop bindable controls with (loop number) added to their name, e.g. 'record (3)'
This is updated whenever the loop selection is changed or when selected_loop_cc_binding is set to True.
The earlier implementation was incorrect in multiple aspects:
This check is now no longer made.
Fixed this by calling
self.state_manager.send_cuia("refresh_screen", ["control"])(I did not find a more direct way)
I noticed I inadvertently added IndexError, in order to not have a naked except (so vscode told me). Revert if this is an incorrect assumption.