Looks good overall, I only have a minor point.

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

Frankly speaking, I'm not a fan of adding another setting to only use it as a global variable.

I would search for a more proper way, maybe adding get/setMapServerURL() methods to LLWorldMap.
Perhaps a person more familiar with the world map code than me would suggest a better approach.

1. Aleric Inglewood Dec. 23, 2010, 5:45 a.m. (Dec. 23, 2010, 5:45 a.m.)

   The difference is, if I understand this snippet correctly, that a new debug setting like CurrentMapServerURL, would survive logout and still be there when logging in (on a different grid), while if you store it in an object (ie LLWorldMap) then it would not be persistent.
   In that light, I agree with Vadim that the use of a new settings is not correct here.
   

2. Merov Linden Dec. 27, 2010, 9:41 p.m. (Dec. 27, 2010, 9:41 p.m.)

   I don't like it either but, unfortunately, the LLWorldMap is lazy instantiated in LLFloaterWorldMap::createWorldMapView() and I can't guarantee that it exists at that point in llstartup.cpp. So I either store the map_server_url response in an ad-hoc global or, as I did, in a setting which a different sort of global when you think about it but somewhat cleaner (documented at least and with specific usage). I choose the second solution.
   
   

3. Vadim ProductEngine Dec. 28, 2010, 3:39 a.m. (Dec. 28, 2010, 3:39 a.m.)

   I see. Then what about storing the URL in a static member of LLWorldMap (and using static getter/setter), so we don't need it to be instantiated? Or even store in LLWorldMipmap?

4. Merov Linden Dec. 28, 2010, 10:12 a.m. (Dec. 28, 2010, 10:12 a.m.)

   Hmmm... That'll require to include llworldmipmap.h or llworldmap.h in llstartup.cpp, creating yet another dependency. It's possible but I don't like it. Hard coupling (explicit calls between object instances) can become issues later, with too many code snippets knowing too many objects by name just because they need one generic access to them. Here, llstartup shouldn't have any notion of the existence of a map. It just gets some data at login from the server and save them for whoever needs them later. I prefer soft coupling where a general context is created (here by settings) and have object instances checking this context when needed. That scheme has issues also but it's more flexible. Imagine for instance what we throw the map UI and use a web based panel to display the map in the viewer (likely scenario BTW). Whoever code that will simply get the root URL from the settings, ignoring when and how it got there and ignoring the old llworldmipmap. If we create a static in llworldmipmap, that devs will have to keep llworldmipmap around or relocate that static in a new object, therefore modifying llstartup.
   The bottom line I think is that llstartup shouldn't be concerned by the existence of a map object. It receives some info at login and should save them in a global context. May be we should have a global "grid" context/global but I don't think we have one and the closest thing to it is a set of data saved in settings.

5. Vadim ProductEngine Dec. 28, 2010, 11:41 a.m. (Dec. 28, 2010, 11:41 a.m.)

   Right, sounds reasonable.