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

lukas.toenne at gmail.com lukas.toenne at gmail.com
Thu Sep 19 12:26:31 CEST 2013


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)
On 2013/09/19 09:15:37, ideasman42 wrote:
> 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.

Done.

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))
On 2013/09/19 09:15:37, ideasman42 wrote:
> very similar to reset_default_button_exec(). could de-duplicate here.

True. Added operator_button_property_finish function for doing update
calls and return value in the success case.

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 */
On 2013/09/19 09:15:37, ideasman42 wrote:
> would add /* XXX, check on a better way to denote unset-state */

Will try different UI concepts, leaving as it is here for now.

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;
On 2013/09/19 09:15:37, ideasman42 wrote:
> 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.

This function does not seem to be used anywhere yet, so would not risk
breakage. It also doesn't work as expected, which is why i changed it in
the first place.

RNA_struct_idprops_check also doesn't tell whether a property uses ID
props, just whether the struct generally supports them.

Then again, the PROP_IDPROPERTY is checked in RNA_property_unset anyway,
so not strictly necessary to check in execute (in poll it would be
useful but this was removed as you suggested). All it does it prevent
unnecessary updates when unsetting non-ID properties, which has no
effect.

Removing this change for the time being.

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


More information about the Bf-codereview mailing list