feat: allow part to be queued from onTake #1497
Conversation
|
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
jstarpl
added
Contribution
There was a problem hiding this comment.
I think that returning this function and reusing the actionService is not the best way to achieve this.
I would prefer this to:
- In the
queuePartAfterTakemethod, perform the relevant bits of the validation that the actionService currently performs, stopping beforeinsertQueuedPartWithPieces. partToQueuein the context should then contain thenewPartandpiecesthat this generates as part of the validation/processing- Instead of calling your returned
queuePartcallback, perform theinsertQueuedPartWithPiecescall
As a bonus, the validation moving will mean that the blueprint call to queuePartAfterTake will throw errors instead of them being thrown later when they will likely stop the take from happening.
| await queuePart() | ||
| } else { | ||
| // Once everything is synced, we can choose the next part | ||
| await setNextPart(context, playoutModel, nextPart, false) |
There was a problem hiding this comment.
The definition of nextPart would be good to move into this else block, so that it is only run when needed and make sure it doesnt get used elsewhere accidentally
| if (onSetAsNextContext.partToQueue) { | ||
| const partToQueue = onSetAsNextContext.partToQueue | ||
| queuePart = async () => { | ||
| await actionService.queuePart(partToQueue.rawPart, partToQueue.rawPieces) |
There was a problem hiding this comment.
I don't like how this function gets returned, and am also wondering does this even work correctly?
Largely because this working is reliant on the nextPartState in the service as being NONE. So if any other changes are made to the current part in the callback then this will fail.
Being pickier (perhaps too much so), this relies on the implementation detail of how the actionService gets currentPartInstance. If that were changed to be a less dynamic reference, then this would break (I am pretty sure that nowhere else relies on this mutability).
About the Contributor
This pull request is posted on behalf of TV 2 Norge.
Type of Contribution
This is a:
Feature
Current Behavior
Not possible to queue a part from
onTake()New Behavior
Blueprints can queue a dynamic part during the
onTake()callback. AqueuePartAfterTake()method was addedIOnTakeContext. Naming of the method is quite verbose, to make it clear that it queues the new part after the part currently being Taken. In the future there might be a need to add another method that e.g. queues a part instead of performing the Take.Testing
Affected areas
This PR affects the playout logic, specifically what happens during a Take. It introduces a non-breaking addition to the Blueprints API.
Time Frame
Other Information
Status