[Bf-committers] Proposal: rationalize string-length #defines

[Joshua Leung]

Hi,

AFAIK, changes on this issue (magic numbers vs defines) have been
proposed at least a few times over the years (I might have done it a
few times myself too). However, so far it seems that everytime this
issue has reared its head, the problem of makesdna not supporting such
defines to be used as array size specifiers comes up. I know that this
is why I haven't already done this for quite a few cases.

Oddly enough, at one stage I'm pretty sure I saw a patch in the patch
tracker which would have added support for this to makesdna. However,
after loosing track of it a few months while busy working on some
other projects, I've never been able to find this in the tracker
anywhere ever since.

In the meantime, in many places where the length of these items
actually matters, Campbell went through about a year ago and replaced
all the places which used magic numbers or defines with the
sizeof(struct->name) idiom seen throughout the code. In a way, this
has mitigated the problem somewhat, in that even if defines got out of
sync, the code would still manage to cope.

Now, as far as know, at some point during the 2.6 releases, we were
planning on finally upping the size limits on all of these name fields
to some consistent value, and which would be sufficient for all of the
crazy names that people are giving things. Especially now with unicode
support coming in (?) in places, bumping these up is an important
issue (i.e. ID-blocks are limited to having 21 character names, given
that the name buffer is 24 chars long, including two for type
identification and a null terminator, not to mention about 3-4 of
those characters often being taken for unique naming purposes too).

>From the release planning page, it appears that this was slated to
finally go ahead in this coming release cycle. If that's the case,
then perhaps there's a case to be made for unifying all of these into
a single "name length" define while we're at it, and after makesdna
has been safely and successfully hacked ;)


Aligorith

On Fri, Dec 16, 2011 at 12:27 AM, Bastien Montagne
<montagne29 at wanadoo.fr> wrote:
> Hi devs!
>
> While working on some improvements to WeightVG modifiers, I stumbled
> upon that: in DNA def of modifier structs, all vgroup and uvmap name
> lengths are literal (32)… Imho, a define would be much better (and much
> easier in case we decide to change it one day). We do have a
> MAX_VGROUP_NAME defined in DNA_object_types.h – but I guess including
> that file into DNA_modifier_types.h just for that wouldn’t be wise.
> Also, the FILE_MAX & co are defined twice, once in BKE, once in DNA…
>
> So here is my proposition: why not gather all those string-length
> defines in one place (e.g. the recent DNA_defs.h, which seems to be
> safely includeable every where) ? After a quick search, this would
> concern at least:
> * FILE_MAX and co
> * MAX_VGROUP_NAME, MAX_STYLE_NAME, MAX_FONT_NAME (and others, like [new]
> MAX_OBJ_NAME, MAX_UVMAP_NAME, etc.)
> * MAX_ID_NAME and MAX_IDPROP_NAME
>
> Also, would we need a MAX_VGROUP_NAME, MAX_OBJ_NAME, MAX_UVMAP_NAME,
> etc., or just define a common MAX_BLDEF_NAME (e.g.), as they all seem
> set to 32 currently?
>
> I’m well aware that this involves many parts of Blender code, and as
> such might need a much deeper knowledge of that code than I currently
> have (even though changes could be made progressively, as values would
> remain the same) – but I really think this should be addressed, for
> sanity of code.
>
> Best regards,
> Bastien.
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers