Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix context change on save #68

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kramester
Copy link
Contributor

@kramester kramester commented Jan 10, 2020

  • on_load and on_save callbacks were refreshing all the engine apps every time the user saved/loaded even if the context was unchanged.
  • added if statement to prevent refresh if ctx was unchanged

This was causing some serious lag every time an artist would save.

kramester and others added 4 commits July 10, 2018 09:51
Engine.context_change() works in headless mode (shotgunsoftware#55)
For SG-7668: Updated Nuke version to support 11.2 (shotgunsoftware#57)
- on_load and on_save callbacks were refreshing all the engine apps every time the user saved/loaded even if the context was unchanged.
- added if statment to prevent refresh if ctx was unchanged
@kramester kramester requested a review from pscadding January 21, 2020 16:16
@pscadding
Copy link
Contributor

pscadding commented Jan 21, 2020

Hi @kramester, thanks a lot for submitting this. Someone else has submitted something similar before, and when I tested it, it caused issues.

It introduces a bug, where the sgtk menu remains disabled in the scenario where you were in a context (say shot 10, comp), then save file outside of the structure so that the menu becomes disabled. If you then save the file again back into the work directory with the same context as previously, the menu stays disabled.
This is because when the menu is disabled, it doesn't actually destroy the engine or change context, and the menu only gets rebuilt if you start the engine. So if the disabled menu is present and your context doesn't change when you save a file into the correct location then it will stay disabled.
I appreciate this is a bit of an edge case, and in theory, there is nothing wrong with your suggestion per se but additional work would need to be done to find a way around this.

@kramester
Copy link
Contributor Author

@pscadding, sounds like we just need to store the menu state and add that as a condition on whether to run the callbacks.

The lag on save is the biggest issue here. Artists don't expect to have to wait a couple seconds for a quick CRTL+S save. Adding this condition made the saves instantaneous as they should be. The edge case you're describing is very much an edge case for us as we never save outside the pipeline. We're going to continue to use my patch here until you guys come up with something better.

@pscadding
Copy link
Contributor

@kramester Yeah if the refreshing of the context is causing pain, then we should find a way to avoid the refresh. I've logged an internal ticket to look at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants