-
Notifications
You must be signed in to change notification settings - Fork 36
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
Load and manage products from a library project #1005
base: develop
Are you sure you want to change the base?
Load and manage products from a library project #1005
Conversation
I'm afraid you chose easy way, which does complicate things a lot. Once we decide we want to work with different projects on containers, we have to work with that in UI and api too. I'll use one example All places using that method have access to container items, so they also have access to correct project name. You should use that as advantage. |
version_items_by_product_id = self._controller.get_version_items( | ||
product_ids | ||
product_ids, project_names | ||
) |
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 this is the wrong approach because we'd need to know which product id belongs to which project. What @iLLiCiTiT mentioined before is to separate the query to self._controller.get_version_items()
to have separate calls per project. So above, we'd get the product ids per project and then here we would do:
version_items_by_product_id = self._controller.get_version_items(project_name, product_id)
As stated before - as far as I know on the backend nothing is keeping the database from having a product entity with the same id
across different projects. Is that correct @martastain ? (It's unlikely by just creating products to happen, but when cloning e.g. projects I suppose it may very well share the same ids?) As such, we should still make sure to be explicit about which project each entry belongs to.
The same goes for version_items_by_product_id
dict. That should technically also be "per project" because otherwise product_id
may overlap accidentally.
@iLLiCiTiT thoughts?
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.
Yes
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.
Aside of their being plenty of code style notes I gave this a quick test run - and it seems to work fine.
Opening the scene inventory with containers from another project showed these entities just fine. However, visually there's NO clue whatsoever in the scene inventory that these are from another project.
I wonder whether it warrants somehow highlighting them. I was thinking of adding a project_name
column to the Scene Inventory UI and potentially highlighting the name AYON blue using a delegate if the project name is not the current project.
But this is looking really nice.
I tested it with this PR: ynput/ayon-maya#176
…ainer_data and make sure the scene inventory won't show Entity N/A as name and version
It should be somewhat functional. Please re-test and report bugs. QUESTION: What to do about switch dialog? Even I'm not able to make it work across projects, and it will be PITA to allow it for different than current project. |
But we need to think about if there is any case of which same folder path and task name across different project.... |
Changelog Description
This PR is to support loading assets from the library project while the users can update and set versions with the containers from the loaded assets in the scene inventory tools. In the DCC hosts, you need to add the
project_name
as part of the container data, otherwise it will use the current project name instead(which results to the Entity N/A shown in the #983)Resolve #983
Additional info
Make sure using the self.filepath_from_context(context) for the updating function in the loaders.
Testing notes: