[Bf-codereview] Fix for #36226, Select Linked works not in touch with Prefs. (issue 13244048)

ideasman42 at gmail.com ideasman42 at gmail.com
Thu Sep 19 11:15:37 CEST 2013


Found one issue I'm hesitant about changing and some minor issues,
otherwise LGTM code-wise.

On the usability side would be good to have other devs check before
committing into trunk since it changes how keymap editing works.


https://codereview.appspot.com/13244048/diff/1/source/blender/editors/interface/interface_ops.c
File source/blender/editors/interface/interface_ops.c (right):

https://codereview.appspot.com/13244048/diff/1/source/blender/editors/interface/interface_ops.c#newcode508
source/blender/editors/interface/interface_ops.c:508: static int
unset_property_button_poll(bContext *C)
This poll() function is quite intensive, it loops over button area
blocks and searches for the button.

We try to keep poll functions fairly minimal - I think the poll function
could be left out or just check ED_operator_regionactive or so. The menu
item can be left out if unset isnt possible.

https://codereview.appspot.com/13244048/diff/1/source/blender/editors/interface/interface_ops.c#newcode520
source/blender/editors/interface/interface_ops.c:520: static int
unset_property_button_exec(bContext *C, wmOperator *UNUSED(op))
very similar to reset_default_button_exec(). could de-duplicate here.

https://codereview.appspot.com/13244048/diff/1/source/blender/editors/interface/interface_templates.c
File source/blender/editors/interface/interface_templates.c (right):

https://codereview.appspot.com/13244048/diff/1/source/blender/editors/interface/interface_templates.c#newcode3374
source/blender/editors/interface/interface_templates.c:3374: /* add
property */
would add /* XXX, check on a better way to denote unset-state */

https://codereview.appspot.com/13244048/diff/1/source/blender/makesrna/intern/rna_access.c
File source/blender/makesrna/intern/rna_access.c (right):

https://codereview.appspot.com/13244048/diff/1/source/blender/makesrna/intern/rna_access.c#newcode4968
source/blender/makesrna/intern/rna_access.c:4968: return (prop->flag &
PROP_IDPROPERTY) != 0;
Not sure about this commit, could have bad side-effects (since relase is
coming up).
In the case where you're using this, you can use
RNA_struct_idprops_check(ptr->type) instead of checking the prop and
modifying this function.

https://codereview.appspot.com/13244048/


More information about the Bf-codereview mailing list