-
Notifications
You must be signed in to change notification settings - Fork 49
Launcher tool: Use webactions #1226
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
base: develop
Are you sure you want to change the base?
Conversation
Can you elaborate on what this change adds as functionality, or why we need this change to begin with?
I'd say that each of those are quite problematic and are really only worth it if it provides us massive benefits another way. |
This PR was created to trigger discussion. It is a must have for planned per-project bundle feature.
Something that can be resolved, as mentioned in description.
Something that needs discussion, and I guess can be changed if needed.
I can't tell how much is that a must have feature, again one of reasons why this PR is created. The feature is also reason that applications action is handled by ayon-core instead of applications addon, which is not ideal either. There is issue #1178 for that.
True. |
This might need more explanation - why does it need to go through 'web scheme url' etc. Why can't we run it through own mechanism? Anyway, I'd merge this to a temporary dev branch dedicated to moving to web actions or per project bundles, instead of to E.g. we use "Skip opening workfile" a lot all over the place - and I can imagine others would as well. Similarly without the grouped applications, launcher would really grow unusable for us. We have e.g. a few houdini versions in some projects, where each version has both a FX and Core variant. So that one button could easily become say six entries, etc. Grouping them is really helpful and UX would otherwise be way worse. |
Ok, thank you for your input.
It doesn't make sense to push it to separate branch, the issues won't get resolved in different branch, it would only get merged to develop at some point without even looking into them... But don't worry this PR probably won't get merged soon, as I mentioned, this needs a discussion. And I'm not here to defend the changes in the PR. You don't like it, I get it, but that won't help to resolve anything. At some point we will need to do it. The PR is opened here to see what it does, how it behaves and what we need to change first.
The actions discovery has to go through webactions, because server does know about all addon versions assigned to a project and has access to it's code, so at the moment is the only entity in the entire system that is capable of telling launcher tool what actions should be shown for that specific context based on bundle, project bundle and variant. And because the webaction also defines what should actually happen, it should be processed the same way as webactions are processed in browser. As I mentioned above, that can be resolved to do it differently for ayon launcher scheme if it's necessary I just don't want to burn hours on something that might not be needed in future, so please stop being so aggresive about this PR, that really doesn't help. |
This may have been miscommunication - I did not intend to be aggressive at this all, merely expressing my opinion about the downsides and not knowing the full extent of why it's needed. Even with this explanation I'm still not convinced why we can't just have a call to the server that tells us what to do and end up doing it directly instead of requiring to go through the scheme url. Even if all we'd need from the server is a URL to trigger, I feel like we can request that 'url' or 'identifier' directly from the server and run it directly? Or is it because the running of the action would need to go through completely different launchers? Like project X may use launcher v1.1 but project Y uses launcher v1.2? |
@@ -4,6 +4,7 @@ description="AYON core addon." | |||
|
|||
[tool.poetry.dependencies] | |||
python = ">=3.9.1,<3.10" | |||
markdown = "^3.4.1" |
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.
Just to clarify, the dependency is already in AYON launcher, so it is not necessary to create new dependency package.
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.
Tested in Nuke and PS
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.
Some last time code comments. It works otherwise.
|
||
""" | ||
pass | ||
|
||
@abstractmethod | ||
def set_application_force_not_open_workfile( | ||
self, project_name, folder_id, task_id, action_ids, enabled | ||
def trigger_webaction( |
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 has so many arguments. could we wrap some of them to context dataclass? project_name
, folder_id
, all those are used around often.
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.
Launching Nuke gave me this error
Traceback (most recent call last):
File "E:\Ynput\ayon-nuke\client\ayon_nuke\startup/menu.py", line 2, in <module>
from ayon_nuke.api import NukeHost
File "C:\Program Files\Nuke14.0v5\pythonextensions\site-packages\shiboken2\files.dir\shibokensupport\__feature__.py", line 142, in _import
return original_import(name, *args, **kwargs)
File "E:\Ynput\ayon-nuke\client\ayon_nuke\api\__init__.py", line 1, in <module>
from .workio import (
File "C:\Program Files\Nuke14.0v5\pythonextensions\site-packages\shiboken2\files.dir\shibokensupport\__feature__.py", line 142, in _import
return original_import(name, *args, **kwargs)
File "E:\Ynput\ayon-nuke\client\ayon_nuke\api\workio.py", line 5, in <module>
from .utils import is_headless
File "C:\Program Files\Nuke14.0v5\pythonextensions\site-packages\shiboken2\files.dir\shibokensupport\__feature__.py", line 142, in _import
return original_import(name, *args, **kwargs)
File "E:\Ynput\ayon-nuke\client\ayon_nuke\api\utils.py", line 11, in <module>
from ayon_core.tools.utils import show_message_dialog
File "C:\Program Files\Nuke14.0v5\pythonextensions\site-packages\shiboken2\files.dir\shibokensupport\__feature__.py", line 142, in _import
return original_import(name, *args, **kwargs)
File "E:\Ynput\ayon-core\client\ayon_core\tools\utils\__init__.py", line 2, in <module>
from .widgets import (
File "C:\Program Files\Nuke14.0v5\pythonextensions\site-packages\shiboken2\files.dir\shibokensupport\__feature__.py", line 142, in _import
return original_import(name, *args, **kwargs)
File "E:\Ynput\ayon-core\client\ayon_core\tools\utils\widgets.py", line 10, in <module>
import markdown
File "C:\Program Files\Nuke14.0v5\pythonextensions\site-packages\shiboken2\files.dir\shibokensupport\__feature__.py", line 142, in _import
return original_import(name, *args, **kwargs)
File "E:\Ynput\ayon-launcher\.venv\lib\site-packages\markdown\__init__.py", line 42, in <module>
from .core import Markdown, markdown, markdownFromFile
File "C:\Program Files\Nuke14.0v5\pythonextensions\site-packages\shiboken2\files.dir\shibokensupport\__feature__.py", line 142, in _import
return original_import(name, *args, **kwargs)
File "E:\Ynput\ayon-launcher\.venv\lib\site-packages\markdown\core.py", line 28, in <module>
from .preprocessors import build_preprocessors
File "C:\Program Files\Nuke14.0v5\pythonextensions\site-packages\shiboken2\files.dir\shibokensupport\__feature__.py", line 142, in _import
return original_import(name, *args, **kwargs)
File "E:\Ynput\ayon-launcher\.venv\lib\site-packages\markdown\preprocessors.py", line 30, in <module>
from .htmlparser import HTMLExtractor
File "C:\Program Files\Nuke14.0v5\pythonextensions\site-packages\shiboken2\files.dir\shibokensupport\__feature__.py", line 142, in _import
return original_import(name, *args, **kwargs)
File "E:\Ynput\ayon-launcher\.venv\lib\site-packages\markdown\htmlparser.py", line 41, in <module>
spec.loader.exec_module(htmlparser)
AttributeError: 'zipimporter' object has no attribute 'exec_module'
Launched as usual:
- Houdini
- Maya
- Tray Publisher
- Blender
- Cinema 4d
- After Effects
- Unreal
- Terminal Main
- 3DE
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, overall - this seems to work for me.
I'm a bit sad about two things:
-
The "Open Last Workfile" override toggle now isn't task-specific (I sure hope it's still User-specific?) which is very annoying, because there are cases where users wanted it enabled by default but only certain tasks they wanted to disable it. Now it's always on or off - and without the visual dot overlay it's actually quite annoying that you don't know what it's currently set to.
- I believe previously the override of "open last workfile" was also for the whole application group, and not per application version.
- What's the stopper for exposing that overlay icon again?
-
I'm quite sad that the same applications aren't combined/grouped anymore. We have a separate icon for Houdini FX versus Houdini Core configured in production so the user can pick which license they want - with two houdini releases enabled in a project this suddenly means you'll be seeing 4 houdini icons instead of one.
For reference - here's what it looks like for me on Win11:
Other question
- Something that did stand out for me - is that e.g. Silhouette app is listed with a lowercase
s
in this PR. (see screenshots) - Why is the Terminal now listed first - instead of after the other applications like it was before?
- 🐌 I also noticed that clicking through the folder list for whatever reason is much slower than before (if you hold and drag over the folders for example it's now much less responsive in switching between folders than it was before?) It's quite slow compared to the old.
@@ -131,6 +136,29 @@ def __init__(self, *args, **kwargs): | |||
viewport.setPalette(filter_palette) | |||
|
|||
|
|||
class MarkdownLabel(QtWidgets.QLabel): |
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.
Is there a reason why this doesn't suffice?
label = QtWidgets.QLabel("Hello **Markdown**")
label.setTextFormat(QtCore.Qt.MarkdownText)
Changelog Description
Show webactions in launcher tool.
Additional info
This PR changes basic functionality of launcher tool. The tools shows webactions the same way AYON WebUI does. The tool still does support local actions. With this PR only applications actions were replaced with webactions.
Messages overlay
Replaced existing footer message with message overlay.
Downsides
We should probably use category in the UI somehow.
Applications actions lost functionality to "Skip open last workfile" checkbox because webactions do not support such modifications.Because webactions don't have "main label" and "variant label" some actions are not grouped as were before. For example 3 version of Nuke would show 3 actions instead of one action with 3 sub-menu actions for each variant.If launcher did not register scheme handler webactions cannot be launched which might be an issue for developers who usually don't have installed launcher. Also does not allow to run launcher from code. The launcher scheme should be probably handled directly without user's scheme handler registered for dev mode?Why we need this change?
This change is requirement for future "per-project bundle" feature where actions may differ based on selected project.
Testing notes: