pyblish / pyblish-qml

Pyblish QML frontend for Maya 2013+, Houdini 11+, Nuke 8+ and more
GNU Lesser General Public License v3.0
114 stars 44 forks source link

Fix registered targets lost when re-launching on existed server #355

Closed davidlatwe closed 4 years ago

davidlatwe commented 4 years ago

This was reported in Gitter 👉 February 3, 2020 4:54 PM from @BigRoy .

It was a bug after #347, it was trying to solve #335 which was assigning and changing targets via pyblish_qml.api.show. But the solution did not pay attention to the case that is using targets from pyblish.api.registered_targets, hence we got the bug.

Reproducible

import pyblish.api
import pyblish_qml.api

# Register some random target.
pyblish.api.register_target("custom")
# Launch !
pyblish_qml.api.show()
# Now navigate to the `Terminal` page on top of GUI, should see our target "custom" in view.
# While window still exists, launch again.
pyblish_qml.api.show()
# Targets has been wiped..

What's changed

BigRoy commented 4 years ago

Nice - thanks for looking into it so quickly.

This could be a tricky one though. What happens on:

pyblish_qml.show(targets=["custom"])
# ... keep window open
pyblish_qml.show(targets=None)

Would we want the latter to resort to the default of using [default] + api.registered_targets() like on the initial run or to have it persist whatever it had.

It seems most logical if it behaved the same to a complete refresh (close and run) with the same arguments. As such, should it on targets=None be targets = [default] + api.registered_targets() as is done on __init__. I'd expect that to be the most logical behavior. Does it currently behave like that?

davidlatwe commented 4 years ago

Does it currently behave like that?

No ! And I think you are right, that would be what I'll expected, too.

davidlatwe commented 4 years ago

Does it currently behave like that?

And now it does.

Previously, targets provided from pyblish_qml.api.show will remain the same if next show call has no targets given. After 3fdccb5, it will fallback to [default] + pyblish.api.registered_targets().

davidlatwe commented 4 years ago

Thanks ! Will merge this later today :D

davidlatwe commented 4 years ago

Arrr sorry, forgot to bump the version, will push to master. 😭