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

Menu presets: automatic application. #4856

Merged
merged 2 commits into from
Jan 4, 2023
Merged

Menu presets: automatic application. #4856

merged 2 commits into from
Jan 4, 2023

Conversation

portnov
Copy link
Collaborator

@portnov portnov commented Jan 2, 2023

Addressed problem description

This is a continuation of #4841.

When we add a node, or just rearrange menu presets for some reason, user's index.yaml in datafiles directory still has to be updated. Before current PR, user had to press "Set" in preferences manually after each sverchok update. This is not obvious at all.

Options which were considered

  1. Original solution: copy the preset from sverchok directory to user's datafiles directory only at first sverchok startup, when there is no file in datafiles. - ❌ Rejected: the file will not be updated when we add a new node in Sverchok, or rearrange the default menu.
  2. Add a possibility to apply the selected menu preset automatically, at each blender startup. If enabled, at startup, copy the selected preset file from sverchok directory to user's datafiles. — ❌ Rejected: there is no sense in having index.yaml in user's datafiles, if that file is never actually used, as it is overwritten at each blender startup.
  3. ...If enabled, overwrite user's index.yaml only when the file in sverchok directory is newer than the one in datafiles. ❌ Rejected: not obvious for user, when his changes are overwritten, and there is no clear enough way to notify the user of such actions (message in the log will be most probably ignored).
  • Sub-option: preserve previous version of user's index.yaml as index.yaml.bak. ❌ Rejected: there is no clear or obvious way for user to merge his version of menu with the one from sverchok preset. And again, no clear way for user to know when his changes were overwritten.
  1. Options 1 - 3 were designed having in mind the problem that we can not refer to preferences at the moment when we are loading the menu. But, if we need an option to enable or disable menu file overwriting, then we will have to solve that problem somehow. But, once it is solved, we can look into preferences and use menu preset file name from there. So: If the option is enabled, then do not copy anything, just use menu preset file from sverchok directory. If it is disabled, then we copy the preset file once at first Sverchok startup, and when the user explicitly presses the button. This means that if the option is disabled, the user takes responsibility of updating his index.yaml. ✔️

Solution description

Add a possibility to use the menu preset file from sverchok directory directly, ignoring the file in user's datafiles.

If the feature is not enabled, it means the user takes the responsibility on updating his index.yaml manually.

Screenshot_20230104_115648

Technically, for this to work, I had to implement "lazy" initialization of "add node" menu, because in previous state, when I tried to call to preferences at the place where we located index.yaml, it appeared that preferences were not registered yet.

Preflight checklist

Put an x letter in each brackets when you're done this item:

  • Code changes complete.
  • Code documentation complete.
  • Documentation for users complete (or not required, if user never sees these changes).
  • Manual testing done.
  • Unit-tests implemented.
  • Ready for merge.

Add a possibility to apply the selected menu preset automatically, at
each blender startup.

This is aimed for cases when we added a new node (or just rearranged
something in the menu preset), but user's index.yaml in datafiles still
needs to be updated.

If the feature is not enabled, it means the user takes the
responsibility on updating his index.yaml manually.
option 1: use preset file from sverchok directory
option 2: use local copy of preset file. In this case, the user is
responsible for maintaining this copy.
@portnov portnov merged commit 9e6d8a5 into master Jan 4, 2023
@portnov portnov deleted the menu_presets_v2 branch January 4, 2023 07:49
Comment on lines +497 to +501
def get_add_node_menu():
global add_node_menu
if add_node_menu is None:
setup_add_menu()
return add_node_menu
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can't agree less that the function represents functionality of the module. If it returns Category instances than it can/should be a class method of the Category. The same is about setup_add_menu.

Also I don't like a laze concept here and in general - it makes things more complicated than they could be. It probably would be valid to use if initialization of the menu was really in arbitrary place in code. I assume you don't know from where the menu is called what leads to treating code as a black box / magic. From my knowledge this is not good approach.

I had a look at the places and it seems that the menu is only used in panels and operators what means that if you need the settings to be available during initialization of the menu it's possible to initialize the menu in the register function.

def register():
    add_menu = Category.generate_main_menu()
    add_menu.register()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will not have objections if you rewrite it according to your suggestions :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm too tired to rewrite somebody code.

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.

2 participants