Add possibility to use variables for broadcast selection in actions and feedbacks #115
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jendamozna
approved these changes
Jul 9, 2024
|
Please add unit tests for your feature. |
Comment on lines
+99
to
120
| { | ||
| type: 'checkbox', | ||
| label: 'Use variables for broadcast selection', | ||
| id: 'use_var', | ||
| default: false | ||
| }, | ||
| { | ||
| type: 'dropdown', | ||
| label: 'Broadcast:', | ||
| id: 'broadcast_id', | ||
| choices: [...broadcastEntries, ...broadcastUnfinishedEntries], | ||
| default: defaultBroadcast, | ||
| isVisible: (options) => options.use_var == false | ||
| }, | ||
| { | ||
| type: 'textinput', | ||
| label: 'Broadcast', | ||
| id: 'broadcast_vars', | ||
| useVariables: true, | ||
| tooltip: 'Use unfinished_ followed by a number to dynamically select an unfinished broadcast, or the Video ID for static selection', | ||
| isVisible: (options) => options.use_var == true | ||
| }, |
There was a problem hiding this comment.
This is becoming unmanageable. I think you should create a function createBroadcastChoices that takes the varying aspects (like the choices for the broadcast_id), and returns an array with the use_var, broadcast_id and broadcast_vars elements. The function is then called with the correct parameters instead of the three elements:
Suggested change
| { | |
| type: 'checkbox', | |
| label: 'Use variables for broadcast selection', | |
| id: 'use_var', | |
| default: false | |
| }, | |
| { | |
| type: 'dropdown', | |
| label: 'Broadcast:', | |
| id: 'broadcast_id', | |
| choices: [...broadcastEntries, ...broadcastUnfinishedEntries], | |
| default: defaultBroadcast, | |
| isVisible: (options) => options.use_var == false | |
| }, | |
| { | |
| type: 'textinput', | |
| label: 'Broadcast', | |
| id: 'broadcast_vars', | |
| useVariables: true, | |
| tooltip: 'Use unfinished_ followed by a number to dynamically select an unfinished broadcast, or the Video ID for static selection', | |
| isVisible: (options) => options.use_var == true | |
| }, | |
| ...createBroadcastChoices([...broadcastEntries, ...broadcastUnfinishedEntries]), |
astellingwerf
suggested changes
Sep 7, 2024
There was a problem hiding this comment.
This code should be made a lot more DRY. Repeating the same code 15 times is a bad idea.
| @@ -16,6 +17,7 @@ export enum FeedbackId { | |||
|
|
|||
| /** | |||
| * Get a list of feedbacks for this module | |||
| * @param The companion instance used to | |||
There was a problem hiding this comment.
Suggested change
| * @param The companion instance used to |
Comment on lines
+252
to
+261
| if (event.options.use_var) { | ||
| if (!event.options.broadcast_vars) return {}; | ||
| } else { | ||
| if (!event.options.broadcast) return {}; | ||
| } | ||
| const id = ( | ||
| event.options.use_var | ||
| ? await context.parseVariablesInString(event.options.broadcast_vars as string) | ||
| : event.options.broadcast | ||
| ) as BroadcastID; |
There was a problem hiding this comment.
The same, or very similar code, is present in this PR at least three times.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.