Fix#335

[davidlatwe]

This PR resolves #335, with some PEP8 changes, and fixed one minor bug.

[davidlatwe]

Was thinking about adding buttons for switching targets directly in QML, and here's what I have got.

Directions

The newly added tab was called directions.
I named it directions was because it's a list of targets, so I thought maybe we could say a list of target is a direction (like a vector), and this tab is listing all directions for user to pick.

For listing multiple directions, you have to define them in pyblish_qml.settings, like:

pyblish_qml.settings.Directions = {
    "Remote Publish": {
        "description": "Publish in remote machine",
        "targets": ["default", "deadline"],
    },
    "Local Publish": {
        "description": "Publish via local machine",
        "targets": ["default", "localhost"],
    },
}

QML will pick up and listing them in tab as buttons, and indicate current targets.

Note that this is backward compatible, so it's okay to leave pyblish_qml.settings untouched, if you only use one set of targets.

Motivation

After #335 been fixed, you still need to press button out side of QML for changing targets, so I though maybe we could improve this a bit further by adding this feature.

Please let me know what you think, and I'll elaborate more later today. :)
And if the responds are good, I will prepare another PR after this one gets merged.

[mottosso]

Looks interesting. I'm a little unsure of how targets are being used, so I can't give a definite opinion on how to represent it visually. Overall, I would have thought "target" is the most important and first choice to make when publishing. Like when you go to McDonalds and make your selection in one of their order screens, you start by choosing "Eat in" or "Take away".

For targets, I would have thought that this choice should come even before a Pyblish GUI is shown, like as another option for showing the GUI.

• Publish Local
• Publish Remote

With that in mind, it seems a little backwards to have it nested three or more clicks into the GUI. But then again, maybe this is a decision you make late, or several times from within the same publishing session?

I named it directions was because it's a list of targets, so I thought maybe we could say a list of target is a direction (like a vector), and this tab is listing all directions for user to pick.

Not too sure about this one, would rather not introduce more jargon; I can already hear the questions in email, chat and forums xD

How about making it a list if checkboxes instead? One where default is already checked? Alternatively, do we need multiple targets per target? I.e. can we have just localhost and deadline and implicitly include default?

[davidlatwe]

Overall, I would have thought "target" is the most important and first choice to make when publishing.
...
For targets, I would have thought that this choice should come even before a Pyblish GUI is shown

Yes, that's definitely, so in production there's a few buttons (currently two of them here) for artist to pick.

maybe this is a decision you make late, or several times from within the same publishing session?

Yes ! That's what it's for, but also an indicator for current targets.

How about making it a list if checkboxes instead? One where default is already checked?

I have thought about that too. :P

But I think the best is to make user can only select one target from GUI, since there may have conflict between targets. Like local and remote, it's obviously redundant to enable both of them, but a bit hard to implement the targets' relation logic onto GUI if you want to prevent that happen. Like you can have A and B but you can not have B and C at the same time.

Alternatively, do we need multiple targets per target?

I think we need, target for me, may refer to "the site which actually handling the final publish process" and "the destination of the publish result". Like dailies, it could work with local or remote.

[tokejepsen]

What happens with the environment/plugins when you choose a different target?
The collection would need to be refreshed when choosing a different target.

The newly added tab was called directions.

I think the terminology will get confusing if you call the tab directions rather than targets.

Yes ! That's what it's for, but also an indicator for current targets.

@davidlatwe do you have a use-case for using different targets within the same session?

[davidlatwe]

What happens with the environment/plugins when you choose a different target?

If you only selecting targets, nothing will happen, the targets only get sets to host when you press reset button on and only on that tab. And once you press the reset, yes, the collection would be refreshed as well. :)

I think the terminology will get confusing if you call the tab directions rather than targets.

Haha, second objection vote ! Then I'll change that in GUI, but I think I still need that name internally for naming variables, need a think. :P

do you have a use-case for using different targets within the same session?

The example use case would be, like publishing cameras and renders from the same scene, the camera only need to be published from local machine, but the renders require to be sent to render farm. So there's two sets of targets need to switch. ☺️

[tokejepsen]

If you only selecting targets, nothing will happen, the targets only get sets to host when you press reset button on and only on that tab. And once you press the reset, yes, the collection would be refreshed as well. :)

I completely forgot how the targets worked there :)

Nice work!

[BigRoy]

On vacation till Wednesday so my reply might be brief.

Looks really nice!! Some artists on our end will love this.

Not sure about the directions name and having more terminology around, but allowing to switch within Pyblish I think some Artists on our end might love quite a bit. It's like exposed allowed combinations of targets. Like a targets group or targets combo. What @mottosso describes as "checkboxes" might be problematic on our end as some combinations of targets should not be used together, e.g. remote with local. So I can see how exposing allowed targets groups would be useful.

I wonder if we need that separate page or whether it could be part of something like a pop-up combobox styled like that (very nice and clear!) when maybe clicking on the targets or having a clear button on the main page that describes you can switch. In the meantime the standalone page is a very nice setup.

[davidlatwe]

Thanks @BigRoy. :D

Sorry again for not posting that new GUI stuff in another issue. A bit excited at the time :/
I'll push the code in a new PR.

Back to the main issue on fixing #335, I think we could merge this ?

[BigRoy]

Unable to test this myself. But just the fix itself for changing the targets in the main session would be great to see merged. Other than that, the GUI issue into its own PR would make things easier yes. Nice work on both though!

[mottosso]

Back to the main issue on fixing #335, I think we could merge this ?

Hold up a minute, at the moment, targets is being changed by calling on the Application, which modifies the contained Controller. That does work, but it looks like a really roundabout way of accomplishing it. Are there any alterantives to this? Can we reach the controller, or Pyblish running in the host, some other way?

[davidlatwe]

Are there any alterantives to this? Can we reach the controller, or Pyblish running in the host, some other way?

Hmmm, I don't think so. Since the main pyblish logic (main iter) is running in Controller, and filtering plugins by targets happens here (also in Controller).

So the host only responsible for discovering plugins and executing plugins which assigned by Controller, based on that, I think contact the Application is the only way for now. 🤔

[davidlatwe]

The targets changing workflow (host to app) after this PR is as follow :

1. Firing new targets while QML still open by pyblish_qml.show(targets=["new_target"])
2. The host using server.Proxy to send new targets
3. Application receive new targets
4. Application put new targets to Controller
5. Controller reset

[mottosso]

Ok, I think that makes sense. It runs via the same "channel" as showing/hiding the GUI, whilst still running. Was a little worried we introduced a new line of communication, when we clearly are already capable of communicating with plug-ins/instances. But this looks legit, thanks for confirming.

Happy with this. 👍

[davidlatwe]

Thanks @mottosso ☺️
Preparing merge now.