Text datablocks don't self-register when getting a .00x suffix #89532

New Issue

Demeter Dzadik · 2021-06-29T15:57:44+02:00

Demeter Dzadik commented

2021-06-29 15:57:44 +02:00

Problem
Although all text blocks that don't have an extension use script highlighting, only those which end in exactly .py allow for the "Register" checkbox's functionality to work. I think this design forgot to take into account that Blender will slap a .001 suffix after duplicates datablocks, which can cause scripts to fail to register themselves pretty easily, and in a way that is very hard to troubleshoot for a user who is not in the know.

Example cases
For example, someone might rig two characters in two files, and create rig UI scripts for them, both named rig_ui.py. Then, since they're working on a small hobby level production, they will append the characters into the same scene, resulting in the appended script getting a .001 suffix and failing to execute.

In another case, one might want to append multiple of the same character, resulting in the same issue, and in this case naming the rig script in a unique way won't even help.

Even in a real production where linking allows these datablocks to have their own namespaces, sometimes there's small flaws in pipeline tools, or somebody makes a small mistake, and a script can end up with a .001 suffix in a plethora of ways.

Solutions
I would propose allowing scripts to use the Register feature not only if their names end in exactly .py but also .py.### (I think there's already naming util functions to check for this)

Alternative would be to put the .001 before the .py when a duplicate appears but I think that would require adding a special case to much deeper code which probably nobody wants to do ;).

This was also mentioned in an Animation Module meeting once upon a time and according to my memory there was general agreement toward the first solution, but my memories are faded and I'm biased.

CC @dr.sybren @BassamKurdali @LucianoMunoz @icappiello @jpbouza-4 Thoughts? :)

**Problem** Although all text blocks that don't have an extension use script highlighting, only those which end in exactly `.py` allow for the "Register" checkbox's functionality to work. I think this design forgot to take into account that Blender will slap a .001 suffix after duplicates datablocks, which can cause scripts to fail to register themselves pretty easily, and in a way that is very hard to troubleshoot for a user who is not in the know. **Example cases** For example, [someone](https://gitlab.com/blender/CloudRig) might rig two characters in two files, and create rig UI scripts for them, both named `rig_ui.py`. Then, since they're working on a small hobby level production, they will append the characters into the same scene, resulting in the appended script getting a .001 suffix and failing to execute. In another case, one might want to append multiple of the same character, resulting in the same issue, and in this case naming the rig script in a unique way won't even help. Even in a real production where linking allows these datablocks to have their own namespaces, sometimes there's [small flaws](https://dev-files.blender.org/file/data/z3cc7hh2vepxxybjiz72/PHID-FILE-dhmjgabyiqk2rl4l45w2/image.png) in pipeline tools, or somebody makes a small mistake, and a script can end up with a .001 suffix in a plethora of ways. **Solutions** I would propose allowing scripts to use the Register feature not only if their names end in exactly `.py` but also `.py.###` (I think there's already naming util functions to check for this) Alternative would be to put the .001 before the .py when a duplicate appears but I think that would require adding a special case to much deeper code which probably nobody wants to do ;). This was also mentioned in an Animation Module meeting once upon a time and according to my memory there was general agreement toward the first solution, but my memories are faded and I'm biased. CC @dr.sybren @BassamKurdali @LucianoMunoz @icappiello @jpbouza-4 Thoughts? :)

Demeter Dzadik commented

2021-06-29 15:57:44 +02:00

Added subscribers: @icappiello, @Bassam, @LucianoMunoz, @jpbouza-4, @dr.sybren, @Mets

Demeter Dzadik commented

2021-06-29 15:58:17 +02:00

Added subscriber: @BassamKurdali

Philipp Oeser commented

2021-06-30 12:31:52 +02:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2021-06-30 12:31:52 +02:00

Changed status from 'Needs Triage' to: 'Needs Developer To Reproduce'

Philipp Oeser commented

2021-06-30 12:31:52 +02:00

can confirm the issue, will leave classification up to module devs

Sybren A. Stüvel commented

2021-07-02 10:37:17 +02:00

+1 for the first solution. The exact datablock name is less important now that they cannot be directly imported any more; import rig_ui used to work for importing a text datablock rig_ui.py; now you need rig_ui = bpy.data.texts['rig_ui'].as_module() for this.

This might require changing the logic in ED_text_is_syntax_highlight_supported() a bit. Currently it always returns true for any datablock ending in a .### numerical suffix. If we say that blabla.xxx.### should be treated the same as blablabla.xxx, the current logic in that function is not suitable any more.

Furthermore, TEXT_MT_text should be adjusted. Currently it's the menu's Python code that determines whether the "Register" checkbox is enabled or not. IMO this is a bad design -- the ability to register a text datablock should be handled in RNA and not in UI code.

+1 for the first solution. The exact datablock name is less important now that they cannot be directly imported any more; `import rig_ui` used to work for importing a text datablock `rig_ui.py`; now you need `rig_ui = bpy.data.texts['rig_ui'].as_module()` for this. This might require changing the logic in `ED_text_is_syntax_highlight_supported()` a bit. Currently it always returns `true` for any datablock ending in a `.###` numerical suffix. If we say that `blabla.xxx.###` should be treated the same as `blablabla.xxx`, the current logic in that function is not suitable any more. Furthermore, `TEXT_MT_text` should be adjusted. Currently it's the menu's Python code that determines whether the "Register" checkbox is enabled or not. IMO this is a bad design -- the ability to register a text datablock should be handled in RNA and not in UI code.

Campbell Barton commented

2021-08-10 10:12:24 +02:00

Added subscriber: @ideasman42

Campbell Barton commented

2021-08-10 10:12:24 +02:00

In #89532#1185872, @dr.sybren wrote:
+1 for the first solution. The exact datablock name is less important now that they cannot be directly imported any more; import rig_ui used to work for importing a text datablock rig_ui.py; now you need rig_ui = bpy.data.texts['rig_ui'].as_module() for this.

Since the scripts are executed instead of loading as a module there is no longer a need to have any constraint on the naming.

This might require changing the logic in ED_text_is_syntax_highlight_supported() a bit. Currently it always returns true for any datablock ending in a .### numerical suffix. If we say that blabla.xxx.### should be treated the same as blablabla.xxx, the current logic in that function is not suitable any more.

Seems reasonable this can be handled separately.

Furthermore, TEXT_MT_text should be adjusted. Currently it's the menu's Python code that determines whether the "Register" checkbox is enabled or not. IMO this is a bad design -- the ability to register a text datablock should be handled in RNA and not in UI code.

This can't be done in RNA, it's only possible to conditionally be read-only, where as for this we would only want to grey it out.

> In #89532#1185872, @dr.sybren wrote: > +1 for the first solution. The exact datablock name is less important now that they cannot be directly imported any more; `import rig_ui` used to work for importing a text datablock `rig_ui.py`; now you need `rig_ui = bpy.data.texts['rig_ui'].as_module()` for this. Since the scripts are executed instead of loading as a module there is no longer a need to have any constraint on the naming. > This might require changing the logic in `ED_text_is_syntax_highlight_supported()` a bit. Currently it always returns `true` for any datablock ending in a `.###` numerical suffix. If we say that `blabla.xxx.###` should be treated the same as `blablabla.xxx`, the current logic in that function is not suitable any more. Seems reasonable this can be handled separately. > Furthermore, `TEXT_MT_text` should be adjusted. Currently it's the menu's Python code that determines whether the "Register" checkbox is enabled or not. IMO this is a bad design -- the ability to register a text datablock should be handled in RNA and not in UI code. This can't be done in RNA, it's only possible to conditionally be read-only, where as for this we would only want to grey it out.

Campbell Barton commented

2021-08-10 10:36:26 +02:00

Submitted D12181, this removes the check for the extension entirely.

Submitted [D12181](https://archive.blender.org/developer/D12181), this removes the check for the extension entirely.

blender-admin commented

2021-08-13 07:51:08 +02:00

This issue was referenced by 3e775a4fc5

This issue was referenced by 3e775a4fc57cfd48954adcf284354312f34d5412

Campbell Barton commented

2021-08-13 07:55:00 +02:00

Changed status from 'Needs Developer To Reproduce' to: 'Resolved'

Campbell Barton closed this issue

2021-08-13 07:55:00 +02:00

Campbell Barton self-assigned this 2021-08-13 07:55:00 +02:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Text datablocks don't self-register when getting a .00x suffix #89532