lmms
lmms copied to clipboard
Widget highlighting
This PR contains multiple changes and overall it is focused on improving lmms functionality communication with the user and on immediate feedback to the user.
These changes are the following:
- The
StringPairDragsystem and theStringPairsystem now uses a unifiedenumas key instead of custom strings. Before this, key names weren't documented and they were scattered across multiple files. - A new class was added that implements handling ~~shortcuts~~ and highlighting widgets:
- This class was designed to encourage developers to implement ~~shortcuts~~ and pasting data into the widget.
- Highlighting:
- The idea behind highlighting is to show users which widget can accept which
StringPairkey while dragging or copying. If the user starts to drag a clip, the other clips will be highlighted to show that they can accept the datatype associated with the drag event. - Highlighting will be active for 10 seconds after the user starts to ctrl + drag a widget or when a widget is copied.
- Highlighting will stop after a successful paste, but the user can keep pasting into other widgets.
- The idea behind highlighting is to show users which widget can accept which
- The new class is implemented inside Knobs (
FloatModelEditorBase) and all clips. I plan to implement it in other widgets in the future (if this gets merged).
How to test:
- Hover over a knob, hold ctrl and start dragging.
- Other knobs and automation clips should get highlighted meaning that they accept the data type that is being dragged.
- Drop the knob over an other knob. They should now be linked. The highlighting should stop.
- Repeat this with automation clips, midi clips, sample clips and pattern clips.
- After dropping a clip on an other clip, one should be the copy of the other.
Problems:
- Linking sometimes causes a crash (this is probably a bug in the linking code), spam the ctrl + c and ctrl + v and unlink shortcuts on knobs to trigger this.
ClipView::getClipStringPairTypeandTrackView::getTrackStringPairTypedoes not implement Special types of tracks (video, event)- I removed
ClipView::dropEventcopying code because I found that this condition is never true:
// Defer to rubberband paste if we're in that mode
if (m_trackView->trackContainerView()->allowRubberband() == true)
Picture showing highlighting (for knobs and clips separately):
So, overall:
- shortcut system is fine, not absolutely perfect and across entirety of lmms, but much better than having nothing
- linked controls, when user gets an idea on how to use it, is actually well done and useful. Just don't link a lot of controls, as disconnecting one by one can be tedious.
- new automation pattern drag & drop behaviour, while changes few things in workflow, is much less buggy and had properly working undo, comparing to old method
It doesn't cause major issues with lmms in my case of using it daily for around two - three weeks.
the volume of the midi track goes down without an active automation track.
I'm sorry, but I'm unsure if I see where this occurs. I don't see knobs moving and in the clip, both times the volume stays the same.
the volume of the midi track goes down without an active automation track.
I'm sorry, but I'm unsure if I see where this occurs. I don't see knobs moving and in the clip, both times the volume stays the same.
I looked elsewhere and nothing is automating the volume to make it go down.
I looked elsewhere and nothing is automating the volume to make it go down.
I don't see where this happens, could you point me to the right direction?
I looked elsewhere and nothing is automating the volume to make it go down.
I don't see where this happens, could you point me to the right direction?
The issue is in the envelope tab in the instrument ui. when you turn the amt to the right, the audio bug happens. But if you don't the audio bug is not there.
The issue is in the envelope tab in the instrument ui. when you turn the amt to the right, the audio bug happens. But if you don't the audio bug is not there.
Please could you show a screenshot of what happens and a description of what happens and a description of how it should behave? And a description of how to recreate it. I'm sorry, but I don't know what you mean by "audio bug", since I didn't touch the audio code. I believe you are trying to show that a knob moves without any automation channel attached, but I can't see this in the video you showed. I tried to reproduce the video and I believe everything worked as expected.
Please could you describe the issue you are experiencing in detail?
And please could you share your opinion about the actual features this PR brings? @Spacemagehq I would appreciate it if I got feedback on how the shortcut system could be changed and how this PR could be improved, or at least it would be useful if you confirmed that everything else worked as it should.
With this pr there's an audio bug on Windows 11 when you go to the volume tab in the build in instruments when you move the amt to the right the volume of the midi track goes down without an active automation track. I tested this with Nescaline as well, not just with TripleOscillator.
As far as I can tell, this is not a bug. Your envelope has a bit of hold, with the sustain at about half. So, after the hold time is up, the note will go down to the sustain volume of 0.5. If you don't want you note to decay, turn the sustain knob to 1. I don't believe this is relevant to this PR.
With this pr there's an audio bug on Windows 11 when you go to the volume tab in the build in instruments when you move the amt to the right the volume of the midi track goes down without an active automation track. I tested this with Nescaline as well, not just with TripleOscillator.
As far as I can tell, this is not a bug. Your envelope has a bit of hold, with the sustain at about half. So, after the hold time is up, the note will go down to the sustain volume of 0.5. If you don't want you note to decay, turn the sustain knob to 1. I don't believe this is relevant to this PR.
I looked at it again the nightly and the pr they both set to default values on both the pr and the nightly lmms the nightly doesn't have this audio issue. The issue can be user error and my fault, if it is, I just delete the video from the github.
And please could you share your opinion about the actual features this PR brings? @Spacemagehq I would appreciate it if I got feedback on how the shortcut system could be changed and how this PR could be improved, or at least it would be useful if you confirmed that everything else worked as it should.
The pup up hotkey and the highlights I had no issues with. This might be a useless thing to add, but maybe the ability to change the highlight color in the future ?
Fixed the audio issue turns out it was something on my end didn't have a setting set correctly, both Golden Plunger on discord and regulus give me the solution. Deleting the video that is on here.
I just briefly tested this; maybe the build I'm using wasn't from the best commit, but it seems that the drag and drop connections/copying clips into other clips isn't working.
https://github.com/user-attachments/assets/0f250705-c1da-46de-a29d-722d42f53574
Also, this is just my personal opinion, but I feel like it would make more sense for the highlighting to disappear right after you release ctrl-drag. Idk, it just kind of feels like it's just staying there even after you finish the drag, and it takes a while for it to disappear.
Oh and also, this is probably unrelated, but how does linking a widget work with "Ctrl-C (2x)"? When I do Ctrl-C twice, it says "Link Widget" but I'm not sure what I'm supposed to do after that.
I just briefly tested this; maybe the build I'm using wasn't from the best commit, but it seems that the drag and drop connections/copying clips into other clips isn't working.
Maybe I'm doing something wrong but everything is working for me on my build and in the linux build from github. What you showed in the video seems wrong. The highlighting behaves wrongly. Here is how it should work:
- you start dragging or pressing ctrl + c over a widget
- widgets that accept that datatype will be highlighted
- you drop a widget on an other or press ctrl + v over on an other widget
- highlighting is stopped, the datatype is pasted into the widget
- ctrl + v still works on other widgets
Also, this is just my personal opinion, but I feel like it would make more sense for the highlighting to disappear right after you release ctrl-drag. Idk, it just kind of feels like it's just staying there even after you finish the drag, and it takes a while for it to disappear.
It should disappear if something accepts it. Usually users start dragging something with the intent of dropping it somewhere, so this should not be a problem for 90% use cases. Maybe It would be better if it would disappear after the drag event is destroyed, but it is an edge case. It will disappear automatically after 10 seconds anyways.
Oh and also, this is probably unrelated, but how does linking a widget work with "Ctrl-C (2x)"? When I do Ctrl-C twice, it says "Link Widget" but I'm not sure what I'm supposed to do after that.
It is really simple: everything is highlighted that accepts that datatype. You can press ctrl + v on any highlighted widget such as an other Knob or an automation clip.
I did some quick testing... Here are some initial thoughts...
This feature seems to aim to be three things:
- Refactor string-pair drag
- Add a keyboard shortcut system to commonly used GUI controls
- Add a highlighting system (although I'm not entirely sure what the highlighting is intended for, more below)
My initial glance is that we could greatly benefit from the first two items however I'm uncertain as to the specific implementation. I'll elaborate a bit:
- Copy/Paste should be a small visual indicator of the source and target (perhaps a brief highlight informing the user of the widget that a value was taken from or applied to). This PRs current mechanism highlights a lot of widgets and I'm not sure what value that adds to the user experience.
- Linking to an automation pattern should be as simple as looking at what type of widget value was copied and setting up the link on paste. This mimics the fact that we re-use Ctrl for copying and for linking (Well, macOS uses Alt since #2890, but I think "Command + C" is still intuitive here, no changes needed).
- If the item being pasted to isn't an automation pattern, require an additional modifier key (e.g. Ctrl + Shift + V).
- Why doesn't "Unlink" show/work for Automation Patterns? (Is this a one-to-many problem? Maybe we display an error if there's' more than one?)
This leaves a lot of questions:
- Did this PR remove the hints about dragging automatable items?
- What reason were E, Q chosen for increasing/decreasing?
- e.g. Would it make sense to introduce the shortcut functionality first, then individual PRs that help settle on the best shortcuts so we're not reviewing so much at once?
- Is there a way to deselect a widget once it's been highlighted? The only way I could figure out was to paste, which doesn't really make sense
- How come sometimes a linked automation segment highlights too? Is this to display what's linked? If that's the case, why do unrelated things also get highlighted?
- Why does this PR re-use the help text area when an action is performed? This is especially frustrating during the learning process because the help text disappears the moment a shortcut is pressed, making it harder to learn.
- Why do all the volume and pan knobs highlight the same color when I hit Ctrl + C?
- What gets "copied" when my mouse isn't hovering over anything?
- Why doesn't things like Copy/Paste work on the mixer sliders, track labels, etc?
In general, I think I agree with some of @sakertooth's sentiments on Discord that this PR may need to be broken up into digestible parts so that we're not trying to review too much at once. If this is done, I would recommend that this PR is kept open and converted to draft so that it can be referenced until all desired features have open (or -- if blocking -- merged) PRs.
@szeli1 In the future could you please merge master into your branches rather than rebasing on master? Once you open a PR and people start reviewing it, rebasing makes it difficult for reviewers to see what changed.