getavalon / core

The safe post-production pipeline - https://getavalon.github.io/2.0
MIT License
218 stars 49 forks source link

Update Maya fileDialog's start-dir when task changed #340

Closed davidlatwe closed 6 years ago

davidlatwe commented 6 years ago

Before:

task_before

FileDialog's start-dir will stay at previous opened folder path after task changed.

After:

task_after

Now it will point to the scenes folder of that task after task changed. And, this is not recorded in GIF, but it will prompt a confirm dialog to notify artist to use Launcher, due to the workspace of that task is un-initialized (folder not exists).

davidlatwe commented 6 years ago

One thing that I am not sure of is, should we reset back to previous context when Avalon says new context's workspace not exists ?

To me, I think it should, and the workspace check should implement in avalon.api.update_current_task, right before emitting taskChanged, raise error if not exists, and the prompt dialog should be implement in context manager.

What do you think ?

BigRoy commented 6 years ago

One thing that I am not sure of is, should we reset back to previous context when Avalon says new context's workspace not exists ?

We have on task changed callback that creates the workspace if it does not exist yet (in Colorbleed config). So for us it's fine if it does not exist yet. :) Does that help?

Could port that somehow to core? (Thinking of it... that might even be in our core repo, but I am not sure)

davidlatwe commented 6 years ago

Oh I see~ I found the implementation of workspace initialization in Colorbleed config, you use avalon.Application to initialize it, which making more sense ! Nice :P

Then I think I will remove the prompt dialog in this PR.

Could port that somehow to core?

I think we can ship that logic to core in next PR, take Maya for example:

# avalon/pipeline.py
...

def on_task_changed(*args):
    """Wrapped function of app initialize and maya's on task changed
    (copied from Colorbleed config)
    """

    # Inputs (from the switched session and running app)
    session = avalon.Session.copy()
    app_name = os.environ["AVALON_APP_NAME"]

    # Find the application definition
    app_definition = pipeline.lib.get_application(app_name)

    App = type("app_%s" % app_name,
               (avalon.Application,),
               {"config": app_definition.copy()})

    # Initialize within the new session's environment
    app = App()
    env = app.environ(session)
    app.initialize(env)
# avalon/maya/pipeline.py
from .. import pipeline
...

def _on_task_changed(*args):
    pipeline.on_task_changed()

    # Do something app specific action..
    _update_menu_task_label()
    workdir = api.Session["AVALON_WORKDIR"]
    if os.path.exists(workdir):
        _set_project()
        ...

Then avalon.maya.install() will register this chained callback for us !

(Thinking of it... that might even be in our core repo, but I am not sure)

I'll go there and sniff around for good stuff :D

BigRoy commented 6 years ago

I'll go there and sniff around for good stuff :D

Sure do! ;)

This Maya browser fix is already really good - nice work!

davidlatwe commented 6 years ago

Thanks @BigRoy :) I'll leave it as is and merge in next couple day if no further objections.

BigRoy commented 6 years ago

The warning message might still be conflicting with our "solution" to ensuring the workspace actually exists in our config. Because I think ours will do it directly after this callback - so the user gets warned for no apparent reason?

Can we avoid the pop-up but just have it log a sensical message?

davidlatwe commented 6 years ago

Because I think ours will do it directly after this callback

On the contrary, I believe it's initializing workspace before this callback, cause I just stealing your "solutions". :P

But since you have mentioned, I will remove that for good, I now aware it's a bit redundant for the future development heading.

davidlatwe commented 6 years ago

Done. I leave the logger warning massage as is, should be fine right ?

BigRoy commented 6 years ago

Sure is! Thanks again. Feel free to merge!

davidlatwe commented 6 years ago

Merging this. Thanks @BigRoy :)