----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/347/#review1012 -----------------------------------------------------------
indra/newview/lllocalbitmaps.cpp <http://codereview.secondlife.com/r/347/#comment1030> It's better to initialize member variables in the constructor initialization list: LLLocalBitmap::LLLocalBitmap(std::string filename) : mFilename(filename) , mShortName(gDirUtilp->getBaseFileName(filename, true)) , mValid(false) , mLastModified(NULL) , mLinkStatus(LS_ON) , mUpdateRetries(LL_LOCAL_UPDATE_RETRIES) { (that order should be the declaration order).... and make sure that all members are initialized. indra/newview/lllocalbitmaps.cpp <http://codereview.secondlife.com/r/347/#comment1029> I'd rather see a single return at the bottom. indra/newview/lllocalbitmaps.cpp <http://codereview.secondlife.com/r/347/#comment1032> I think that a debug level log entry here might someday come in handy, and the failure cases below should get info or warning level as appropriate indra/newview/lllocalbitmaps.cpp <http://codereview.secondlife.com/r/347/#comment1028> I'd much rather see a 'decoded' variable set here and in each of the other cases, with a single return at the bottom. indra/newview/lllocalbitmaps.cpp <http://codereview.secondlife.com/r/347/#comment1031> log an error here indra/newview/lllocalbitmaps.cpp <http://codereview.secondlife.com/r/347/#comment1027> log a warning here indra/newview/lllocalbitmaps.cpp <http://codereview.secondlife.com/r/347/#comment1033> ... then it really should be logged when it does indra/newview/lllocalbitmaps.cpp <http://codereview.secondlife.com/r/347/#comment1034> log indra/newview/lllocalbitmaps.cpp <http://codereview.secondlife.com/r/347/#comment1035> There's no reason to put block scope in a case statement unless you're going to declare something that needs to be local to that case. indra/newview/lllocalbitmaps.cpp <http://codereview.secondlife.com/r/347/#comment1036> replace the early returns with breaks, and return at the bottom. indra/newview/lllocalbitmaps.cpp <http://codereview.secondlife.com/r/347/#comment1037> I bet you can guess what I want here - Oz On Sept. 4, 2011, 8:22 a.m., Vaalith Jinn wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/347/ > ----------------------------------------------------------- > > (Updated Sept. 4, 2011, 8:22 a.m.) > > > Review request for Viewer. > > > Summary > ------- > > Local Bitmaps is a mechanism to locally load images into the viewer, track > them and optionally (per each image) > have it check if the image has been overwritten locally and if so - update it > in the viewer and inworld. > > This change affects the texture picker - adding radio checks that let the > user choose between the "Inventory" (regular inventory) and "Local" tabs > (list of locally added files). > > * Only tested this myself, as opposed to the previous implementation which > has been live for, uh, a year, > this one is completely new, written mostly from scratch and uses different > mechanisms > > ** This is strictly a review/alpha version, but i'd appreciate feedback on it > before i finalize it. > > > This addresses bug STORM-64. > http://jira.secondlife.com/browse/STORM-64 > > > Diffs > ----- > > doc/contributions.txt 8da01486a36a > indra/newview/CMakeLists.txt 8da01486a36a > indra/newview/lllocalbitmaps.h PRE-CREATION > indra/newview/lllocalbitmaps.cpp PRE-CREATION > indra/newview/lltexturectrl.h 8da01486a36a > indra/newview/lltexturectrl.cpp 8da01486a36a > indra/newview/llviewertexturelist.h 8da01486a36a > indra/newview/llwearable.h 8da01486a36a > indra/newview/llwearable.cpp 8da01486a36a > indra/newview/skins/default/xui/en/floater_texture_ctrl.xml 8da01486a36a > > Diff: http://codereview.secondlife.com/r/347/diff > > > Testing > ------- > > Texture/Sculptmap/Avatar Layer show. > Texture/Sculptmap/Avatar Layer update. > Multiple sculpties update. > > > Thanks, > > Vaalith > >
_______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
