Collection cue status reporting

[dklementowski]

When I first gave LiSP a shot, I found the behavior of Collection cue unexpected - it only executes other cues and immediately ends, so it's completely asynchronous. Because of that when setting up triggers on "Ended" event, it actually fires when the cue is started, so it works exactly like "Started" event and thus it feels more like a bug than intentional behavior.

With this change:

• Collection cue indicates that it's being played as long as any of its child cues is still active
• Triggers work as expected
• In consequence the cue also handles stopping for proper cleanup

If for some reason the original behavior is expected, please explain, I'm open for discussion as this is a feature that I'd actually like to see here.

Please also review the implementation. The change is quite simple and it works fine in my testing, but I don't know the application very well and maybe I didn't test some use-cases.
On a side note, this feature relies on ended cues count. We can make it more strict by comparing those targets identifiers, but it's enough for PoC.

[FrancescoCeruti]

The original behavior was intended, the idea is that the collection cue is simply a trigger for multiple cues at once.
That said, i agree that someone might expect, or desire, the behavior this PR is implementing. I'd like to include this, but I think it needs some changes.

First, I'd like to preserve the old behavior, we can provide a flag to enable a "tracking" mode, or if you have a better name feel free to use it. We should keep the old as the default, for compatibility reasons.

Second, the current implementation is going to miss some "events", you should also connect the "stopped", "interrupted", and the "error" signal. If some cue is stopped before it reach the end, you will not count it.

Third, the stop behavior might be equally confusing, someone might expect that all the cues would stop. I'm not sure if we even need it in it's current form?

[dklementowski]

Thanks for the review!

First, I'd like to preserve the old behavior, we can provide a flag to enable a "tracking" mode, or if you have a better name feel free to use it. We should keep the old as the default, for compatibility reasons.

Agreed! Tracking mode is a nice name, but I'm not sure if that requires some further explanation for user. Maybe writing few sentences about it in documentation would be enough?

Second, the current implementation is going to miss some "events", you should also connect the "stopped", "interrupted", and the "error" signal. If some cue is stopped before it reach the end, you will not count it.

That's something I realized today while messing with the app more. I'm not sure if simply connecting more events would be enough, but that's to be tested.

Third, the stop behavior might be equally confusing, someone might expect that all the cues would stop. I'm not sure if we even need it in it's current form?

You mean stopping the collection cue alone? Yes, that would be logical consequence of the cue being tracked, but there's no control in UI for this as far as I see, and that's also the case with other types of cues like 'command'. I think we can implement it just to make it easier if one day such control was introduced. Correct me if I'm wrong or if you had something else in mind.

[dklementowski]

Alright, I tested the app and am unable to find any problems or misbehaviors. @FrancescoCeruti if you see something that could be improved in the code, I'm happy to make adjustments.

I thought to maybe change the simplistic logic with counter to more precise ID comparison and maybe check additionally if a child que is actually in stopped state, but at the same time I cannot think about any circumstance that this makes a problem. Moreover I prefer simpler solutions.

[FrancescoCeruti]

Thanks, I'll look and let you know :)

[s0600204]

Would it be preferable to count down when cues end instead of up?

For instance: say I have a (tracking) collection cue containing two audio cues, and I Go the collection. Whilst it's still running, I edit the collection and add a third audio cue. When the two running audio cues finish, the collection cue does not enter a stopped state, because it is now expecting three cues to finish, not two. Counting down - "I started two cues, so I'm waiting for two cues to finish" - would prevent this.

On the other hand, starting a (tracking) collection cue (that counts down as cues end) with three cues and removing one of them whilst the collection is running does mean that it now waits for a cue that's no longer part of the collection to finish before entering a stop state...

Admittedly I wouldn't expect to remove or add cues to collections under show conditions, but it is a scenario I could see users getting into when building or editing a show.

---

there's no control in UI for this as far as I see

Setting self.duration to a value greater than 0 within the collection cue class will make it appear in the List Layout's list of playing cues, with a UI that has a stop button. (As long as __start__() returns True.)

(And setting it to a value between 0 and 1 causes the "Action" column in the List Layout to show what Qt5's documentation calls a "busy indicator", which looks a little more interesting than the static 00:00.00 value you'd get otherwise. But doing so causes errors in the Cart Layout, so perhaps avoid doing that. For now.)

[fnetX]

First, I'd like to preserve the old behavior

Isn't / Wasn't (in 0.5) there an option to trigger cues on starting a cue? ("AutoNext" or something like this allowed this IIRC). Maybe instead use a generic way to either start a following cue on cue start or end, instead of creating a specific setting.

Admittedly I wouldn't expect to remove or add cues to collections under show conditions, but it is a scenario I could see users getting into when building or editing a show.

This problem should be prevented properly IMHO, thank you for bringing this up. I had similar problem in QLC+ recently, where I edited something in the wrong moment and had a function literally never stop. If you don't know why it doesn't work and you are stressed, it's the worst kind of experience.

Can adding / removing cues potentially alter the counter in the right direction?

[FrancescoCeruti]

Isn't / Wasn't (in 0.5) there an option to trigger cues on starting a cue? ("AutoNext" or something like this allowed this IIRC). Maybe instead use a generic way to either start a following cue on cue start or end, instead of creating a specific setting.

The option is still there, but it's not a substitute for the current behavior, i think you could emulate it using triggers

On the other hand, starting a (tracking) collection cue (that counts down as cues end) with three cues and removing one of them whilst the collection is running does mean that it now waits for a cue that's no longer part of the collection to finish before entering a stop state...

Yes, it will behave like the cue is still in the collection (until it's finished) ... a lot better than a deadlock.
A possible solution (for both methods) is to override the "update_properties" method and handle running cues that have been added/removed from the collection.

The only gripe is still the "stop" ... someone might want to do the same as the "start" action and to choose what action to trigger on the other cue, but we could add that at a later point to keep things easier.

[FrancescoCeruti]

As noted we should use a countdown, which will prevent "deadlocks" if a cue is added to the collection.

[FrancescoCeruti]

Suggested change

	self.bottomBarGroup.setLayout(QHBoxLayout())
	self.bottomBarGroup.setLayout(QHBoxLayout())
	self.bottomBarGroup.layout().setContentsMargins(0, 0, 0, 0)

We can remove the margins of the inner layout

[FrancescoCeruti]

I think that "Tracking mode" could be a better description for the option

[FrancescoCeruti]

If we don't stop tracking the cue when we "mark (it) as done", if the cue is started again, the counting will be off