PyAPI: postpone loading add-ons until after key-maps have been defined #110092

Campbell Barton · 2023-07-14T10:19:13+02:00

Campbell Barton commented

2023-07-14 10:19:13 +02:00

Loading add-ons after key-maps resolves a problem where add-ons would setup shortcuts before Blender had created the key-maps.
Making add-ons have to declare the key-maps using region & window types matching Blender's internal values.

This PR addresses a pitfall pointed out in #110030.

Loading add-ons after key-maps resolves a problem where add-ons would setup shortcuts before Blender had created the key-maps. Making add-ons have to declare the key-maps using region & window types matching Blender's internal values. ---- This PR addresses a pitfall pointed out in #110030.

Campbell Barton requested review from Demeter Dzadik 2023-07-14 10:19:45 +02:00

Campbell Barton requested review from Brecht Van Lommel 2023-07-14 10:19:55 +02:00

Campbell Barton referenced this pull request

2023-07-14 10:21:51 +02:00

PyAPI: New KeyConfig Utils #110030

Campbell Barton commented

2023-07-14 10:22:11 +02:00

@blender-bot build

Demeter Dzadik commented

2023-07-14 12:01:21 +02:00

I tried the patch but I don't think it's working, or I'm missing the idea.

Here's what I tried:

# Make sure patch is applied by importing a function that doesn't exist without the patch.
from bpy.utils import load_scripts_extended

import bpy
kc = bpy.context.window_manager.keyconfigs.addon
km = kc.keymaps.get('Text') # Returns None, like before.

# Try registering without caring about region_type or space_type. (I thought this was the purpose of the patch)
if not km:
    km = kc.keymaps.new('Text') # Don't specify space_type, which should be 'TEXT_EDITOR'.
kmi = km.keymap_items.new('text.run_script', type='Q', value='PRESS', alt=True) # Runs without error. Hotkey does not work.

# Try the old way.
km = kc.keymaps.new('Text', space_type='TEXT_EDITOR', region_type='WINDOW')
kmi = km.keymap_items.new('text.run_script', type='Q', value='PRESS', shift=True) # Hotkey will work.

print("Done")

#

And what I found is that with the patch applied, the script actually needs to run twice, in order for the Shift+Q hotkey to start working, which is not the case without the patch.

I tried the patch but I don't think it's working, or I'm missing the idea. Here's what I tried: ```python # Make sure patch is applied by importing a function that doesn't exist without the patch. from bpy.utils import load_scripts_extended import bpy kc = bpy.context.window_manager.keyconfigs.addon km = kc.keymaps.get('Text') # Returns None, like before. # Try registering without caring about region_type or space_type. (I thought this was the purpose of the patch) if not km: km = kc.keymaps.new('Text') # Don't specify space_type, which should be 'TEXT_EDITOR'. kmi = km.keymap_items.new('text.run_script', type='Q', value='PRESS', alt=True) # Runs without error. Hotkey does not work. # Try the old way. km = kc.keymaps.new('Text', space_type='TEXT_EDITOR', region_type='WINDOW') kmi = km.keymap_items.new('text.run_script', type='Q', value='PRESS', shift=True) # Hotkey will work. print("Done") # ``` And what I found is that with the patch applied, the script actually needs to run twice, in order for the Shift+Q hotkey to start working, which is not the case without the patch.

Campbell Barton changed title from ~~PyAPI: postpone loading add-ons until after key-maps have been defined~~ to WIP: PyAPI: postpone loading add-ons until after key-maps have been defined

2023-07-14 12:09:51 +02:00

Campbell Barton commented

2023-07-14 12:55:00 +02:00

Updated the patch with an experimental function to create a key-map based on the default:

km = kc.keymaps.get('Text') # Returns None, like before.

# Try registering without caring about region_type or space_type. (I thought this was the purpose of the patch)
if not km:
    km = kc.keymaps.new('Text') # Don't specify space_type, which should be 'TEXT_EDITOR'.

Can be replaced with

km = kc.keymaps.ensure_from_default("Text")

The key-map uses settings (space type, region type etc...) from bpy.context.window_manager.keyconfigs.default.

Updated the patch with an experimental function to create a key-map based on the default: ``` km = kc.keymaps.get('Text') # Returns None, like before. # Try registering without caring about region_type or space_type. (I thought this was the purpose of the patch) if not km: km = kc.keymaps.new('Text') # Don't specify space_type, which should be 'TEXT_EDITOR'. ``` Can be replaced with ``` km = kc.keymaps.ensure_from_default("Text") ``` The key-map uses settings (space type, region type etc...) from `bpy.context.window_manager.keyconfigs.default`.

Demeter Dzadik commented

2023-07-14 16:06:32 +02:00

That's much better, thank you! But something still seems off:

import bpy

wm = bpy.context.window_manager
kc = wm.keyconfigs.addon
km = kc.keymaps.ensure_from_default("Text")
km.keymap_items.new('text.run_script', type='Q', value='PRESS', shift=True)

print("hi")

When first executing this script, it prints "hi", but the hotkey doesn't appear in the preferences UI yet, and doesn't work either. When running the script again, the hotkey appears, and starts functioning. Could that be to do with having changed the order of things?

That's much better, thank you! But something still seems off: ```python import bpy wm = bpy.context.window_manager kc = wm.keyconfigs.addon km = kc.keymaps.ensure_from_default("Text") km.keymap_items.new('text.run_script', type='Q', value='PRESS', shift=True) print("hi") ``` When first executing this script, it prints "hi", but the hotkey doesn't appear in the preferences UI yet, and doesn't work either. When running the script again, the hotkey appears, and starts functioning. Could that be to do with having changed the order of things?

Campbell Barton commented

2023-07-15 08:54:50 +02:00

This is strange, in my tests the resulting hot-key worked as expected.

Demeter Dzadik commented

2023-07-17 10:58:07 +02:00

This is strange, in my tests the resulting hot-key worked as expected.

You seem to be right, I think I made a mistake! Tried again, and it seems to work on first try. Not quite sure what was happening earlier, maybe I was going crazy.

> This is strange, in my tests the resulting hot-key worked as expected. You seem to be right, I think I made a mistake! Tried again, and it seems to work on first try. Not quite sure what was happening earlier, maybe I was going crazy.

Brecht Van Lommel requested changes 2023-07-17 19:51:22 +02:00

Brecht Van Lommel left a comment

There is now a WM_init -> wm_homefile_read_post call before loading add-ons, which in turn calls:

wm_read_callback_post_wrapper, which does a load post handler. An add-on may rely on this being called once on startup.
wm_file_read_post, which does some app template stuff, more handlers, depsgraph evaluation, etc.

Maybe a safer change is to move some of this code from creator.c to the end of WM_init, right before wm_homefile_read_post?

  RE_engines_init_experimental();

  WM_keyconfig_init(C);

#ifdef WITH_FREESTYLE
  FRS_init();
  FRS_set_context(C);
#endif

  WM_init_scripts_extended_once(C);

  wm_homefile_read_post(C, params_file_read_post);

There is now a `WM_init` -> `wm_homefile_read_post` call before loading add-ons, which in turn calls: * `wm_read_callback_post_wrapper`, which does a load post handler. An add-on may rely on this being called once on startup. * `wm_file_read_post`, which does some app template stuff, more handlers, depsgraph evaluation, etc. Maybe a safer change is to move some of this code from `creator.c` to the end of `WM_init`, right before `wm_homefile_read_post`? ``` RE_engines_init_experimental(); WM_keyconfig_init(C); #ifdef WITH_FREESTYLE FRS_init(); FRS_set_context(C); #endif WM_init_scripts_extended_once(C); wm_homefile_read_post(C, params_file_read_post); ```

scripts/modules/bpy/utils/__init__.py Outdated

						
				@ -190,3 +190,3 @@

				def load_scripts(*, reload_scripts=False, refresh_scripts=False):

				def load_scripts(*, reload_scripts=False, refresh_scripts=False, extended=True):

Brecht Van Lommel commented

2023-07-17 17:16:09 +02:00

Maybe renameextended to extensions?

To me that seems like a logical name given the future direction for naming such things extensions, while "extended" is different enough that it sounds like it could be something different.

Maybe rename`extended` to `extensions`? To me that seems like a logical name given the future direction for naming such things extensions, while "extended" is different enough that it sounds like it could be something different.

ideasman42 marked this conversation as resolved

Campbell Barton force-pushed pr-postpone-loading-add-ons from aa2fbbbbc0 to 2bdffbe774

2023-08-30 09:45:31 +02:00

Compare

Campbell Barton requested review from Brecht Van Lommel 2023-08-30 09:46:01 +02:00

Brecht Van Lommel requested changes 2023-08-30 10:04:59 +02:00

Brecht Van Lommel left a comment

You only moved WM_init_scripts_extended_once to WM_init, not all the functions I suggested. In particular, WM_keyconfig_init now runs after WM_init_scripts_extended_once. Was this intentional?

You only moved `WM_init_scripts_extended_once` to `WM_init`, not all the functions I suggested. In particular, `WM_keyconfig_init` now runs after `WM_init_scripts_extended_once`. Was this intentional?

Campbell Barton commented

2023-08-30 14:45:43 +02:00

No, not intentional, it's a bit late here, I'll update the patch & double-check tomorrow.

Campbell Barton force-pushed pr-postpone-loading-add-ons from 2bdffbe774 to ecc61aaf8e

2023-08-31 07:51:51 +02:00

Compare

Campbell Barton commented

2023-08-31 07:58:56 +02:00

Updated, although I'd rather keep freestyle & render engine init out of WM_init().
While this probably works OK either way, they don't really fit in WM initialization.

If freestyle moves into WM_init, The callback defined in FRS_init before wm_homefile_read_post runs, although in practice this doesn't matter either way at the moment.

Updated, although I'd rather keep freestyle & render engine init out of `WM_init()`. While this probably works OK either way, they don't really fit in WM initialization. If freestyle moves into WM_init, The callback defined in FRS_init before `wm_homefile_read_post` runs, although in practice this doesn't matter either way at the moment.

Campbell Barton requested review from Brecht Van Lommel 2023-08-31 07:59:31 +02:00

Campbell Barton added 1 commit 2023-08-31 08:10:08 +02:00

buildbot/vexp-code-patch-coordinator Build done. Details

b746502e89 Remove ensure_from_default method, this can be added separately

Campbell Barton commented

2023-08-31 08:33:04 +02:00

@blender-bot build
The test that failed was unrelated to this PR.

@blender-bot build _The test that failed was unrelated to this PR._

Campbell Barton changed title from ~~WIP: PyAPI: postpone loading add-ons until after key-maps have been defined~~ to PyAPI: postpone loading add-ons until after key-maps have been defined

2023-08-31 10:06:01 +02:00

Brecht Van Lommel approved these changes 2023-09-04 11:21:48 +02:00

Campbell Barton referenced this issue from a commit

2023-09-05 03:38:59 +02:00

PyAPI: postpone loading add-ons until after key-maps have been defined

Campbell Barton closed this pull request

2023-09-05 03:45:23 +02:00

Campbell Barton reopened this pull request

2023-09-05 03:45:33 +02:00

Campbell Barton manually merged commit 6de294a191 into main

2023-09-05 03:45:47 +02:00

Daniel Gryningstjerna commented

2023-11-04 17:47:32 +01:00

First-time contributor

Is there any chance the old behavior for keymaps will return to the way it used to be in 4.0? It was super useful to be able to change user shortcuts with addons at startup, but now it has stopped working, it is no longer possible without some really bad hacks :(

Brecht Van Lommel commented

2023-11-04 18:00:45 +01:00

@Dangry it's not clear to me how this would make it impossible to change user shortcuts. If anything this change should give more control. Consider making a bug report with specific details about what stopped working.

Daniel Gryningstjerna commented

2023-11-04 18:23:45 +01:00

First-time contributor

@Dangry it's not clear to me how this would make it impossible to change user shortcuts. If anything this change should give more control. Consider making a bug report with specific details about what stopped working.

@brecht
Hi Breacht,
sorry if I was unclear, might have missed something here, but to the best of my understanding, this patch unfortunately got reverted 2 weeks ago because of a bug, as can be seen here: cdbde7d941.

This means that it is way harder to change user keymaps when an addon is loading at startup, so its not a bug, but how it used to be, since it requires a hack to make it work, just like before 4.0.

This patch was super nice, and it was something as an addon developer I looked forward to a lot in 4.0. I am a little sad to see it gone since it's a very nice quality of life feature when making addons. I really hope it will be reconsidered in the futer!

> @Dangry it's not clear to me how this would make it impossible to change user shortcuts. If anything this change should give more control. Consider making a bug report with specific details about what stopped working. @brecht Hi Breacht, sorry if I was unclear, might have missed something here, but to the best of my understanding, this patch unfortunately got reverted 2 weeks ago because of a bug, as can be seen here: https://projects.blender.org/blender/blender/commit/cdbde7d9413c67ee5cecf9ccbd13fc9d719553c8. This means that it is way harder to change user keymaps when an addon is loading at startup, so its not a bug, but how it used to be, since it requires a hack to make it work, just like before 4.0. This patch was super nice, and it was something as an addon developer I looked forward to a lot in 4.0. I am a little sad to see it gone since it's a very nice quality of life feature when making addons. I really hope it will be reconsidered in the futer!

Brecht Van Lommel commented

2023-11-06 09:19:30 +01:00

It didn't get reverted but modified.

It's still unclear to me what you wanted to do and why. The design is that there is a Blender base keymap, then add-ons can add their own items on top of that, and then as the last step user modifications get applied.

Typical add-ons should not be messing with the user modifications, unless it's an add-on specifically about keymap editing.

It didn't get reverted but modified. It's still unclear to me what you wanted to do and why. The design is that there is a Blender base keymap, then add-ons can add their own items on top of that, and then as the last step user modifications get applied. Typical add-ons should not be messing with the user modifications, unless it's an add-on specifically about keymap editing.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

PyAPI: postpone loading add-ons until after key-maps have been defined #110092