A couple of things to look at, but overall very very nice looking code.

indra/newview/lllocalbitmaps.h (Diff revision 1)

Minor point ... it would be clearer to have the parameter be an enum whose values were first_use and texture_updated or something.  

Just because something only has two values doesn't make it a 'bool'.

indra/newview/lllocalbitmaps.cpp (Diff revision 1)

Would an LL_WARN be appropriate here?

indra/newview/lllocalbitmaps.cpp (Diff revision 1)

The cases in this switch all fall through if something unexpected happens... and there's no code to handle the unexpected.

If falling through is intentional, there should be a comment saying so, and if not the break should follow the end of the (missing) else block.

indra/newview/lllocalbitmaps.cpp (Diff revision 1)

you could shorten this loop by adding ' && add_object ' to the termination condition

indra/newview/lllocalbitmaps.cpp (Diff revision 1)

clearer to put the assignment in the first clause of the for loop

indra/newview/lllocalbitmaps.cpp (Diff revision 1)

I'd prefer a single 'return result;' at the bottom, replacing all those scattered ones with 'break;' because it's easier to set breakpoints on.

indra/newview/lllocalbitmaps.h (Diff revision 1)

For clarity's sake i kept private enums block below where it's currently at.

If i were to add an enum value in here i'd have to either move the whole block up to keep consistency or break consistency and write in a single enum below the main public block..

Neither is a really pretty solution as far as readability is concerned.

Is it really worth it?

1. Boroondas Gupte March 30, 2012, 5:20 p.m. (March 30, 2012, 5:20 p.m.)

   I think it'd be worth it, as it'd make the calling code much more readable. See http://doc.qt.nokia.com/qq/qq13-apis.html#thebooleanparametertrap
   
   Note that in the example at hand, the function will only be called with boolean literals, not boolean variables, so we don't have the problem of forcing additional if-else code onto the caller. Also, half if not all of the lengthy comment above the one place where you call the function with "true" could be saved if a well-named constant or enum would be passed instead.

2. Vaalith Jinn March 30, 2012, 6:38 p.m. (March 30, 2012, 6:38 p.m.)

   Um... several questions:
   
   A) Unless i am misreading, are you implying that an enum would make it easier to understand than the existing albeit lengthy comment?
   
   B) The way i designed it is actually the way one would see the mechanism: The updateself function updates by default, it's what it does no more no less.
   The bool is there to indicate a special case, special because in the life of a single local bitmap object it is only used once and that is during it's creation.
   
   So to me it seems that forcing the function to take a "run regularly" enum parameter during each regular, non first-run call would be something akin to stating the obvious,
   which would seem redundant to me to the point of causing confusion.

3. Boroondas Gupte March 31, 2012, 2:57 a.m. (March 31, 2012, 2:57 a.m.)

   A) Yes, that's what I'm implying. It might not be true for every reader, just as there is no single writing style that can be considered more understandable to everyone then every other writing style. But for many (me included), switching between reading comments written in natural language and code in a programming language requires a little effort, so one tends to either read just the code, skipping over just the comments, or the comments, skipping over the code. A good use of both seems to try explaining *what* you do with the code itself and *why* you do it (if not obvious) in the comments.
   
   The advantage for self-explanatory code over comments it that it is precise, up-to-date and "true". If code gets changed and comments are not adapted accordingly, remaining out-of-date comments might confuse or even mislead the reader. (Maybe unlikely to become an issue right here, but this is an important point I took away from reading Robert C. Martin's "Clean Code" book.)
   
   B) Can't enum arguments have default values, too?

4. Oz Linden March 31, 2012, 1:15 p.m. (March 31, 2012, 1:15 p.m.)

   Since the enum would be part of the interface to the public function, it shouldn't be private, and belongs right there next to the method:
   
   typedef enum { FIRST_USE, REPLACING } UpdateType;
   bool updateSelf(UpdateType usage);
   
   and then the test inside the method becomes:
   
   if (FIRST_USE == usage)
   

indra/newview/lllocalbitmaps.cpp (Diff revision 2)

Need 'break;' here.

indra/newview/lllocalbitmaps.cpp (Diff revision 2)

Need 'break;' here.

indra/newview/lllocalbitmaps.cpp (Diff revision 2)

Need 'break;' here.

indra/newview/lllocalbitmaps.cpp (Diff revision 2)

log the fact that it's an unknown wearable type here?