Action icons

[tokejepsen]

This is a working version for #53.

One thing I'm not happy about is that when executing an action, the UI becomes unresponsive. I tried to follow how @mottosso went about it for the plugin processing, but I couldn't figure it out.

[BigRoy]

Sweet stuff! :)

One thing I'm not happy about is that when executing an action, the UI becomes unresponsive. I tried to follow how @mottosso went about it for the plugin processing, but I couldn't figure it out.

If the gui is attached to the hosts doesn't it always get unresponsive during processing? I think Pyblish QML works because it's a separate process.

[tokejepsen]

If the gui is attached to the hosts doesn't it always get unresponsive during processing? I think Pyblish QML works because it's a separate process.

You are right :) I was getting confused by the debugging mode and that the gui is responsive between plugins processes.

[mottosso]

Yes, an unfortunate consequence of running a GUI within a host. :(

I've emulated the behavior of QML by having the GUI update in between processes. Happy to hear it went unnoticed for this long. :)

[tokejepsen]

This PR is ready for some testing and inspection then :)

[mottosso]

Would it be possible to see a screenshot? :)

I'll try and take a closer look asap.

[tokejepsen]

[mottosso]

That's great!

[mottosso]

Ok, here we go.

Great job overall. I think it fits the overall code well, and aligns with how it's done in pyblish-qml. It's understandable, is clean and seems maintainable.

There are a few issues with it that shouldn't be too challenging.

• action_processing. The naming of this signal bothers me. A signal generally doesn't trigger its observers until the event loop is at idle. It merely registers the Qt that something should happen as soon as it's ready to handle things. This name, and the location and manner in which it is emitted, suggests that the observers are called before the next line of code. Which isn't true. Now, in this particular case, it would be the case, because the below defer() function will introduce a delay into the code, effectively putting the event-loop into idle, thus triggering this signal where you'd expect. But that's circumstantial. In fact, during testing, this delay is muted, causing no idle, which means tests will experience a different order of operations than actual use. Tests must mute this delay, otherwise they become asynchronous too which greatly complicates testing. Dangerous territory. TL;DR, don't emit signals that are meant to be handled immediately, find an asynchronous way of achieving the same result.
• actions in the general item. Considering plug-ins are the only items capable of hosting actions (as of yet), it would make more sense to store these members in the plug-in item only.
• action argument in update_with_result. This can be determined much earlier I think, as far back as when the plug-in is first added to the model. Maybe take a peek at how it's done in pyblish-qml.

Great work, and let me know what you think!

[tokejepsen]

action_processing.

Is it the naming you have a problem with? Is action_about_to_process similar to the plugin equivalent, better?

find an asynchronous way of achieving the same result.

I tried to defer the action processing, so the icon color would update but didn't have any luck with util.defer

Considering plug-ins are the only items capable of hosting actions (as of yet), it would make more sense to store these members in the plug-in item only.

Sure, makes sense :)

This can be determined much earlier I think, as far back as when the plug-in is first added to the model.

I did it this way to avoid having to have a duplicate method action_update_with_result. Maybe I should have done that instead?

[mottosso]

Is action_about_to_process similar to the plugin equivalent, better? ..
I did it this way to avoid having to have a duplicate method action_update_with_result. Maybe I should have done that instead?

No there shouldn't be any need for this. What we need, is for the GUI to update, before processing begins. Doing this asynchronously could work like this.

1. Right click, an action is clicked
2. Trigger a GUI update, and defers processing till later (by say, 200 ms)
3. Processing begins, GUI is showing signs of currently processing

This way, results can be passed as usual to the model, and the GUI is given plenty of time to redraw itself. No synchronous functions needed. See how _run() is using defer to defer calling some of its own internal functions.

[tokejepsen]

Hey @mottosso,

I'm sorry but I really don't know what I'm doing with these changes. I can't see what the end result should be, so I'm just fumbling around. There are mostly likely big areas of the UI that I don't have the knowledge about to see what is wrong with my code.

[mottosso]

No problem, Toke. I think just by trying you're already gaining some insight, and then later tonight I can show you what I mean and hopefully you should be able to find that aha-moment along with the ability to implement it yourself next time.

GUIs are hard, more so when concurrent. I learned the abstract thinking about it by getting into ZeroMQ and their book which is really great. He talks about how you don't need to wait (i.e. block) for other tasks to finish, but can instead rely on emitting and handling signals when there's time. That's exactly what we're doing here, even with just one thread.

[mottosso]

Ok, having a look at this now.

(1) For starters, I think we can reduce the number of roles for actions. pyblish-qml gets by with only three roles.

• actionIconVisible
• actionPending
• actionHasError

Visible is self-explanatory. Pending means it hasn't yet been run; so it's white/grayed out. If pending is False, it must have either succeeded or failed. Which depends on the value of actionHasError.

(2) In pyblish-qml, action-related data is updated in the controller, not the model. Having gotten back into the reasoning for this, it was because actions in overall never quite fit into the mental model of processing plug-ins and therefore requires special treatment. As a side-note, this is another reason why thinking about action brought shivers down my spine.

What this means is, we won't have to actually update update_with_result but can instead handle it all in the action calling method. We can cause the plug-in to not update in tandem with an action, by simply not updating the model with the result from the action.

I've PR'd your fork, let's continue the discussion in here.

[tokejepsen]

pyblish-qml gets by with only three roles.

My reasoning for having a fourth role action_processing, was to improve the feedback to the user. Currently when a long running action is processing, nothing visual happens causing the user to think the UI has stalled.

[mottosso]

This feedback should still happen, the GUI updates when the user hits the button, but actual processing doesn't start until 100 milliseconds later, as per the call to defer().

[tokejepsen]

This feedback should still happen

The icon does not turn light-blue for processing, it just turns green/red depending on the result of the action. This is how it looks with your PR.

[mottosso]

Have a look at whether the delegate actually as code in it to color it light-blue.

• https://github.com/mottosso/pyblish-lite/blob/45d96d9b27e32de52b530ef7ab40613741dffe50/pyblish_lite/delegate.py#L101

The timing looks right to me, it does update the color when you press it and only freezes afterwards.

[tokejepsen]

Have a look at whether the delegate actually as code in it to color it light-blue.

I get that it doesn't have the code for colouring it light-blue for processing, but I don't get how to implement this behaviour with just three roles for actions.

[mottosso]

Ahaa, ok. Now I'm with you. In that case, it's possible you need one more role. Unless you can utilise the current state of the program, being in "publishing" mode, from within the delegate.

[tokejepsen]

Unless you can utilise the current state of the program, being in "publishing" mode, from within the delegate.

That's a good idea. Giving it a try.

[tokejepsen]

Here is a preview of the current state of this feature.

One visual difference is that the plugins checkbox updates as well when the action is processing. The checkbox still retains its previous state, but runs light-blue when processing. This is because of using the model.IsProcessing role, but I think it works.

The code is ready for another review as well.

[mottosso]

Looks great!

Does an action affect the color of the plug-in indicator, when the plug-in hasn't yet been run?

[tokejepsen]

Does an action affect the color of the plug-in indicator, when the plug-in hasn't yet been run?

Nope, stays idle color if the plugin was idle before the action was run.

[mottosso]

Ooh, neat. I noticed it was altering the color if it was already green. As in your gif.

If that's predictable, then I have no problem with that. Looks good, will take a look at the code asap.

[mkolar]

I'm wondering if this could be merged as it kinda stayed hanging here :). We've used in in production for a month now without issues. Apart from the ones originating from other parts of pyblish-lite.

[mottosso]

Sorry about that, I haven't had time to look into it yet.

Having a look now.

[mottosso]

This is the only thing making me hesitate.

Some day, me or someone else will try and use this function and wonder "Why does update_with_result() take a result and an action?".

But I think it's safe to look past, so long as we have this conversation on record and that it will at some point be fixed. :)

[mottosso]

@tokejepsen Could you version this up for me? A minor increase should suffice.

Then I'll make a new release with it.

[tokejepsen]

Assuming that is enough?

[mottosso]

Ops, don't forget to zero out the 3.

• 0.4.0.

Starting fresh from new minor version.

[tokejepsen]

Whoopsie, good point:)

[mottosso]

Done!