[Bf-codereview] Pepper to trunk merge (issue 4934047)

mp.theeth at gmail.com mp.theeth at gmail.com
Sun Aug 28 18:23:18 CEST 2011


First pass review


http://codereview.appspot.com/4934047/diff/6043/intern/audaspace/FX/AUD_BaseIIRFilterReader.cpp
File intern/audaspace/FX/AUD_BaseIIRFilterReader.cpp (right):

http://codereview.appspot.com/4934047/diff/6043/intern/audaspace/FX/AUD_BaseIIRFilterReader.cpp#newcode36
intern/audaspace/FX/AUD_BaseIIRFilterReader.cpp:36: #define CC
m_specs.channels + m_channel
Should be undefed at the end of the file to be safe.

http://codereview.appspot.com/4934047/diff/6043/intern/audaspace/intern/AUD_3DMath.h
File intern/audaspace/intern/AUD_3DMath.h (right):

http://codereview.appspot.com/4934047/diff/6043/intern/audaspace/intern/AUD_3DMath.h#newcode160
intern/audaspace/intern/AUD_3DMath.h:160: inline AUD_Vector3
operator*(const float& op) const
Why use a reference to the float?

http://codereview.appspot.com/4934047/diff/6043/intern/audaspace/intern/AUD_Reference.h
File intern/audaspace/intern/AUD_Reference.h (right):

http://codereview.appspot.com/4934047/diff/6043/intern/audaspace/intern/AUD_Reference.h#newcode123
intern/audaspace/intern/AUD_Reference.h:123: m_original = 0;
I'd prefer NULL instead of 0 for clarity sake.

Same for the other occurrences through the file.

http://codereview.appspot.com/4934047/diff/6043/intern/audaspace/intern/AUD_ReferenceHandler.cpp
File intern/audaspace/intern/AUD_ReferenceHandler.cpp (right):

http://codereview.appspot.com/4934047/diff/6043/intern/audaspace/intern/AUD_ReferenceHandler.cpp#newcode33
intern/audaspace/intern/AUD_ReferenceHandler.cpp:33: std::map<void*,
int> AUD_ReferenceHandler::m_references;
unsigned int would be good too.

http://codereview.appspot.com/4934047/diff/6043/release/scripts/startup/bl_ui/properties_data_speaker.py
File release/scripts/startup/bl_ui/properties_data_speaker.py (right):

http://codereview.appspot.com/4934047/diff/6043/release/scripts/startup/bl_ui/properties_data_speaker.py#newcode32
release/scripts/startup/bl_ui/properties_data_speaker.py:32: return
context.speaker and (engine in cls.COMPAT_ENGINES)
It would be nice to have a decorator for engine compatibility, that code
is everywhere way often.

http://codereview.appspot.com/4934047/diff/6043/source/blender/editors/transform/transform_generics.c
File source/blender/editors/transform/transform_generics.c (right):

http://codereview.appspot.com/4934047/diff/6043/source/blender/editors/transform/transform_generics.c#newcode846
source/blender/editors/transform/transform_generics.c:846:
recalcData_actedit(t);
Spring cleaning, great.

http://codereview.appspot.com/4934047/diff/6043/source/blender/makesdna/DNA_speaker_types.h
File source/blender/makesdna/DNA_speaker_types.h (right):

http://codereview.appspot.com/4934047/diff/6043/source/blender/makesdna/DNA_speaker_types.h#newcode42
source/blender/makesdna/DNA_speaker_types.h:42: short flag;
I'd suggest moving flag at the end of the struct, this way the alignment
padding is at the end.

http://codereview.appspot.com/4934047/diff/6043/source/blender/makesrna/intern/rna_sound.c
File source/blender/makesrna/intern/rna_sound.c (right):

http://codereview.appspot.com/4934047/diff/6043/source/blender/makesrna/intern/rna_sound.c#newcode99
source/blender/makesrna/intern/rna_sound.c:99:
RNA_def_property_ui_text(prop, "Mono", "If the file contains multiple
audio channels they are mixdown to a signle one.");
signle -> single

Also, mixdown in this context sounds like bad grammar, but I'm not sure
what would be better: 'mixed down' is weird and 'reduced' might be a bit
too generic.

http://codereview.appspot.com/4934047/diff/6043/source/blender/makesrna/intern/rna_userdef.c
File source/blender/makesrna/intern/rna_userdef.c (right):

http://codereview.appspot.com/4934047/diff/6043/source/blender/makesrna/intern/rna_userdef.c#newcode2954
source/blender/makesrna/intern/rna_userdef.c:2954: prop=
RNA_def_property(srna, "use_update_recent_files_on_load", PROP_BOOLEAN,
PROP_NONE);
Agreed.

If needed, I think the proper solution would be to keep two separate
lists (recent opened, recent saved) and display the one that makes sense
in context.

http://codereview.appspot.com/4934047/


More information about the Bf-codereview mailing list