[Bf-committers] [Bf-blender-cvs] SVN commit: /data/svn/bf-blender [54258] trunk/blender/source/blender/ blenloader/intern/readfile.c: add missing break in direct_link_constraints, CONSTRAINT_SPACEONCE flag was getting set to CONSTRAINT_TYPE_KINEMATIC.

Joshua Leung aligorith at gmail.com
Sat Feb 2 06:09:28 CET 2013


+1 for the fix

BUT
-1 for the trojan horsed style cleanup.

break;  statements here are part of the switch-case structure. By lumping
them inside these blocks, you run a greater risk of people glossing over
them or even leaving them out by mistake, as they effectively look like
just any instruction there.

A particularly ugly (and confusing) example from a few thousand commits ago
ended up looking like
   case ABC:
   {
       ...
       if (...) {
            break;
       }
       break;
    }
    case DEF:
    ...

Not indenting them while leaving them outside the block, i.e.
    case ABC:
    {
        ... some code here ...
    }
    break;
    case ...
Again raises this gloss-over risk - here break+case may be mistaken for
case+case (as many if not most IDE's I've seen usually highlight these in
the same way).


On Sat, Feb 2, 2013 at 5:13 PM, Campbell Barton <ideasman42 at gmail.com>wrote:

> Revision: 54258
>
> http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=54258
> Author:   campbellbarton
> Date:     2013-02-02 04:13:38 +0000 (Sat, 02 Feb 2013)
> Log Message:
> -----------
> add missing break in direct_link_constraints, CONSTRAINT_SPACEONCE flag
> was getting set to CONSTRAINT_TYPE_KINEMATIC.
>
> Modified Paths:
> --------------
>     trunk/blender/source/blender/blenloader/intern/readfile.c
>
> Modified: trunk/blender/source/blender/blenloader/intern/readfile.c
> ===================================================================
> --- trunk/blender/source/blender/blenloader/intern/readfile.c   2013-02-02
> 02:42:12 UTC (rev 54257)
> +++ trunk/blender/source/blender/blenloader/intern/readfile.c   2013-02-02
> 04:13:38 UTC (rev 54258)
> @@ -2679,15 +2679,15 @@
>                                 data->prop = newdataadr(fd, data->prop);
>                                 if (data->prop)
>                                         IDP_DirectLinkProperty(data->prop,
> (fd->flags & FD_FLAGS_SWITCH_ENDIAN), fd);
> +                               break;
>                         }
> -                               break;
>                         case CONSTRAINT_TYPE_SPLINEIK:
>                         {
>                                 bSplineIKConstraint *data= con->data;
> -
> +
>                                 data->points= newdataadr(fd, data->points);
> +                               break;
>                         }
> -                               break;
>                         case CONSTRAINT_TYPE_KINEMATIC:
>                         {
>                                 bKinematicConstraint *data = con->data;
> @@ -2697,14 +2697,15 @@
>
>                                 /* version patch for runtime flag, was not
> cleared in some case */
>                                 data->flag &= ~CONSTRAINT_IK_AUTO;
> +                               break;
>                         }
>                         case CONSTRAINT_TYPE_CHILDOF:
>                         {
>                                 /* XXX version patch, in older code this
> flag wasn't always set, and is inherent to type */
>                                 if (con->ownspace == CONSTRAINT_SPACE_POSE)
>                                         con->flag |= CONSTRAINT_SPACEONCE;
> +                               break;
>                         }
> -                               break;
>                 }
>         }
>  }
>
> _______________________________________________
> Bf-blender-cvs mailing list
> Bf-blender-cvs at blender.org
> http://lists.blender.org/mailman/listinfo/bf-blender-cvs
>


More information about the Bf-committers mailing list