From 89e790d2a494e9a322331aeba273f9fdc9689982 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Fri, 7 Jun 2024 14:47:01 +0200 Subject: [PATCH] Cleanup: Improve/fix some IDProperty comments and TODOs. Mainly correct/update some comments (e.g. missing reference to Boolean type), and add some notes essentially about issues with current IDProp String code (see also #86960 ). No functional change. --- source/blender/blenkernel/BKE_idprop.hh | 2 +- source/blender/blenkernel/intern/idprop.cc | 6 ++++++ source/blender/makesdna/DNA_ID.h | 2 +- source/blender/python/generic/idprop_py_api.cc | 12 +++++++++--- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/source/blender/blenkernel/BKE_idprop.hh b/source/blender/blenkernel/BKE_idprop.hh index 4d15d3eff94..af3770f5659 100644 --- a/source/blender/blenkernel/BKE_idprop.hh +++ b/source/blender/blenkernel/BKE_idprop.hh @@ -208,7 +208,7 @@ bool IDP_EqualsProperties(const IDProperty *prop1, const IDProperty *prop2) ATTR_WARN_UNUSED_RESULT; /** - * Allocate a new ID. + * Allocate a new IDProperty. * * This function takes three arguments: the ID property type, a union which defines * its initial value, and a name. diff --git a/source/blender/blenkernel/intern/idprop.cc b/source/blender/blenkernel/intern/idprop.cc index 5d1f7cbe3fa..0339b1a41fd 100644 --- a/source/blender/blenkernel/intern/idprop.cc +++ b/source/blender/blenkernel/intern/idprop.cc @@ -406,6 +406,8 @@ static IDProperty *IDP_CopyString(const IDProperty *prop, const int flag) return newp; } +/* FIXME: This function is broken for bytes (in case there are null chars in it), needs a + * dedicated function which takes directly the size of the byte buffer. */ void IDP_AssignStringMaxSize(IDProperty *prop, const char *st, const size_t st_maxncpy) { BLI_assert(prop->type == IDP_STRING); @@ -421,6 +423,7 @@ void IDP_AssignStringMaxSize(IDProperty *prop, const char *st, const size_t st_m } } +/* FIXME: Should never be called for `byte` subtype, needs an assert. */ void IDP_AssignString(IDProperty *prop, const char *st) { IDP_AssignStringMaxSize(prop, st, 0); @@ -997,6 +1000,9 @@ IDProperty *IDP_New(const char type, const IDPropertyTemplate *val, const char * prop->data.val = bool(val->i); break; case IDP_ARRAY: { + /* FIXME: This seems to be the only place in code allowing `IDP_GROUP` as subtype of an + * `IDP_ARRAY`. This is most likely a mistake. `IDP_GROUP` array should be of type + * `IDP_IDPARRAY`, as done e.g. in #idp_from_PySequence_Buffer in bpy API. */ if (ELEM(val->array.type, IDP_FLOAT, IDP_INT, IDP_DOUBLE, IDP_GROUP, IDP_BOOLEAN)) { prop = static_cast(MEM_callocN(sizeof(IDProperty), "IDProperty array")); prop->subtype = val->array.type; diff --git a/source/blender/makesdna/DNA_ID.h b/source/blender/makesdna/DNA_ID.h index 14c9d02df72..dda15c3b254 100644 --- a/source/blender/makesdna/DNA_ID.h +++ b/source/blender/makesdna/DNA_ID.h @@ -226,7 +226,7 @@ typedef enum eIDPropertySubType { /** #IDProperty.flag. */ enum { /** - * This #IDProperty may be statically overridden. + * This #IDProperty may be library-overridden. * Should only be used/be relevant for custom properties. */ IDP_FLAG_OVERRIDABLE_LIBRARY = 1 << 0, diff --git a/source/blender/python/generic/idprop_py_api.cc b/source/blender/python/generic/idprop_py_api.cc index 55f1e657355..f50815abfb4 100644 --- a/source/blender/python/generic/idprop_py_api.cc +++ b/source/blender/python/generic/idprop_py_api.cc @@ -328,7 +328,8 @@ static PyObject *BPy_IDGroup_Map_GetItem(BPy_IDProperty *self, PyObject *item) return BPy_IDGroup_WrapData(self->owner_id, idprop, self->prop); } -/* returns nullptr on success, error string on failure */ +/* Return identified matching IDProperty type, or -1 if error (e.g. mixed and/or incompatible + * types, etc.). */ static char idp_sequence_type(PyObject *seq_fast) { PyObject **seq_fast_items = PySequence_Fast_ITEMS(seq_fast); @@ -346,6 +347,7 @@ static char idp_sequence_type(PyObject *seq_fast) type = IDP_DOUBLE; } else if (PyBool_Check(item)) { + /* Mixed boolean and any other type. */ if (i != 0 && (type != IDP_BOOLEAN)) { return -1; } @@ -357,7 +359,8 @@ static char idp_sequence_type(PyObject *seq_fast) } } else if (PyMapping_Check(item)) { - if (i != 0 && (type != IDP_IDPARRAY)) { /* mixed dict/int */ + /* Mixed dict and any other type. */ + if (i != 0 && (type != IDP_IDPARRAY)) { return -1; } type = IDP_IDPARRAY; @@ -470,6 +473,7 @@ static int idp_array_type_from_formatstr_and_size(const char *typestr, Py_ssize_ if (itemsize == 4) { return IDP_INT; } + /* TODO: Support Booleans? */ } return -1; @@ -522,9 +526,11 @@ static IDProperty *idp_from_PySequence_Fast(const char *name, PyObject *ob) ob_seq_fast_items = PySequence_Fast_ITEMS(ob); + /* IDProperties do not support mixed type of data in an array. Try to extract a single type from + * the whole sequence, or error. */ if ((val.array.type = idp_sequence_type(ob)) == char(-1)) { PyErr_SetString(PyExc_TypeError, - "only floats, ints and dicts are allowed in ID property arrays"); + "only floats, ints, booleans and dicts are allowed in ID property arrays"); return nullptr; }