blender/blender-addons
19
43
Fork 96
Code Issues 629 Pull Requests 27 Projects 1 Releases Wiki Activity

3D Panel Pose Library filter doesn't appear to be working correctly. #91461

New Issue
Closed
opened 2021-09-16 19:37:25 +02:00 by Jason schleifer · 17 comments
Jason schleifer commented 2021-09-16 19:37:25 +02:00
Member
Copy Link

System Information
Operating system: macOS-11.5.2-x86_64-i386-64bit 64 Bits
Graphics card: AMD Radeon Pro 560X OpenGL Engine ATI Technologies Inc. 4.1 ATI-4.6.20

Blender Version
Broken: version: 3.0.0 Alpha, branch: master, commit date: 2021-09-13 12:13, hash: blender/blender@4b06420e65
Worked: (newest version of Blender that worked as expected)

Short description of error
I'm using the new pose library in the 3D viewport, and when I try and filter the list of poses, the poses all disappear.

Exact steps for others to reproduce the error
Load the attached file (poseLib.blend)
In the 3d viewport, choose the Animation tab
there are two poses saved in the file - Bend and Neutral
In the filter field, type Bend to search for the Bend pose.
The list of poses clears out - and bend doesn't show up.

It should just filter the list.
poseLib.blend

**System Information** Operating system: macOS-11.5.2-x86_64-i386-64bit 64 Bits Graphics card: AMD Radeon Pro 560X OpenGL Engine ATI Technologies Inc. 4.1 ATI-4.6.20 **Blender Version** Broken: version: 3.0.0 Alpha, branch: master, commit date: 2021-09-13 12:13, hash: `blender/blender@4b06420e65` Worked: (newest version of Blender that worked as expected) **Short description of error** I'm using the new pose library in the 3D viewport, and when I try and filter the list of poses, the poses all disappear. **Exact steps for others to reproduce the error** Load the attached file (poseLib.blend) In the 3d viewport, choose the Animation tab there are two poses saved in the file - Bend and Neutral In the filter field, type Bend to search for the Bend pose. The list of poses clears out - and bend doesn't show up. It should just filter the list. [poseLib.blend](https://archive.blender.org/developer/F10431200/poseLib.blend)
Jason schleifer commented 2021-09-16 19:37:25 +02:00
Author
Member
Copy Link

Added subscriber: @JasonSchleifer

Added subscriber: @JasonSchleifer
blender-admin commented 2021-09-16 19:37:25 +02:00
Copy Link

blender/blender#91231 was marked as duplicate of this issue

blender/blender#91231 was marked as duplicate of this issue
FOUNTMEDIA commented 2021-09-16 19:47:09 +02:00 (Migrated from localhost:3001)
Copy Link

Added subscriber: @FOUNTMEDIA

Added subscriber: @FOUNTMEDIA
FOUNTMEDIA commented 2021-09-16 19:47:09 +02:00 (Migrated from localhost:3001)
Copy Link

Changed status from 'Needs Triage' to: 'Needs User Info'

Changed status from 'Needs Triage' to: 'Needs User Info'
FOUNTMEDIA commented 2021-09-16 19:47:09 +02:00 (Migrated from localhost:3001)
Copy Link

This comment was removed by @FOUNTMEDIA

*This comment was removed by @FOUNTMEDIA*
Thomas Dinges commented 2021-09-17 08:16:38 +02:00
Owner
Copy Link

Added subscriber: @ThomasDinges

Added subscriber: @ThomasDinges
Thomas Dinges commented 2021-09-17 08:17:14 +02:00
Owner
Copy Link

Changed status from 'Needs User Info' to: 'Needs Triage'

Changed status from 'Needs User Info' to: 'Needs Triage'
Philipp Oeser commented 2021-09-17 10:32:30 +02:00
Member
Copy Link

Added subscriber: @lichtwerk

Added subscriber: @lichtwerk
Philipp Oeser self-assigned this 2021-09-17 10:32:30 +02:00
Philipp Oeser commented 2021-09-17 10:32:30 +02:00
Member
Copy Link

Changed status from 'Needs Triage' to: 'Confirmed'

Changed status from 'Needs Triage' to: 'Confirmed'
Philipp Oeser commented 2021-09-17 10:32:30 +02:00
Member
Copy Link

Can confirm, checking...

Can confirm, checking...
blender-admin commented 2021-09-17 11:30:05 +02:00
Copy Link

This issue was referenced by blender/blender@a229a9dd64

This issue was referenced by blender/blender@a229a9dd64821575b27e6e6e317a1ce97e23f6d7
Philipp Oeser removed their assignment 2021-09-17 11:35:04 +02:00
Philipp Oeser commented 2021-09-17 11:35:04 +02:00
Member
Copy Link

Added subscribers: @JulianEisel, @dr.sybren

Added subscribers: @JulianEisel, @dr.sybren
Philipp Oeser commented 2021-09-17 11:35:04 +02:00
Member
Copy Link

Well, since AssetHandle does not have a name_property (RNA_def_struct_name_property), and the UIList is just using the default uilist_filter_items_default it simply cannot filter on names (RNA_struct_name_get_alloc wont succeed).

Not sure what the plans are, there are comments about this design using FileSelectEntry (and getting names via ED_asset_handle_get_name) pretty much being a moving target atm.
Super quicky bandaid (but pretty sure there are other plans here):

P2401: T91461_snippet



diff --git a/source/blender/editors/interface/interface_template_list.cc b/source/blender/editors/interface/interface_template_list.cc
index 8246759ad36..08f28396dd9 100644
--- a/source/blender/editors/interface/interface_template_list.cc
+++ b/source/blender/editors/interface/interface_template_list.cc
@@ -31,6 +31,7 @@
 
 #include "BLT_translation.h"
 
+#include "ED_asset.h"
 #include "ED_screen.h"
 
 #include "MEM_guardedalloc.h"
@@ -216,7 +217,18 @@ static void uilist_filter_items_default(struct uiList *ui_list,
     RNA_PROP_BEGIN (dataptr, itemptr, prop) {
       bool do_order = false;
 
-      char *namebuf = RNA_struct_name_get_alloc(&itemptr, nullptr, 0, nullptr);
+      char *namebuf;
+      bool free_namebuf = false;
+      if (RNA_struct_is_a(itemptr.type, &RNA_AssetHandle)) {
+        AssetHandle *asset_handle = (AssetHandle *)itemptr.data;
+        const char *asset_name = ED_asset_handle_get_name(asset_handle);
+        BLI_strncpy(namebuf, asset_name, sizeof(asset_name));
+      }
+      else {
+        namebuf = RNA_struct_name_get_alloc(&itemptr, nullptr, 0, nullptr);
+        free_namebuf = true;
+      }
+
       const char *name = namebuf ? namebuf : "";
 
       if (filter[0]) {
@@ -244,7 +256,7 @@ static void uilist_filter_items_default(struct uiList *ui_list,
       }
 
       /* free name */
-      if (namebuf) {
+      if (namebuf && free_namebuf) {
         MEM_freeN(namebuf);
       }
       i++;
diff --git a/source/blender/makesrna/intern/rna_asset.c b/source/blender/makesrna/intern/rna_asset.c
index 1e583f4ca52..7649cb4f2dc 100644
--- a/source/blender/makesrna/intern/rna_asset.c
+++ b/source/blender/makesrna/intern/rna_asset.c
@@ -303,6 +303,8 @@ static void rna_def_asset_handle(BlenderRNA *brna)
   RNA_def_property_ui_text(
       prop, "File Entry", "TEMPORARY, DO NOT USE - File data used to refer to the asset");
 
+  /* RNA_def_struct_name_property of some sorts please. */
+
   prop = RNA_def_property(srna, "local_id", PROP_POINTER, PROP_NONE);
   RNA_def_property_struct_type(prop, "ID");
   RNA_def_property_pointer_funcs(prop, "rna_AssetHandle_local_id_get", NULL, NULL, NULL);

CC @dr.sybren
CC @JulianEisel

Well, since `AssetHandle` does not have a `name_property` (`RNA_def_struct_name_property`), and the UIList is just using the default `uilist_filter_items_default` it simply cannot filter on names (`RNA_struct_name_get_alloc` wont succeed). Not sure what the plans are, there are comments about this design using `FileSelectEntry` (and getting names via `ED_asset_handle_get_name`) pretty much being a moving target atm. Super quicky bandaid (but pretty sure there are other plans here): [P2401: T91461_snippet](https://archive.blender.org/developer/P2401.txt) ``` diff --git a/source/blender/editors/interface/interface_template_list.cc b/source/blender/editors/interface/interface_template_list.cc index 8246759ad36..08f28396dd9 100644 --- a/source/blender/editors/interface/interface_template_list.cc +++ b/source/blender/editors/interface/interface_template_list.cc @@ -31,6 +31,7 @@ #include "BLT_translation.h" +#include "ED_asset.h" #include "ED_screen.h" #include "MEM_guardedalloc.h" @@ -216,7 +217,18 @@ static void uilist_filter_items_default(struct uiList *ui_list, RNA_PROP_BEGIN (dataptr, itemptr, prop) { bool do_order = false; - char *namebuf = RNA_struct_name_get_alloc(&itemptr, nullptr, 0, nullptr); + char *namebuf; + bool free_namebuf = false; + if (RNA_struct_is_a(itemptr.type, &RNA_AssetHandle)) { + AssetHandle *asset_handle = (AssetHandle *)itemptr.data; + const char *asset_name = ED_asset_handle_get_name(asset_handle); + BLI_strncpy(namebuf, asset_name, sizeof(asset_name)); + } + else { + namebuf = RNA_struct_name_get_alloc(&itemptr, nullptr, 0, nullptr); + free_namebuf = true; + } + const char *name = namebuf ? namebuf : ""; if (filter[0]) { @@ -244,7 +256,7 @@ static void uilist_filter_items_default(struct uiList *ui_list, } /* free name */ - if (namebuf) { + if (namebuf && free_namebuf) { MEM_freeN(namebuf); } i++; diff --git a/source/blender/makesrna/intern/rna_asset.c b/source/blender/makesrna/intern/rna_asset.c index 1e583f4ca52..7649cb4f2dc 100644 --- a/source/blender/makesrna/intern/rna_asset.c +++ b/source/blender/makesrna/intern/rna_asset.c @@ -303,6 +303,8 @@ static void rna_def_asset_handle(BlenderRNA *brna) RNA_def_property_ui_text( prop, "File Entry", "TEMPORARY, DO NOT USE - File data used to refer to the asset"); + /* RNA_def_struct_name_property of some sorts please. */ + prop = RNA_def_property(srna, "local_id", PROP_POINTER, PROP_NONE); RNA_def_property_struct_type(prop, "ID"); RNA_def_property_pointer_funcs(prop, "rna_AssetHandle_local_id_get", NULL, NULL, NULL); ``` CC @dr.sybren CC @JulianEisel
Philipp Oeser commented 2021-09-17 12:55:35 +02:00
Member
Copy Link

Quoting @JulianEisel from chat:

We can just add a name property to the AssetHandle RNA
Which would just have a getter callback, like local_id there

Problem here is that AssetHandle inherits from PropertyGroup which already sets the name property :/
Why cant RNA_def_struct_name_property just overwrite this? (commented out the check there and it seems to work :))

Quoting @JulianEisel from chat: > We can just add a name property to the AssetHandle RNA > Which would just have a getter callback, like local_id there Problem here is that `AssetHandle` inherits from `PropertyGroup` which already sets the name property :/ Why cant `RNA_def_struct_name_property` just overwrite this? (commented out the check there and it seems to work :))
Philipp Oeser commented 2021-09-17 13:01:33 +02:00
Member
Copy Link

In #91461#1221358, @lichtwerk wrote:
Problem here is that AssetHandle inherits from PropertyGroup which already sets the name property :/
Why cant RNA_def_struct_name_property just overwrite this? (commented out the check there and it seems to work :))

So this isnt working:
P2402: T91461_snippet2



diff --git a/source/blender/makesrna/intern/rna_asset.c b/source/blender/makesrna/intern/rna_asset.c
index 7649cb4f2dc..44b19260a92 100644
--- a/source/blender/makesrna/intern/rna_asset.c
+++ b/source/blender/makesrna/intern/rna_asset.c
@@ -160,6 +160,25 @@ static PointerRNA rna_AssetHandle_local_id_get(PointerRNA *ptr)
   return rna_pointer_inherit_refine(ptr, &RNA_ID, id);
 }
 
+static int rna_AssetHandle_name_length(PointerRNA *ptr)
+{
+  const AssetHandle *asset_handle = ptr->data;
+  return strlen(ED_asset_handle_get_name(asset_handle));
+}
+
+static void rna_AssetHandle_name_get(PointerRNA *ptr, char *value)
+{
+  const AssetHandle *asset_handle = ptr->data;
+  const char *asset_name = ED_asset_handle_get_name(asset_handle);
+
+  if (asset_name) {
+    strcpy(value, asset_name);
+  }
+  else {
+    value- [x] = '\0';
+  }
+}
+
 const EnumPropertyItem *rna_asset_library_reference_itemf(bContext *UNUSED(C),
                                                           PointerRNA *UNUSED(ptr),
                                                           PropertyRNA *UNUSED(prop),
@@ -303,7 +322,10 @@ static void rna_def_asset_handle(BlenderRNA *brna)
   RNA_def_property_ui_text(
       prop, "File Entry", "TEMPORARY, DO NOT USE - File data used to refer to the asset");
 
-  /* RNA_def_struct_name_property of some sorts please. */
+  prop = RNA_def_property(srna, "name", PROP_STRING, PROP_NONE);
+  RNA_def_property_clear_flag(prop, PROP_EDITABLE);
+  RNA_def_property_string_funcs(prop, "rna_AssetHandle_name_get", "rna_AssetHandle_name_length", NULL);
+  RNA_def_struct_name_property(srna, prop);
 
   prop = RNA_def_property(srna, "local_id", PROP_POINTER, PROP_NONE);
   RNA_def_property_struct_type(prop, "ID");

gives (for above reasons):

ERROR (rna.define): rna_define.c:1129 RNA_def_struct_name_property: "AssetHandle.name", name property is already set.
> In #91461#1221358, @lichtwerk wrote: > Problem here is that `AssetHandle` inherits from `PropertyGroup` which already sets the name property :/ > Why cant `RNA_def_struct_name_property` just overwrite this? (commented out the check there and it seems to work :)) So this isnt working: [P2402: T91461_snippet2](https://archive.blender.org/developer/P2402.txt) ``` diff --git a/source/blender/makesrna/intern/rna_asset.c b/source/blender/makesrna/intern/rna_asset.c index 7649cb4f2dc..44b19260a92 100644 --- a/source/blender/makesrna/intern/rna_asset.c +++ b/source/blender/makesrna/intern/rna_asset.c @@ -160,6 +160,25 @@ static PointerRNA rna_AssetHandle_local_id_get(PointerRNA *ptr) return rna_pointer_inherit_refine(ptr, &RNA_ID, id); } +static int rna_AssetHandle_name_length(PointerRNA *ptr) +{ + const AssetHandle *asset_handle = ptr->data; + return strlen(ED_asset_handle_get_name(asset_handle)); +} + +static void rna_AssetHandle_name_get(PointerRNA *ptr, char *value) +{ + const AssetHandle *asset_handle = ptr->data; + const char *asset_name = ED_asset_handle_get_name(asset_handle); + + if (asset_name) { + strcpy(value, asset_name); + } + else { + value- [x] = '\0'; + } +} + const EnumPropertyItem *rna_asset_library_reference_itemf(bContext *UNUSED(C), PointerRNA *UNUSED(ptr), PropertyRNA *UNUSED(prop), @@ -303,7 +322,10 @@ static void rna_def_asset_handle(BlenderRNA *brna) RNA_def_property_ui_text( prop, "File Entry", "TEMPORARY, DO NOT USE - File data used to refer to the asset"); - /* RNA_def_struct_name_property of some sorts please. */ + prop = RNA_def_property(srna, "name", PROP_STRING, PROP_NONE); + RNA_def_property_clear_flag(prop, PROP_EDITABLE); + RNA_def_property_string_funcs(prop, "rna_AssetHandle_name_get", "rna_AssetHandle_name_length", NULL); + RNA_def_struct_name_property(srna, prop); prop = RNA_def_property(srna, "local_id", PROP_POINTER, PROP_NONE); RNA_def_property_struct_type(prop, "ID"); ``` gives (for above reasons): ``` ERROR (rna.define): rna_define.c:1129 RNA_def_struct_name_property: "AssetHandle.name", name property is already set. ```
Julian Eisel commented 2021-09-17 15:18:41 +02:00
Member
Copy Link

There are some other ways we could solve this, but they are more of a hassle than worth it. Esp. considering that the whole AssetHandle design is meant to be temporary anyway. So I'm fine with the if (RNA_struct_is_a(itemptr.type, &RNA_AssetHandle)) hack for now. Just please add a XXX comment to note that this is not meant as a permanent solution.

There are some other ways we could solve this, but they are more of a hassle than worth it. Esp. considering that the whole `AssetHandle` design is meant to be temporary anyway. So I'm fine with the `if (RNA_struct_is_a(itemptr.type, &RNA_AssetHandle))` hack for now. Just please add a `XXX` comment to note that this is not meant as a permanent solution.
Philipp Oeser commented 2021-09-18 08:25:45 +02:00
Member
Copy Link

Changed status from 'Confirmed' to: 'Resolved'

Changed status from 'Confirmed' to: 'Resolved'
Philipp Oeser self-assigned this 2021-09-18 08:25:45 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels   
Interest
Animation & Rigging

  
Interest
Blender Cloud

  
Interest
Collada

  
Interest
Core

  
Interest
Documentation

  
Interest
Eevee & Viewport

  
Interest
Geometry Nodes

  
Interest
Grease Pencil

  
Interest
Import and Export

  
Interest
Modeling

  
Interest
Modifiers

  
Interest
Nodes & Physics

  
Interest
Pipeline, Assets & IO

  
Interest
Platforms, Builds, Tests & Devices

  
Interest
Python API

  
Interest
Rendering & Cycles

  
Interest
Sculpt, Paint & Texture

  
Interest
Translations

  
Interest
User Interface

  
Interest
UV Editing

  
Interest
VFX & Video

  
Meta
Good First Issue

  
Meta
Papercut

  
Module
Add-ons (BF-Blender)

  
Module
Add-ons (Community)

  
Platform
Linux

  
Platform
macOS

  
Platform
Windows

  
Priority
High

  
Priority
Low

  
Priority
Normal

  
Priority
Unbreak Now!

  
Status
Archived

  
Status
Confirmed

  
Status
Duplicate

  
Status
Needs Info from Developers

  
Status
Needs Information from User

  
Status
Needs Triage

  
Status
Resolved

  
Type
Bug

  
Type
Design

  
Type
Known Issue

  
Type
Patch

  
Type
Report

  
Type
To Do

No Label
Interest
Animation & Rigging
Interest
Blender Cloud
Interest
Collada
Interest
Core
Interest
Documentation
Interest
Eevee & Viewport
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
Import and Export
Interest
Modeling
Interest
Modifiers
Interest
Nodes & Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds, Tests & Devices
Interest
Python API
Interest
Rendering & Cycles
Interest
Sculpt, Paint & Texture
Interest
Translations
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Meta
Good First Issue
Meta
Papercut
Module
Add-ons (BF-Blender)
Module
Add-ons (Community)
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
Philipp Oeser
5 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender-addons#91461
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?