[Bf-codereview] Operator to setup scene for camera tracking (issue 5432048)

[ideasman42 at gmail.com]

a few replies for remaining issues.


http://codereview.appspot.com/5432048/diff/1/release/scripts/startup/bl_operators/clip.py
File release/scripts/startup/bl_operators/clip.py (right):

http://codereview.appspot.com/5432048/diff/1/release/scripts/startup/bl_operators/clip.py#newcode353
release/scripts/startup/bl_operators/clip.py:353: sc =
context.space_data
On 2011/11/23 12:45:31, sergey.vfx wrote:
> Not sure understand you correct, but now "sc = .." moved above "if"
and "if"
> replaced with "if sc.type blablabla"

Done.

http://codereview.appspot.com/5432048/diff/1/release/scripts/startup/bl_operators/clip.py#newcode417
release/scripts/startup/bl_operators/clip.py:417: def
_findRenderLayer(self, scene, name):
On 2011/11/23 12:45:31, sergey.vfx wrote:
> Indeed. But think i can drop this function then and use
> scene.render.layers.get(name) instead.

Im not fussed if this function is simplified or replaced with
scene.render.layers.get, IMHO either is ok.

http://codereview.appspot.com/5432048/diff/1/release/scripts/startup/bl_operators/clip.py#newcode497
release/scripts/startup/bl_operators/clip.py:497: while len(tree.links):
On 2011/11/23 12:45:31, sergey.vfx wrote:
> Hrm. Think this clear isn't really needed.

ok

http://codereview.appspot.com/5432048/diff/1/release/scripts/startup/bl_operators/clip.py#newcode673
release/scripts/startup/bl_operators/clip.py:673: if ob.type == 'MESH'
and ob.name == 'Ground':
On 2011/11/23 12:45:31, sergey.vfx wrote:
> Don't think it'll work. "Ground" is creating by this script and this
check was
> added to prevent adding it several times. Artist can easily already
have it's
> own planes before running this script and in this case ground plane
wouldn't be
> created.
> Probably custom properties can be used to distinguish automatically
created
> objects by script from objects, created by user

agree, in that case tagging an object as being ground makes more sense
though we don't really have a precedent here.

obj["is_ground"] = True  # ?, maybe something like this.

http://codereview.appspot.com/5432048/diff/1/release/scripts/startup/bl_operators/clip.py#newcode679
release/scripts/startup/bl_operators/clip.py:679: all_layers = []
On 2011/11/23 12:45:31, sergey.vfx wrote:
> One liners.. If you think it's easier to read, then ok :)

eeh, this is a bit subjective, I like LC's and this seems an appropriate
use to me, but some people complained about my code using LC's too much.

reminds me, would be nice to have boolean op support on boolean arrays
so you could just do (layers_a & layers_b) but thats for later.

http://codereview.appspot.com/5432048/