Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 142 additions & 21 deletions client/ayon_nuke/api/lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import platform
import tempfile
import contextlib
from collections import OrderedDict
from collections import defaultdict, OrderedDict

import nuke
from qtpy import QtCore, QtWidgets
Expand Down Expand Up @@ -824,6 +824,17 @@ def get_view_process_node():
return duplicate_node(ipn_node)


def on_create_root_node():
if script_name():
log.info("Not a new Script. Skipping setting context settings.")
return

log.info("New Script. Setting context settings ...")

# Set context settings.
WorkfileSettings().set_context_settings()


def on_script_load():
"""Callback for ffmpeg support"""
if nuke.env["LINUX"]:
Expand Down Expand Up @@ -1453,6 +1464,60 @@ def create_backdrop(label="", color=None, layer=0,
bdn["note_font_size"].setValue(20)
return bdn

class ConfirmSetContextSettingMessageBox(QtWidgets.QDialog):
def __init__(self, change_description):
super().__init__()
layout = QtWidgets.QVBoxLayout()
self.setLayout(layout)
layout.addWidget(QtWidgets.QLabel(
"Would you like this script to have the following change:\n\n"
f"{change_description}\n\n"
"This is to match the correct settings for this context."
))
if not ASSIST:
self._always_check_check_box = QtWidgets.QCheckBox(
"Always check this Script's context settings on load\n"
"(if switched off, can be switched back on in Project Settings User tab)"
)
self._always_check_check_box.setChecked(True)
layout.addWidget(
self._always_check_check_box
)
self._always_check_check_box.toggled.connect(self._always_check_check_box_toggled)
Comment on lines +1477 to +1486
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not expose to the user to change the settings. Not every user has these permissions, or should even be allowed to care. That's for admin permissions, and even then you don't want anyone in production to accidentally misclick here either.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just for excluding the current script from context setting checks on load, rather than the global setting.

button_box = QtWidgets.QDialogButtonBox(
QtWidgets.QDialogButtonBox.StandardButtons.Yes|
QtWidgets.QDialogButtonBox.StandardButtons.No,
)
layout.addWidget(button_box)
button_box.accepted.connect(self.accept)
button_box.rejected.connect(self.reject)

def _always_check_check_box_toggled(self, is_checked):
root_node = nuke.root()
if "check_context_settings_on_load" not in root_node.knobs():
root_node.addKnob(nuke.Boolean_Knob(
"check_context_settings_on_load",
"Check Context Settings on Load",
True,
))
root_node["check_context_settings_on_load"].setValue(is_checked)

@classmethod
def ask_for_confirmation(cls, message):
project_settings = get_current_project_settings()
if not project_settings["nuke"]["general"]["check_context_settings_on_script_open"]:
return False

root_node = nuke.root()
# a specific script can be set to never check
if (
"check_context_settings_on_load" in root_node.knobs()
and not root_node["check_context_settings_on_load"].value()
):
return False

return cls(message).exec() == QtWidgets.QDialog.Accepted


class WorkfileSettings(object):
"""
Expand Down Expand Up @@ -2013,7 +2078,7 @@ def set_colorspace(self):
if read_clrs_inputs:
self.set_reads_colorspace(read_clrs_inputs)

def reset_frame_range_handles(self):
def reset_frame_range_handles(self, requires_confirmation=False):
"""Set frame range to current folder."""

if "attrib" not in self._task_entity:
Expand Down Expand Up @@ -2052,22 +2117,64 @@ def reset_frame_range_handles(self):
frame_start_handle = frame_start - handle_start
frame_end_handle = frame_end + handle_end

self._root_node["lock_range"].setValue(False)
self._root_node["fps"].setValue(fps)
self._root_node["first_frame"].setValue(frame_start_handle)
self._root_node["last_frame"].setValue(frame_end_handle)
self._root_node["lock_range"].setValue(True)

# update node graph so knobs are updated
update_node_graph()

frame_range = '{0}-{1}'.format(frame_start, frame_end)

for node in nuke.allNodes(filter="Viewer"):
node['frame_range'].setValue(frame_range)
node['frame_range_lock'].setValue(True)
node['frame_range'].setValue(frame_range)
node['frame_range_lock'].setValue(True)
knob_name_current_new_value_triplets_by_node = defaultdict(list)
for knob_name_new_value_pairs, nodes in (
(
(
("lock_range", False),
("fps", fps),
("first_frame", frame_start_handle),
("last_frame", frame_end_handle),
("lock_range", True),
),
(self._root_node,),
),
(
(
("frame_range", frame_range),
("frame_range_lock", True),
),
nuke.allNodes(filter="Viewer"),
)
):
for node in nodes:
for knob_name, new_value in knob_name_new_value_pairs:
current_value = node[knob_name].value()
if current_value != new_value:
knob_name_current_new_value_triplets_by_node[node].append(
(knob_name, current_value, new_value)
)

if knob_name_current_new_value_triplets_by_node:
if not requires_confirmation or ConfirmSetContextSettingMessageBox.ask_for_confirmation(
"\n\n".join(
"{}:\n{}".format(
node.name(),
"\n".join(
f"{knob_name}: {current_value} -> {new_value}" for knob_name, current_value, new_value in knob_name_current_new_value_triplets
)
) for node, knob_name_current_new_value_triplets in knob_name_current_new_value_triplets_by_node.items()
)
):
was_node_graph_updated = False
for node, knob_name_current_new_value_triplets in knob_name_current_new_value_triplets_by_node.items():
# Root node will be first.
# Based on how the code was when I found it,
# `update_node_graph` needs calling between
# root node and viewers - splidje
if not was_node_graph_updated and node != self._root_node:
# update node graph so knobs are updated
update_node_graph()
was_node_graph_updated = True
for knob_name, _, new_value in knob_name_current_new_value_triplets:
node[knob_name].setValue(new_value)

# in case no Viewers needed knob changes
if not was_node_graph_updated:
# update node graph so knobs are updated
update_node_graph()
Comment on lines +2150 to +2177
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this suddenly need all this complexity? This is very hard to read and grasp.

Admittedly - all we're really changing is:

  • Before reset_frame_range_handles gets called, ask for confirmation of changes.
  • Or better, before any of the reset_..._ gets called handle confirmation.

To know whether we need to ask the user we need two things:

    1. Are we checking?
    1. Is there anything to change?

The way I see it is what we should do is separate the logic of getting the current and expected state from the setting the state.

# overly simplified psuedocode
should_check = ...[settings][check][blabla]
if not should_check:
    return

current, expected = get_current_state()
if current == expected:
    return

if headless:
    # log difference
    log.warning("Values mismatch: {current} != {headless}")
    return

accepted = prompt_user(current, expected):
if accepted:
    set_state(expected)

But in short, the setting of the values should not be changing or get more complicated at all?
We just end up having a library set_frame_range, set_colorspace and set_frame_range functions that will end up being called, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, makes sense to make this neater and more readable.

Above psuedocode makes sense to me.

When I find the time I'll do another sweep and make these changes.


if not ASSIST:
set_node_data(
Expand All @@ -2084,7 +2191,7 @@ def reset_frame_range_handles(self):
"updating custom knobs..."
)

def reset_resolution(self):
def reset_resolution(self, requires_confirmation=False):
"""Set resolution to project resolution."""
log.info("Resetting resolution")
project_name = get_current_project_name()
Expand All @@ -2106,6 +2213,20 @@ def reset_resolution(self):
log.error(msg)
nuke.message(msg)

current_format = nuke.root()["format"].value()
if requires_confirmation and any((
current_format.width() != format_data["width"],
current_format.height() != format_data["height"],
current_format.pixelAspect() != format_data["pixel_aspect"],
)):
if not ConfirmSetContextSettingMessageBox.ask_for_confirmation(
"Format:\n"
f"{current_format.width()}x{current_format.height()} {current_format.pixelAspect()}"
" -> "
f'{format_data["width"]}x{format_data["height"]} {format_data["pixel_aspect"]}'
):
return

existing_format = None
for format in nuke.formats():
if format_data["name"] == format.name():
Expand Down Expand Up @@ -2148,10 +2269,10 @@ def make_format_string(self, **kwargs):
"{name}".format(**kwargs)
)

def set_context_settings(self):
self.reset_resolution()
self.reset_frame_range_handles()
# add colorspace menu item
def set_context_settings(self, requires_confirmation=False):
self.reset_resolution(requires_confirmation)
self.reset_frame_range_handles(requires_confirmation)
# todo: No need to check confirmation for color?
self.set_colorspace()
Comment on lines +2272 to 2276
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will start prompting once for every thing it needs to reset - we'll definitely want to avoid that, especially if we may add more over time. I'd rather present the user of all things invalid in current context, and then tell them about all. Not a massive issue, but may as well tackle now.

Also, when Nuke is running in headless mode - there's no such things as showing user a pop-up. In that case it should just log the fact that it's not the right values and allow it to continue without automatically fixing it (that's my personal preference though; @jakubjezek001 ?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good point about headless mode. I'll fix that.

With one pop-up showing all required changes, are their options "all or nothing" or should that pop-up let them control which to set, and if so how granular should it be?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With one pop-up showing all required changes, are their options "all or nothing" or should that pop-up let them control which to set, and if so how granular should it be?

Probably a single prompt with checkboxes:

  • Fix resolution
  • Fix frame range
  • Fix colorspace

With default having all checkboxes enabled? and the prompt can be confirmed or cancelled.


def set_favorites(self):
Expand Down
17 changes: 10 additions & 7 deletions client/ayon_nuke/api/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
check_inventory_versions,
set_avalon_knob_data,
read_avalon_data,
on_create_root_node,
on_script_load,
dirmap_file_name_filter,
add_scripts_menu,
Expand Down Expand Up @@ -169,14 +170,14 @@ def add_nuke_callbacks(project_settings: dict = None):
project_settings = get_current_project_settings()

nuke_settings = project_settings["nuke"]
workfile_settings = WorkfileSettings()

# Set context settings.
nuke.addOnCreate(
workfile_settings.set_context_settings, nodeClass="Root")
nuke.addOnCreate(on_create_root_node, nodeClass="Root")

# adding favorites to file browser
nuke.addOnCreate(workfile_settings.set_favorites, nodeClass="Root")
nuke.addOnCreate(
lambda: WorkfileSettings().set_favorites(),
nodeClass="Root",
)

# template builder callbacks
nuke.addOnCreate(start_workfile_template_builder, nodeClass="Root")
Expand All @@ -188,8 +189,10 @@ def add_nuke_callbacks(project_settings: dict = None):
nuke.addOnScriptLoad(check_inventory_versions)
nuke.addOnScriptSave(check_inventory_versions)

# set apply all workfile settings on script load and save
nuke.addOnScriptLoad(WorkfileSettings().set_context_settings)
# set apply all workfile settings on script load
nuke.addOnScriptLoad(
lambda: WorkfileSettings().set_context_settings(requires_confirmation=True)
)

if nuke_settings["dirmap"]["enabled"]:
log.info("Added Nuke's dir-mapping callback ...")
Expand Down
2 changes: 2 additions & 0 deletions client/ayon_nuke/api/workio.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def save_file(filepath):


def open_file(filepath):
from .lib import WorkfileSettings

def read_script(nuke_script):
if not ASSIST:
Expand All @@ -31,6 +32,7 @@ def read_script(nuke_script):
nuke.Root()["name"].setValue(nuke_script)
nuke.Root()["project_directory"].setValue(os.path.dirname(nuke_script))
nuke.Root().setModified(False)
WorkfileSettings().set_context_settings(requires_confirmation=True)
else:
nuke.scriptOpen(nuke_script)

Expand Down
4 changes: 2 additions & 2 deletions client/ayon_nuke/plugins/create/convert_legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
get_avalon_knob_data,
NODE_TAB_NAME,
)
from ayon_nuke.api.plugin import convert_to_valid_instaces
from ayon_nuke.api.plugin import convert_to_valid_instances

import nuke

Expand Down Expand Up @@ -50,6 +50,6 @@ def find_instances(self):

def convert(self):
# loop all instances and convert them
convert_to_valid_instaces()
convert_to_valid_instances()
# remove legacy item if all is fine
self.remove_convertor_item()
3 changes: 3 additions & 0 deletions server/settings/general.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,16 @@ class MenuShortcut(BaseSettingsModel):
class GeneralSettings(BaseSettingsModel):
"""Nuke general project settings."""

check_context_settings_on_script_open: bool = SettingsField(True, title="Check Context Settings on Script Open")

menu: MenuShortcut = SettingsField(
default_factory=MenuShortcut,
title="Menu Shortcuts",
)


DEFAULT_GENERAL_SETTINGS = {
"check_context_settings_on_script_open": True,
"menu": {
"create": "ctrl+alt+c",
"publish": "ctrl+alt+p",
Expand Down
Loading