Implement pyblish-base/#250 #266
Conversation
|
Will this also work with the matching algorithms? |
|
Since it triggers the |
|
@tokejepsen are you able to test this? :) We have this live in production here right now, but are not using |
|
Seems to work just fine here :) |
pyblish_qml/control.py
Outdated
| # When filtering to families at least a single instance with | ||
| # that family must be active in the current publish | ||
| if "*" not in plugin.families: | ||
| if not any(instance.data.get("publish") is not False |
There was a problem hiding this comment.
I can't make sense of this line.
If not "any of these" is not "Not True", then loop through the below. Have a look and see if you can't make this more intuitive
There was a problem hiding this comment.
The check is this explicit to check for False because the Pyblish logic does the same with such an explicit check. As such I couldn't just do not instance.data.get("publish") because then it would also consider None whereas the Pyblish logic wouldn't.
The logic is that at least one instance must have its publish set to True. So if there's no "active" one it should continue.
The check whether it's true is: instance.data.get("publish") is not False
This could also work:
def _is_active(instance):
"""Whether instance is currently set active to publish"""
return instance.data.get("publish") is not False
if not any(_is_active(instance) for instance in instances):
continue
|
Ok, I took a stab at simplifying the logic. It's still complex, and I haven't tested it. Let me know what you think. |
pyblish_qml/control.py
Outdated
|
|
||
|
|
||
| no_wildcard = "*" not in plugin.families | ||
| no_active_instance = not any(inst.data.get("publish") for inst in instances) |
There was a problem hiding this comment.
@mottosso note that this is now a different check than what logic.py does in pyblish-base because there it explicitly checks for is not False. This could potentially start behaving different.
There was a problem hiding this comment.
Yes. Do try this locally first, then we'll need to update Lite and Base to follow suit. It's a backwards incompatible change, so odds are we'll put it behind a PYBLISH_SOMETHING_SOMETHING environment flag to be explicitly enabled.
|
The Make sure we don't break that. |
Like this? 😄 Interesting that the previous also passed the tests, that should've been caught by a test. |
This project is lacking a good test suite, which is also why it's a little risky to change things around. But if you could give this a go yourself and keep an eye out for oddities, I think we should be fine. |
|
Ok, thanks! Any idea how to solve this one: 6a20765#diff-ac156cba73ac1ef385b357bbfc21d258R698 ? In pyblish base I made this backwards compatible by implementing a toggle in |
Implement the same functionality for QML as for base regarding: pyblish/pyblish-base#250
This should work with PR: pyblish/pyblish-base#325
Note: Somehow when toggling off an instance of family
ait will still show the ContextPlugins that are filtered to family a, but it will not run them. How to visually hide them I have no clue, but aside from cosmetics it is working.