Skip to content

Conversation

@mkolar
Copy link
Member

@mkolar mkolar commented Mar 14, 2019

This PR add new functionality to asset switcher GUI in cbsceneinventory and improves in user experience in general.

Changes:

  • Dropdown are filtered left to right. If I select an asset on the left, I only see subsets from that particular asset. Same goes for representations. When no new asset is selected, only subsets from currently loaded asset are visible
  • Added visual cues indicating whether currently selected combination of asset, subset and representation exists and is therefore importable. For example if my loaded subset is called modelMain but the asset I selected in the switcher doesn't have this subset, I won't be able to switch it and the subset dropdown will be coloured in red indicating I have to choose a new subset name.
  • When trying to switch more assets at the same time, the GUI shows an intersection of their available subsets by default. So if too assets don't share any subset name, we cannot change them in one go, unless we're replacing them with a new asset.

everything else should work identically so searching and the actual switch mechanism wasn't touched.

Here's the look of it
Mar 14 2019 8_12 PM - Edited

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 14, 2019

Looking sweet. Will give this a roll some day soon to test it out and look at the code.

Great work!

@BigRoy
Copy link
Collaborator

BigRoy commented Jun 5, 2019

@iLLiCiTiT I'll be looking into this one (finally!) today and test the ins- and outs. Can you confirm that this is still in the working state as you are using it? Or do you internally have brought other updates/fixes to this interface?

@iLLiCiTiT
Copy link
Contributor

I think this is final version for this issue(if you won't find bugs). Only other update is LOD support, that requires new issue and probably not my implementation. It is very messy dirty code which works, but any future edit will be hell...
Also there is #377 where @davidlatwe modified this and seems to be working :)

@BigRoy
Copy link
Collaborator

BigRoy commented Jun 5, 2019

Had a quick glance at this PR and there is an issue with it where it disallows a certain switching where previously it would allow it. Specifically, the idea about the switch UI is that you can select any number of different types of content and switch all to a different shot - even where some might not be able to do the same switch.

So say I load:

- pointcacheDefault (shot 1)
- pointcacheBackground (shot 1)
- cameraDefault (shot 1)

And shot two only has pointcacheDefault and cameraDefault but does not have pointcacheBackground.

Then the idea is that the user could still select all things in the Scene Inventory, click Switch Asset UI and switch to shot 2 - with only pointcacheBackground failing. However, with this PR whenever any one fails of the input containers then you cannot perform the switch... which is somewhat fine if it weren't extremely hard to track down which of the items in the selection actually fail to switch.

My proposal would be to merge this in the current state - however create a new issue that describes even better UX. A quick mockup here:

switch_ui_mockup

It could potentially even show to which version it will end up resolving the switch


Aside of that I noticed this version to be slightly slower, but nothing crazy that grinds it to a halt... :) just wanted to mention it.

@BigRoy
Copy link
Collaborator

BigRoy commented Jul 4, 2019

I believe we should merge this and leave this for a separate issue as created: #389

Merge?

@tokejepsen
Copy link
Collaborator

+1 for merge

@davidlatwe
Copy link
Collaborator

Let's merge it :D

@davidlatwe davidlatwe merged commit 447abf1 into getavalon:master Jul 4, 2019
@BigRoy BigRoy mentioned this pull request Jul 26, 2019
@mkolar mkolar deleted the cleanup/PYPE-222_switch_asset_GUI branch December 19, 2019 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants