Allow for many-to-one and one-to-many action merge and split in upgrade scripts

[fa1k3n]

Is this a feature relevant to companion itself, and not a module?

• I believe this to be a feature for companion, not a module

Is there an existing issue for this?

• I have searched the existing issues

Describe the feature

Currently when upgrading an action the upgrade callback gets called for each action on every button. This makes it impossible to clean up user action lists where a new action consolidates two or more old actions. Since the upgrade is one-to-one the result after such an upgrade is that the action list contains multiples of the new action with different parameters, then the user needs to manually clean this up. Todays system is proven to work, but it would be nice for an upgrade to take into account all the actions on the button and consolidate the applicable ones into one new action.

Probably a restriction to put on the system is that it can only work on the same group of actions and if these actions are concurrent in their execution. If a e.g. wait is inserted between the two merge sources then a merge can not take place, e.g. let A, B and C be actions , A and B are from same module and are the merge source, let D be the merge target . Then the following would be allowable merge cases
[A, B, C] => [D, C] , [A, C, B] (concurrent) => [D, C] , [C, wait [B, A]] => [C, wait[D]]
The following would not be allowed
[A, wait [B, C]], [A, [B, C]], [A, C, B] (sequential)

Splitting an action up into more actions would probably be much more straight forward as none of the group, inter module dependencies and timing issues needs to be considered.

Another issue with merging actions is the headline merging, it might be enough to just concat the two merge sources headlines and slap a "MERGED" or similar in front of the new headline.

Usecases

In the Osee module there was 5 actions to control masking of a source, enable, horizontal start, horizontal end, vertical start and vertical end . We created a new action with a multiselect where each or many of these 5 properties can be selected and controlled. In this upgrade it would have been perfect for us to use an "up to 5"-to-1 upgrade where if a specific old action is present on the button then the specific property is added to the new buttons and the old buttons value is copied to the new buttons. So instead of potentially ending up with 5 new actions each setting one property each, we would end up with 1 action with 5 properties

[dnmeid]

Indeed I had several occasions so far where it would have been helpful being able to merge or split things, most often feedbacks.
You named the challenges.
Because of them and the fact that it is very rare case I don't think that this will be addressed soon. But let's keep this issue as a reminder.

[jswalden]

If upgrade scripts were narrowed to (or augmented to additionally permit) simple functions that accept an action and either return a rewritten action (possibly the same one) or null (if no rewrite) as I've roughly implemented in ptzoptics-visca -- and as @Julusian asked me about in a DM recently, musing about improving the upgrade script type -- then splitting could be readily handled by allowing return of an array of actions to replace it. (Or you could use return type [CompanionUpgradeAction, ...CompanionUpgradeAction[]] | null, and usually an array return would have one element.)

For joining, I would think runs of mergeable functions "should" be permitted regardless whether sequential or concurrent, and the processing type would have to be supplied to the merge implementation to factor into whether or not to merge. As for how to do it: if modules supplied a "fold" operation to Companion (a fresh, optional trailing argument to runEntrypoint?), it could be passed in two actions (and "sequential"/"concurrent"), then would either return an action that combined the two actions or returned null to leave them alone. This operation could be run across all action lists in the user's configuration, and on partial/total configuration import. It would only allow a module to fold pairs of its own actions. And it would perform folding after upgrade scripts are run, so that the fold operation only ever considers currently existing actions, not historical actions.

This would only allow binary folds. It would not enable N-ary folds, if in theory a module had actions A/B/C that if they appeared as such could be rewritten to ABC. I don't think this is a crazy limitation. The fold/reduce operation is already well-known, well-used in computer science and seems "good enough" to me. Plus N-ary folding in the abstract would make for debatably nonobvious results if for actions A/B/C/D you could merge A/B/C or B/C/D. (Also I think there are problems if action splitting and action merging were interleaved, e.g. if a split enables a merge which enables a split which...)

[Julusian]

it would perform folding after upgrade scripts are run

I have a concern here. In some cases it will be valid for a user to have A B and to want to keep those separate. If we merge them one time then that will be annoying, but they will get over it. If we keep merging them every launch or update, then we are fighting the users, and telling them that their perfectly valid use is wrong.
So I think this needs to be a step in an upgrade script, so that we can be sure it only gets run once.

This is something that needs some thought. There are reasons for a module to want to merge usages of actions, there are also reasons that a user might not want the most compact form of their actions. So merging is fine, but we should encourage it to only be done when there is a technical reason to do it.

then splitting could be readily handled by allowing return of an array of actions to replace it

Even if we don't change the upgrade api significantly, theres nothing to say we couldn't make the changedActions array support the current object and a new 'split' object: { id: action.id, split: [{...}, {...}] }.
Merge doesn't really fit into this because it needs to know what is allowed to be merged.

---

If we do this, I think it is going to involve companion passing the actions to modules in groups. Where the actions inside each group can be merged if the module wishes. This means groups will be formed based on ones in concurrent groups that will execute at the same time, or in sequential groups where they are directly adjacent.
And we will need to make sure that disabled actions are not groupable with enabled actions, possibly more future properties will be like this too.

Something else to decide on is what to do with the user defined 'headlines' here, but concatenating them may be sufficient.

---

So far this has only talked about actions, but if we do something for actions it will be odd (but not unprecedented) to not have it for feedbacks too.

Feedbacks have mostly the same challenges, but also some more;

• The Grouping will have to consider whether feedbacks are inverted
• They can be inside a group that uses the feedback values as either OR or AND, so when merging it will need to factor that into the merge. When splitting, that will need to be factored in (could be by companion wrapping them in a logic feedback)

[jswalden]

Fair enough making folding an upgrade action, in order that it be only run once, that makes sense.

I guess realistically folding, since it has to be optional because of disabled actions, has to be considered unrelated to changes to the current set of valid action IDs. And if you want to fold to eliminate actionIds, you have to first rewrite all the eliminated actionIds, then the fold can consolidate supported actionIds but doesn't alter the full set of supported actionIds.