companion icon indicating copy to clipboard operation
companion copied to clipboard

Upgrade script for variables

Open Julusian opened this issue 3 years ago • 2 comments

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

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

Is there an existing issue for this?

  • [X] I have searched the existing issues

Describe the feature

When making changes in module, it is possible to write upgrade scripts to fix/convert old actions/feedbacks to any new types. But there is no way to tell companion that a variable has been renamed and should be changed everywhere it is referenced.

I'm not sure if this is actually solvable, because the variable could be referenced inside of any action/feedback and we shouldnt go around modifying the contents of the action objects without getting the modules involved. Additionally, if the variables are referenced in pages which are imported after the upgrade has been run, then what can we do?

Usecases

No response

Julusian avatar Feb 09 '22 09:02 Julusian

I think if you wanted this solved, it would be best to introduce new action option types that have variable parsing baked into them, so the module never sees the "raw" string but only receives the parsed version of it, with variable references removed. Then you would have to incrementally transition all modules over to these new option types, while also removing the parseVariablesInString functions. (This would probably need to come after a better typing system for options -- a system forcing use of accessor functions to get option values, and then the accessor can perform the parsing if such option is accessed.)

One minor benefit along the way is that local variable support would no longer require opt-in, in terms of supporting it or indicating support for it. It would be implied by the option type and could be indicated without any opt-in.

So, yeah: "getting the modules involved" seems effectively required here, and it would be a breaking change.

Also worth note, there's at least in principle a concern about variable references formed from other variables, so that you can't actually see all variables that might be used in order to rewrite them.

jswalden avatar Mar 18 '25 18:03 jswalden

That auto-parsing is being planned as part of https://github.com/bitfocus/companion/issues/2345, which might happen at some point this year. I dont think we have agreed yet on how breaking this change would be, and whether modules need to opt into it in some way.

Also worth note, there's at least in principle a concern about variable references formed from other variables, so that you can't actually see all variables that might be used in order to rewrite them.

Good point, thats another edge case I hadn't considered.

Perhaps the solution here is that maybe this should be done so that when the module defines the rename, that sets up an alias for the old name, so that the old name will continue to work but will not be suggested in the ui or anywhere.
This is what we did when changing custom variables recently

Julusian avatar Mar 19 '25 10:03 Julusian