-
Notifications
You must be signed in to change notification settings - Fork 128
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
Draft: Feature: Handling out of date workfiles in Blender #4176
Draft: Feature: Handling out of date workfiles in Blender #4176
Conversation
* Enhancement: copy last published workfile as reusable methods (WiP) * Added get_host_extensions method, added subset_id and las_version_doc access, added optional arguments to get_last_published_workfile * Plugged in the new methods + minor changes * Added docstrings, last workfile optional argument, and removed unused code * Using new implementation to get local workfile path. Warning: It adds an extra dot to the extension which I need to fix * Refactoring and fixed double dots * Added match subset_id and get representation method, plus clan up * Removed unused vars * Fixed some rebasing errors * delinted unchanged code and renamed get_representation into get_representation_with_task * This time it's really delinted, I hope... * Update openpype/modules/sync_server/sync_server.py reprenation isn't the right spelling (: Co-authored-by: Félix David <felixg.david@gmail.com> * Changes based on reviews * Fixed non imperative docstring and missing space * Fixed another non imperative docstring * Update openpype/modules/sync_server/sync_server.py Fixed typo Co-authored-by: Félix David <felixg.david@gmail.com> Co-authored-by: Hayley GUILLOT <hayleyguillot@outlook.com> Co-authored-by: Félix David <felixg.david@gmail.com>
from openpype.lib import Logger | ||
from openpype.lib.local_settings import get_local_site_id | ||
from openpype.modules.base import ModulesManager | ||
from openpype.pipeline import Anatomy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is public definition of sync server which must be at this moment Python 2 compatible and should be clear without pipeline imports if possible. Sorry not true, this is not in public part.
BTW Even I understand the use case I don't think those functions should be called directly from blender as blender workflow is now dependent on sync server in it's core (which is not good). He should find if there is sync server addon, if it's enabled, and then call a method which does all the logic and return or raise result, but blender should not know anything about how it works (whole content of DownloadLastWorkfile
). If anything changes how sync server works (and it will soon) Blender will break too so he should know almost nothing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I see what you mean. I'll edit my implementation.
|
||
Returns: | ||
None: This is a void method. | ||
""" | ||
|
||
sync_server = self.modules_manager.get("sync_server") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if sync server is not available or enabled it will happen anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these kind of checks must remain in hooks to have explicit logging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works fine, good job! (even if I'll review it after #4102 is merged and your branch will be rebased)
Doesn't work when local settings are both on studio
. It holds forever.
from openpype.hosts.blender.api.workio import current_file | ||
|
||
|
||
def is_work_file_out_of_date() -> bool: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
openpype/hosts/blender/api/ops.py
Outdated
("DL", "Download last workfile", "Download last workfile"), | ||
("QUIT", "Quit blender", "Quit blender"), | ||
("PROC", "Proceed anyway", "Proceed anyway AT YOUR OWN RISK"), |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
openpype/hosts/blender/api/ops.py
Outdated
return self.execute(context) | ||
|
||
|
||
class DrawWorkFileOutOfDateWarning(bpy.types.Operator): |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
openpype/hosts/blender/api/ops.py
Outdated
project_name, subset_id, fields=["_id", "name"] | ||
) | ||
if not last_version_doc: | ||
print("Subset does not have any version") |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
openpype/hosts/blender/api/ops.py
Outdated
bl_idname = "wm.workfile_out_of_date" | ||
bl_label = "WARNING: Workfile is out of date" | ||
|
||
action_enum: bpy.props.EnumProperty( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rename it simply "action"
|
||
# Getting date and time of the latest locally installed workfile | ||
# Time is converted to use the same format as for `last_publishd_time` | ||
workfile_time = get_timestamp( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You better keep the value of the last publish time in a bpy.props.StringProperty
or maybe in a FloatProperty
to have fewer conversions.
@Tilix4 is it already ok to test it / review it? |
…k implementation. Using blender self.report, changing ops prefixes, changing enum short names, added sync_server availability check.
Co-authored-by: Félix David <felixg.david@gmail.com>
Using context passed as argument Co-authored-by: Félix David <felixg.david@gmail.com>
Renamed `is_work_file_out_of_date` into `check_workfile_up_to_date` Co-authored-by: Félix David <felixg.david@gmail.com>
Fixed typo in comment Co-authored-by: Félix David <felixg.david@gmail.com>
…r StringProperty, applied check_workfile_up_to_date renaming, added no published workfile exception
a72b1f8
to
f369486
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did all testing with following findings...
-
when selecting Proceed option , OP menu shows out of date workfile warning and Publish validator will fail correctly...
-
when selecting "Download workfile" blender freezes instantly (I guess its because absence of Sync Server on my machine or?!
- When correct up-to-date workfile exists in asset workfile folder the win pop up doesnt show up as expected and also OP menu doesnt have that workfile warning and all behaves normally in Blender.
So my only concern is about that "download workfile" feature and how should I test it so its properly synced/downloaded... other than that all seems working fine.
local_workfile_path, | ||
) | ||
|
||
return local_workfile_path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, download_last_published_workfile
shouldn't return the local path but the last published wf which has been downloaded. The resolving of the local_workfile_path
must be put into another function and the copy made my the master process or into a utility function.
Co-authored-by: Félix David <felixg.david@gmail.com>
…to 'studio/ft_blender_workfile_out_of_date_handling' Enhancement: workfile out of date check no longer uses modification time. See merge request NORMAAL/openpype-for-normaal!28
calling the check for update operator. Also fixed missing return value in the check for update operator, when user selects "Quit blender".
Feature: Blender update button in OP menu
…being updated in blend file
Draft: This PR depends on this other PR: #4102
Description
Added warning and validator when workfile is out of file (in Blender).
The warning shows up as a props dialog that user cannot ignore as it will be reinvoked on cancel (So if they click anywhere outside of the dialog, it reappears). This props dialog has an enum to select what action to perform, then they can click "OK" to execute.
There are three actions provided:
There is also a variable set when the workfile has been detected as out of date. The validator checks it, so if user decide to proceed anyway, they won't be able to publish, unless they explicitly untick this box.
A warning sign will appear next to the OpenPype menu as well, and there is also an Update workfile button in the OpenPype menu, which is grayed out if the workfile is up to date.
This system however presents a limitation: As the workfile is determined as up to date, or out of date based on modification time, if it is modified and saved then it will always be considered up to date. This brings a few unwanted behavior. For instance if the users proceeds anyway, then closes blender, as the file is saved when making paths absolute, it will always be considered up to date from now on.
Testing
General
Download last workfile using the props dialog
Download last workfile
in the enum and click okUpdate workfile
in the OpenPype menu is grayed outQuit blender using props dialog
Quit blender
in the enum and click okProceed anyway
proceed anyway
in the enum and click okUpdate workfile
is available in the OpenPype menu and has the same behavior as theDownload last workfile
option in the props dialogWorkfile is up to date case
Update workfile
in the OpenPype menu is grayed out