New context menu automation items

[szeli1]

closes #3112

This PR adds new context menu options for automatableModels to improve the automation workflow.
This PR aims to make quick edits to automation tracks easier and faster to do. It does not aim to replace the automation editor.

These options are the following:

• "Add automation node" - this option will search for AutomationTracks that have at least one clip. All clips have to be connected to the model. If a track with these requirements can not be found a new track will be added. After it found a track it will search for a clip before the playback position. If it can not find a clip it will add a new clip. After it found a clip it will add a new AutomationNode to the clip at the playback position. The node's value will be equal to the AutomatableModel's value. The node's placement depends on the quantization setting inside AutomationEditor. If there is a clip before the playback position a new clip won't be created.
• a new submenu is added labeled "Automations" inside the submenu there are more options:
  • "Add automation node to new clip" - mostly "Add automation node" option, but it adds a new clip at the playback position if possible before adding an automation node.
  • "Update closest automation node" - replaces the value of the closest node to the playback position with the AutomatableModel's current value.
  • "Remove closest automation node" - deletes the closest AutomationNode to the playback position. It will not delete clips and it will leave at least one AutomationNode inside a clip (because of AutomationClip limitations).
  • "Open automation clip" - mostly "Add automation node" option, but instead of adding a new AutomationNode it will open the found / created clip in the AutomationEditor. It will try to open the closest clip, it 2 clips are after each other and the playback position is (1/2 bar) close to the second clip then it will open the second clip.

In my view these options will make the automation system faster to use.

And I would like to say that this feature could be combined with #7488 shortcut system, so these context menu items could be activated by pressing a shortcut (while the cursor is hovering over a knob for example).

[Rossmaxx]

I see quite a few beginner mistakes here. Pointed out those. Nothing major, just bad practices. Also look into the codefactor blank line one.

[Rossmaxx]

Do you need that else there?

[Rossmaxx]

There's lots more nesting going on. I am a bit tired to nitpick everything

[szeli1]

There's lots more nesting going on. I am a bit tired to nitpick everything

Feel free to nitpick. I'm open to learn new things.

[Veratil]

First pass of changes that should be made.
EDIT: Looks like my page didn't refresh with the latest commits for some reason. Not sure why. 🤔

[szeli1]

EDIT: Looks like my page didn't refresh with the latest commits for some reason. Not sure why. 🤔

Because I pushed the changes right before when you completed your review.

[Rossmaxx]

More nitpicks.

[Rossmaxx]

Looks good to me.

[szeli1]

How should the new tracks be named? Currently it uses the default names.

[Rossmaxx]

Btw forgot to say, this could fall in scope for #7027

How should the new tracks be named? Currently it uses the default names.

I believe it should be named after whatever knob the automation is created from

[szeli1]

Please note that nowhere did I mention that I meant the current automation editor to be this first and preferred way. When I wrote this I had Bitwig's automation in mind. In Bitwig you can for example wiggle a control in a plugin and it will show as the current parameter in the automation track of that instrument track. You can then directly click points, move them, add tension to curves, etc. It just works straight forward and so far there never was a need to dive into context menus to accomplish something. Of course you can also manually select parameters or show all automation tracks of parameters that are automated.

Sorry I totally misunderstood you. I thought you said exactly the first sentence, sorry.
I'm not familiar with other daws sadly so I can't comment on that. I was thinking about widget highlighting and I think my next PR will be about that. I'm planning to implement a class similar to ModelView that would add widget highlighting and unified keyboard shortcuts.

[szeli1]

I have fixed undo, added a new context menu option to open the closest clip and removed the remove option removing clips (but now it must leave at least one node in the clip to prevent some issues).

[michaelgregorius]

Some things I have observed while checking the current version of the branch:

• If I use "Add automation node" and a new track with a new clip is added then undo only removes the clip and keeps the added track.
• There is still the remove node option in the menu that if I understood correctly was agreed on to be removed.
• "Open automation clip" does not open the nearest existing clip but instead adds a new one. If there is already one in that place another one will be added anyway and the clips will overlap, i.e. users might have several hidden clips which overlay each other.
• "Add automation node" only changes an existing node in a single clip for me so it does not really add new nodes in the automation clip. It's not clear to me what's the "rule" where a new node is added and what's governing this behavior.

To be honest, all these features seem totally confusing to me. I don't understand how it should be feasible and helpful to the users to remotely add nodes to automation clips that they might not even see while they are interacting with context menus in controls. I am not really "dog-fooding" on LMMS and seldom do something with the automations. So it should be a good test to see how easy or hard it is to understand this feature. And for me it is hard to understand.

Therefore I'd like some other @LMMS/developers to chime in here and take the decision with regards to whether this functionality should be added or not.

@szeli1, I propose to wait for the result of the discussion with the other developers before any further reviews are done.

[szeli1]

If I use "Add automation node" and a new track with a new clip is added then undo only removes the clip and keeps the added track.

I will fix this.

There is still the remove node option in the menu that if I understood correctly was agreed on to be removed.

I thought you only wanted the auto clip deletion removed (which was removed, now if all the nodes get deleted the clip will not be removed). I would like to hear what other devs say about this.

"Open automation clip" does not open the nearest existing clip but instead adds a new one. If there is already one in that place another one will be added anyway and the clips will overlap, i.e. users might have several hidden clips which overlay each other.
"Add automation node" only changes an existing node in a single clip for me so it does not really add new nodes in the automation clip. It's not clear to me what's the "rule" where a new node is added and what's governing this behavior.

Could you please describe to me exactly what are you trying to do and what do you expect the result to be? I don't know how could this happen and to me it seems like I did not give a good description of the features or there could be bugs.

Edit: I have rewrote the PR description to make the context menu options easier to understand.

[michaelgregorius]

"Open automation clip" does not open the nearest existing clip but instead adds a new one. If there is already one in that place another one will be added anyway and the clips will overlap, i.e. users might have several hidden clips which overlay each other.
"Add automation node" only changes an existing node in a single clip for me so it does not really add new nodes in the automation clip. It's not clear to me what's the "rule" where a new node is added and what's governing this behavior.

Could you please describe to me exactly what are you trying to do and what do you expect the result to be? I don't know how could this happen and to me it seems like I did not give a good description of the features or there could be bugs.

Reading the updated description I think it's a bit clearer to me what you want to achieve. I was not aware that the playhead plays an important role with regards to that feature. Nevertheless I will describe what I expected without have this description available as this will be the situation most users will be in when they interact with the feature.

Open automation clip

Steps to reproduce:

1. Right click the volume knob of a 3OSC and select "Add automation node" from the context menu.
2. Open the clip that was just added in the Automation Editor.
3. Make some changes, i.e. add some nodes, so that the clip becomes recognizable.
4. Close the Automation Editor.
5. Right click the volume knob again and select "Automations > Open automation clip".

Instead of opening the existing clip a new clip is added and opened. I'd expect it to open the existing clip because IMO it's more likely that I want to edit it instead of a completely new one with no edits.

Add automation node

Steps to reproduce:

1. Right click the volume knob of a 3OSC and select "Add automation node" from the context menu.
2. Open the clip that was just added in the Automation Editor.
3. Move the playhead in the Song-Editor so that it's still within the clip, e.g. in the middle of the clip.
4. Adjust the volume knob.
5. Select "Add automation node".
6. The opened clip will change.
7. Change the volume knob to a completely different value.
8. Select "Add automation node" again.

Instead of adding a new node like the label says the existing node is changed. I'd expect a node to be added.

[szeli1]

If I use "Add automation node" and a new track with a new clip is added then undo only removes the clip and keeps the added track.

Can not be fixed. You can not undo track creation.

[szeli1]

Instead of opening the existing clip a new clip is added and opened. I'd expect it to open the existing clip because IMO it's more likely that I want to edit it instead of a completely new one with no edits.

This is a bug which can be fixed. I will try to fix it.

Instead of adding a new node like the label says the existing node is changed. I'd expect a node to be added.

This is an AutomationClip limitation. You can not add multiple Nodes in the same location. I think this is the case inside the AutomationEditor also. A name change could fix this issue. (But what should be the name? I think it is reasonable to expect that the user understands that it is impossible to do what they want (especially if they know how the AutoamtionEditor works))

Thanks for that comment, it is in great detail (as I have requested).

I think documentation would need to be rewritten so the users will be able to understand the options better.

[Gabrielxd195]

I tried this and although I didn't have all the time to read your comments I would like to quickly contribute some ideas like:

1. Remove all automation clips with the "Remove Closet Automation Node" option: since removing a simple node from a clip, or removing the closest or first clip is useless. If I wanted to remove the value of a clip for that I go directly and do it, but if I need to remove all the clips related to the knob I want to automate, how do I do it? Currently this option is useless when I want to remove all the clips simultaneously.

2. Replace the current "Update Closets Automation Node" option with "Increase or Decrease the value of the clips": where there is the possibility of adding or subtracting values ​​to all the nodes of all the automation clips simultaneously. Basically it would be like the "Transpose" option of the instrument clips but applied to the automation clips. An automation clip can have many nodes with different values ​​but obviously a knob cannot. Currently updating doesn't work for many nodes but for one and the closest one, but this suggestion fixes that. For example: If I have several "Volume" clips with different individual values ​​and I want to increase or decrease the value of all the clips and all their nodes, I would go to the hypothetical option "Increase or Decrease the value of the clips", and the value of all their nodes would change individually, that is, if I have a clip with 25%, another with 50%, and another with 75%, another with several nodes with different values, but I want to reduce those values, I go to the knob menu, open a widget and add -5%, each clip will be updated and all its nodes will be individually reduced by 5%, resulting in a clip of 20%, another of 45%, another of 70% and the one that had several nodes, they would also be reduced, none would exceed their limits, they would increase or decrease as far as they can, basically it is the "Transpose" of the automation clips, but the automation clips would also have that option by themselves you want to "Increase or Decrease" the value of each one separately.

3. Option to Link cloned automation clips: If you clone an automation clip from one knob to many and select them all together, you would have the "Link clips" option in the menu, so that when you make a change in one clip, that change would affect all the other clips. This would affect the value of all their nodes.

4. Open the last selected automation clip with the "Open Automation Clip" option: you can do that with the editor buttons, but don't you think it would be better if you added this function to the automation knobs? That would make more sense to have that option there. Currently the option opens the closest clip, that's useless when there are many clips and you forget the last one you used.

5. Navigate between all assigned automation clips with Alt + Arrows: Currently in the Piano Roll you can open many clips at the same time, by pressing Alt + Left and Right Arrows, the idea is that this also works for automation clips assigned to the knob you are going to automate with the "Open Automation Clip" option. Right now when you press Alt + Arrows the editor head moves, and that head does not work, but this change would make more sense.

I don't know if I'm wrong or how difficult it would be to apply these changes, but without a doubt these suggestions would work better with all clips and their nodes, because for now those options only affect the closest clips and their first node, and that is useless when a knob has many clips and when they have many nodes, I did the test.

(Google Translate)

[szeli1]

Remove all automation clips with the "Remove Closet Automation Node" option since removing a simple node from a clip, or removing the closest or first clip is useless
Replace the current "Update Closets Automation Node" option with "Increase or Decrease the value of the clips":

This PR is built on the playback position. Your suggestion doesn't. This PR doesn't aim to add new features, but to make existing features faster to use. I think the AutomationEditor should have the ability to support complex features such as this. I could add it in the context menu but I would rather not remove the existing option. Adding this in the context menu would be like trying to fix the broken automation system with a small patch (without addressing the actual issues).

Option to Link cloned automation clips

This isn't supported and this PR doesn't touch the related files. You could make a new feature request about this.

Open the last selected automation clip with the "Open Automation Clip" option: you can do that with the editor buttons, but don't you think it would be better if you added this function to the automation knobs? That would make more sense to have that option there. Currently the option opens the closest clip, that's useless when there are many clips and you forget the last one you used.

Currently the option opens the closest clip before. I can see why this would be an issue. Usually users would like to playback the section which they are working on, so the playback position is almost always before where the automation clip would start. I will change the code so It opens the appropriate clip (Edit: Done). I can implement your suggestion instead if more people support it.

Navigate between all assigned automation clips with Alt + Arrows

This is again an automation editor feature suggestion. It is not related to this PR.

I don't know if I'm wrong or how difficult it would be to apply these changes, but without a doubt these suggestions would work better with all clips and their nodes, because for now those options only affect the closest clips and their first node, and that is useless when a knob has many clips and when they have many nodes, I did the test.

Your suggestions are valid. There should be a way to edit multiple nodes at the same time, but I think this PR's goal isn't to mass edit multiple nodes, it is to make simple quick edits faster by using the playback position.

[bratpeki]

I would like to provide my own opinion on the PR.

---

Firstly, I have tested the options and can confirm everything works as intended. I would still like to provide a list of features and what they do, explained in a way better fit for the end user:

Option	Description
Add automation node	Add a new node, with the currently set value of the parameter, to the appropriate automation clip at the position of the playhead. If a clip doesn't exist, create one, in a new automation track
Add automation node to new clip	Same as the previous option, except that it doesn't append the automation node to an already existing clip, but instead to a new clip, one bar long
Update closest automation node	Update the closes note to the playhead with the value of the parameter
Remove closest automation node	Delete the note closest to the playhead in the clip of the appropriate parameter
Open automation clip	Open the editor and edit the appropriate automation clip

NOTE: If any parameter has more than one clip, the one closest to the top is selected.

---

Secondly, I believe these would have to be appropriately documented, as they are quite confusing out-of-the-box. A YouTube video might do just as well, if not better, because this feature is best explained in-action.

---

Thirdly, my honest opinion on the feature. I do not think this makes a big impact on the speed of editing automation. I tried making a slow sweep (0-25-100) using both methods.

testing-cssli-cmenu.mp4

testing-cssli-traditional.mp4

I have not seen a notable difference, time-wise. Granted, this kind of test doesn't do justice to the other features, but I still do not think these make a noticable impact, given how they invite more work relating to documenting the feature.

I would invite some more testers to form their opinions, since the feature is really nicely made, but just seems a bit redundant to me.

[szeli1]

I would like to provide my own opinion on the PR.

Thanks for testing!

If any parameter has more than one clip, the one closest to the top is selected.

This is not always true and I don't know what you mean by "closest to the top".
Some settings select the closest clip, others select the closest clip before the playback position and "Open automation clip" uses a clip's start and end position to choose what it opens.

Thirdly, my honest opinion on the feature. I do not think this makes a big impact on the speed of editing automation. I tried making a slow sweep (0-25-100) using both methods.

I think it helps window management.

I would still like to provide a list of features and what they do, explained in a way better fit for the end user

Thanks for making this description. I think it is better than my description.

[bratpeki]

I don't know what you mean by "closest to the top".

What I mean to say is "the one that appears first when traversing all automation tracks from top to bottom in the song editor".

Thanks for testing!

My pleasure!