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

Chore: Applications settings #16

Merged
merged 40 commits into from
Aug 20, 2024

Conversation

iLLiCiTiT
Copy link
Member

@iLLiCiTiT iLLiCiTiT commented Aug 5, 2024

Changelog Description

Refactored applications settings. Removed unnecessary settings from known applications like icon and label. Added option to define available applications and tools in settings instead of attributes.

Additional info

Using settings instead of attributes for available applications and tools is for now by default turned off, that is to avoid possible confusion on update and to keep opened doors if we find out possible issues with current settings.

The possible issues might be that tools cannot be set per task, but only per folder type and task type, which is more less granular approach, but at the same time easier to set up.

Removed settings for icon and label, known applications have both hardcoded in code.

Screenshot

image

UX enhancements

I would say that the labels in settings might need a change. As there is a lot of places where Applications and Tools is visible in UI.

Also I've used labels from settings labels in launcher, which might be wrong? (e.g. Maya became Autodesk Maya.) Removed company name from application labels.

Tools now have filtering by application and host name in tools definitions. It might be also added to profiles. But I think that current model is better, as the tool define application and host name, whereas tools profiles define context where it will be used, maybe?

NOTE: This PR requires minor version bump.

Testing notes:

  1. Change version in package.py to higher minor version (e.g. 0.3.0-settings). Version in PR already changed to 1.0.0-dev.1 .
  2. Run python ./create_package.py and upload the package to AYON server.
  3. Use the addon in a bundle and set the bundle as production (or current dev).
  4. With that done, you should see applications in launcher as before, using attributes.
  5. Copy settings from older version of applications addon.
  6. Now you should have ayon+settings://applications/project_applications/enabled and ayon+settings://applications/project_tools/enabled set to False They are both enabled by default, but copying of settings from older version should change it. Change it to True, so you can test new features.
  7. Create new profile in ayon+settings://applications/project_applications and define applications that will be shown. You can add different profiles for different task types or folder types.
  8. Launcher should show applications based on the settings profiles.
  9. Create new profile for tools in ayon+settings://applications/project_tools and define tools that should be used.
  10. Launch application and validate that correct tools were used based on the settings profiles.

Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

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

I have followed the testing steps and all seems working well!

All aplications been visible in the Launcher when using Attributes instead of Profiles - so all fine on this front.

Once switched to Profiles I was able to define aps by the profile and it correctly filtered out the aps in the Launcher too... as seen here (my profile and launcher state afterwards)

Screenshot 2024-08-06 110923

Screenshot 2024-08-06 110939

Speaking of Tools and Profiles it also works well and I was able to load tools just for a particular task type...in following example for rigging tasks and Mansur rigging tools

Screenshot 2024-08-06 113459

Screenshot 2024-08-06 113302

...of course both Aplications and Tools were tested for the occasions not falling in the filter set and also tested with Folder type filter...this was causing me some confusion at the begining as I was thinking that folder type folder being pretty much every type aka acting for all folder types but not :) ...after realizing that its actual type too all went well and predictably!

LGTM!

@LiborBatek
Copy link
Member

I would like to note something...just my thought (no deal breaker for Approval)

Once user switches to Aplication Profiles he basically have zero apps in the Launcher available then...which makes sense.

Questionable being if when user creates very first Profile without any filter set (aka matching all typesof folders and tasks) if he should define all of the aplications and tools manually or it could be theoretically kept empty meaning again all types of aplications and tools ...not sure about this one, maybe its correct atm. @iLLiCiTiT ?

Screenshot 2024-08-06 113843

@iLLiCiTiT
Copy link
Member Author

if he should define all of the aplications and tools manually or it could be theoretically kept empty meaning again all types of aplications and tools ...not sure about this one, maybe its correct atm.

100% no for me. You don't want to use all tools if they are empty (you usually don't want to use tools), and you may want to not show any applications for certain task types.

@LiborBatek
Copy link
Member

@iLLiCiTiT I got ya, makes sense...

one more to ask...wouldnt be lovely to use some sort of labeling for profiles? I mean header with name/label so admin can organise those profiles which creates??

@iLLiCiTiT
Copy link
Member Author

NOTE: There is internal discussion related to tools.

@iLLiCiTiT iLLiCiTiT mentioned this pull request Aug 12, 2024
Copy link

@MilaKudr MilaKudr left a comment

Choose a reason for hiding this comment

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

Tested - App filters + several profiles for different tasks types and works properly.
Didn't test Tool filters, because I don't use any tools, but assume that is also working.

@iLLiCiTiT
Copy link
Member Author

Changes made:

  • using attributes can be enabled/disabled for applications and tools separatelly
  • using attributes is by default disabled, but is auto-enabled when settings are converted from older version of addon
  • task path in tools filtering was replaced with folder path and task name

@iLLiCiTiT
Copy link
Member Author

image

Copy link
Contributor

@MustafaJafar MustafaJafar left a comment

Choose a reason for hiding this comment

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

Hello, testing this a user, I didn't dive in the code.
So, in a nut shell, please correct me.

  • We are going to filter applications using task types instead of project names.
  • We are going to filter tools using folder paths and task types or task names.

Tool filter didn't work on my side.


Here's my UX feedback.
I really like how dynamic it is.

  • Applications Filters:
    • Task Types: How can I add more types to Task Types ? from the default anatomy preset.
    • Applications: I really miss that search bar. it looked so cool in the project anatomy.
      image
  • Tools Filters
  • Folder paths: For the first glance, It looks unfamiliar. as I don't have a regex pattern that works with all of my test project.
    but, then I remembered that it can be overridden in project settings anyways!

@iLLiCiTiT
Copy link
Member Author

iLLiCiTiT commented Aug 19, 2024

We are going to filter applications using task types instead of project names.

It can be overriden per project. So, technically we're adding additional filter, and you set application at one place instead of 2.

We are going to filter tools using folder paths and task types or task names.

Yes.

Folder paths: For the first glance, It looks unfamiliar. as I don't have a regex pattern that works with all of my test project.
but, then I remembered that it can be overridden in project settings anyways!

Well, that's why it can be overriden per project.

@iLLiCiTiT
Copy link
Member Author

iLLiCiTiT commented Aug 19, 2024

I really miss that search bar. it looked so cool in the project anatomy.

Probably can be changed on frontend to add search for multiselection enumerator in settings @martastain @Innders ?

EDITED:
I actually have search bar there. Maybe update frontend?
image

Copy link
Contributor

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

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

I haven't reviewed the code and just want to review the functionality.

I think this can work nicely. It works better than I expected and seems like a sensible direction.

Some caveats:

  • This functionally is massively shitty without the AYON server copying studio and project settings on addon updates. Because this easily means everyone needs to manually go into all projects to tweak their project overrides, the whole time.. on every update.
  • The launcher is massively slow switching tasks when ayon+settings://applications/only_available is enabled and a lot of applications are enabled. We have this disabled in our productions, but it's something we should improve for any new users coming to AYON with the new default of "All applications" enabled by default?

@LiborBatek
Copy link
Member

I have to agree with @BigRoy that there should be option to have all aplications enabled by default in AYON (aka no need to set filter and manually cherrypick all apps first)

@iLLiCiTiT
Copy link
Member Author

This functionally is massively shitty without the AYON server copying studio and project settings on addon updates. Because this easily means everyone needs to manually go into all projects to tweak their project overrides, the whole time.. on every update.

Agree, I think it is worked on right now.

The launcher is massively slow switching tasks when ayon+settings://applications/only_available is enabled and a lot of applications are enabled. We have this disabled in our productions, but it's something we should improve for any new users coming to AYON with the new default of "All applications" enabled by default?

I've noticed this too, I though it's because I had running too many processes. Creating issue for it.

I have to agree with @BigRoy that there should be option to have all aplications enabled by default in AYON (aka no need to set filter and manually cherrypick all apps first)

That's what default does.

@iLLiCiTiT iLLiCiTiT merged commit 72d00af into develop Aug 20, 2024
1 check passed
@iLLiCiTiT iLLiCiTiT deleted the enhancement/AY-1301_Applications-per-tasktype branch August 20, 2024 08:55
@iLLiCiTiT iLLiCiTiT added the type: feature Adding something new and exciting to the product label Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature Adding something new and exciting to the product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants