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

Unreal context menus #48

Merged
merged 22 commits into from
Jul 24, 2023
Merged

Conversation

JLHayde
Copy link
Contributor

@JLHayde JLHayde commented Jul 17, 2023

An update that adds an option to setup menus in the context menu of an asset

Copy link
Owner

@hannesdelbeke hannesdelbeke left a comment

Choose a reason for hiding this comment

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

This looks really cool, I do have some concerns regarding this feature only wokring in Unreal.
Ideally the unimenu interface for all apps only adds new args if these are supported in all, or most apps. (see the features overview)
Let's see if we can make this feature a bit more generic.

unimenu/apps/unreal.py Outdated Show resolved Hide resolved
unimenu/apps/_abstract.py Outdated Show resolved Hide resolved
unimenu/apps/unreal.py Outdated Show resolved Hide resolved
unimenu/apps/unreal.py Outdated Show resolved Hide resolved
Copy link
Owner

@hannesdelbeke hannesdelbeke left a comment

Choose a reason for hiding this comment

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

when testing out the sample code on your branch, i get an error

import unreal_context_menu
LogPython: Error: Traceback (most recent call last):
LogPython: Error:   File "<string>", line 1, in <module>
LogPython: Error:   File "C:\Users\hanne\OneDrive\Documents\Unreal Projects\MyProject\Content\Python\Lib\site-packages\shiboken2\files.dir\shibokensupport\__feature__.py", line 142, in _import
LogPython: Error:     return original_import(name, *args, **kwargs)
LogPython: Error:   File "C:\Users\hanne\OneDrive\Documents\Unreal Projects\MyProject\Content\Python\Lib\site-packages\unreal_context_menu.py", line 72, in <module>
LogPython: Error:     app_node = node.setup(backlink=False)
LogPython: Error:   File "C:\Users\hanne\OneDrive\Documents\Unreal Projects\MyProject\Content\Python\Lib\site-packages\unimenu\apps\unreal.py", line 26, in setup
LogPython: Error:     super().setup(parent_app_node=parent_app_node, backlink=backlink)
LogPython: Error:   File "C:\Users\hanne\OneDrive\Documents\Unreal Projects\MyProject\Content\Python\Lib\site-packages\unimenu\apps\_abstract.py", line 214, in setup
LogPython: Error:     self.app_node = self._setup_sub_menu(parent_app_node=parent_app_node)
LogPython: Error:   File "C:\Users\hanne\OneDrive\Documents\Unreal Projects\MyProject\Content\Python\Lib\site-packages\unimenu\apps\unreal.py", line 40, in _setup_sub_menu
LogPython: Error:     return parent_app_node.add_sub_menu(
LogPython: Error: AttributeError: 'NoneType' object has no attribute 'add_sub_menu'

@JLHayde
Copy link
Contributor Author

JLHayde commented Jul 18, 2023

when testing out the sample code on your branch, i get an error
...

I've just tested on a couple projects, and also tested with the branch pulled from git and haven't got the error.
which UE version are you using? I'm currently on 5.2

@hannesdelbeke
Copy link
Owner

hannesdelbeke commented Jul 18, 2023

I've just tested on a couple projects, and also tested with the branch pulled from git and haven't got the error. which UE version are you using? I'm currently on 5.2

i'm on 5.0.2. i tested another sample demo any_dcc_test and that one works (but with a bug #50)

@JLHayde
Copy link
Contributor Author

JLHayde commented Jul 18, 2023

i've run a the sample on another machine and with 5.0.3 and I cant seem to get the error
image

The only thing that could be different is that your running from ..../Unreal Projects\MyProject\Content\Python\Lib\site-packages and in my launcher I'm adding to PYTHONPATH. but that shouldn't make a difference

Edit:
i have manged to get the error by changing 'root_menu' to something like ContentBrowser.AssetContextMenu.Sta
in the sample. this would be correct as that menu doesn't exists. maybe this menu has not been registered at the time the the sample was run?

@hannesdelbeke
Copy link
Owner

hannesdelbeke commented Jul 19, 2023

  1. just retried today and works now 🤷
  2. double checked that wasn't on your branch. (it added the menu to the top menu)
    after swapping to yours it breaks again. same error
  3. got it to work, needed to go to a folder with assets, and right click the menu before running the script. this ensures the root menu exists.
    now it runs correctly and adds the menu to the right click menu.

image

@hannesdelbeke
Copy link
Owner

hannesdelbeke commented Jul 19, 2023

FYI, i suggest to split this up in separate PRs, i noticed you also integrated icon and separator.
It's easier for me to review 1 feature at a time.
deffo want to integrate this though

  • with the parentpath change, i think we don't need the rootmeno / context menu change from this PR anymore.

@JLHayde
Copy link
Contributor Author

JLHayde commented Jul 20, 2023

Sure ill make another pr with just the icons.

Its strange that it does not work without opening the context menu first. I'm running the code on init_unreal.py so its happening at the earliest it can. I'll look into the extend method. and also exclude the separator and icon changes from this pull

@hannesdelbeke
Copy link
Owner

ye by default unreal starts in site package folder, which has no assets.
the context menu doesn't has the section we are adding to either
image

what is still left from this PR? since we can use parent_path we dont need root menu anymore
and you ll move icons and separator to a new PR.

@JLHayde
Copy link
Contributor Author

JLHayde commented Jul 20, 2023

The 'menu_section' kwarg will be needed so the user can specify in the which area of the context menu to put the menu/action

@hannesdelbeke
Copy link
Owner

The 'menu_section' kwarg will be needed so the user can specify in the which area of the context menu to put the menu/action

Ah yes, that goes together with the separator I believe

set the target_section_name on the node .data so were not doing check on each add method

excluded icon and seperator logic.

Updated sample to reflect these changes
…_menus

# Conflicts:
#	dev/unimenu_samples/menu_screen_unreal_context.png
#	dev/unimenu_samples/unreal_context_menu.py
#	unimenu/apps/unreal.py
@JLHayde
Copy link
Contributor Author

JLHayde commented Jul 24, 2023

Pulled your latest changes and now set the menu_section in data from the kwarg. so this can be re-used later in the icons and separator fix

dev/unimenu_samples/menu_screen_unreal_context.png Outdated Show resolved Hide resolved
unimenu/apps/unreal.py Outdated Show resolved Hide resolved
unimenu/apps/unreal.py Outdated Show resolved Hide resolved
unimenu/apps/unreal.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants