[opensource-dev] Review Request: VWR-20962 CTRL-\ Last chatter

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/16/
---

Review request for Viewer.


Summary
---

You have to push CTRL-\(there is a menu item for this too) twice to get the 
last chatter function to work.  My fix unlocks the camera before moving it.


Diffs
-

  indra/newview/llagentcamera.cpp 3d2e71443c58 

Diff: http://codereview.secondlife.com/r/16/diff


Testing
---


Thanks,

Jonathan

___
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

[opensource-dev] Review Request: Settings.xml: redundant entries and unnecessary tag

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/18/
---

Review request for Viewer.


Summary
---

I wrote two programs that use settings.xml to produce this massive table:
http://wiki.secondlife.com/wiki/Debug_Settings

While doing this I found 4 places with duplicate entries and 1 entry that is 
repeated 4 times.  There is also a pair of unnecessary tags.

Having these cleaned out would make running my program, and thus updating the 
wiki table, easier.

I've erased all but the last entry for these redundant debug settings.


This addresses bug vwr-24100.
http://jira.secondlife.com/browse/vwr-24100


Diffs
-

  indra/newview/app_settings/settings.xml 46a990f8296f 

Diff: http://codereview.secondlife.com/r/18/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: Settings.xml: redundant entries and unnecessary tag

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/18/#review40
---



indra/newview/app_settings/settings.xml
<http://codereview.secondlife.com/r/18/#comment20>

I agree with your observation that the first (and deleted) record has a 
better Comment string.  But without looking at the code I do not want to go 
making changes to the wording--my goal was just to eliminate obvious duplicates.



indra/newview/app_settings/settings.xml
<http://codereview.secondlife.com/r/18/#comment21>

The viewer uses the last record of a repeated group, so it in this case it 
is using 10.0 (just to be sure I started the viewer and looked).



indra/newview/app_settings/settings.xml
<http://codereview.secondlife.com/r/18/#comment19>

If I had to make a guess I would say that the 2nd entry for Outbandwidth 
was the original and that someone copied it up 1 level and put in a comment 
"Expand render stats display" but forgot to change the associated key.  One 
could argue that having a F32 is wrong and that it should be an unsigned 
integer, depending on what units are being used (i.e. a limit of 3.25 makes 
sense if you are using kb/sec).


- Jonathan


On 2010-12-14 12:39:34, Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/18/
> ---
> 
> (Updated 2010-12-14 12:39:34)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> I wrote two programs that use settings.xml to produce this massive table:
> http://wiki.secondlife.com/wiki/Debug_Settings
> 
> While doing this I found 4 places with duplicate entries and 1 entry that is 
> repeated 4 times.  There is also a pair of unnecessary tags.
> 
> Having these cleaned out would make running my program, and thus updating the 
> wiki table, easier.
> 
> I've erased all but the last entry for these redundant debug settings.
> 
> 
> This addresses bug vwr-24100.
> http://jira.secondlife.com/browse/vwr-24100
> 
> 
> Diffs
> -
> 
>   indra/newview/app_settings/settings.xml 46a990f8296f 
> 
> Diff: http://codereview.secondlife.com/r/18/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: VWR-20962 CTRL-\ Last chatter

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/16/
---

(Updated 2010-12-22 11:18:28.916125)


Review request for Viewer.


Changes
---

Aleric showed me a slight optimization in the two function calls I moved to fix 
this issue. In llagentcamera.cpp:

setFocusoOnAvatar(FALSE, FALSE) (was FALSE, TRUE)


Summary
---

You have to push CTRL-\(there is a menu item for this too) twice to get the 
last chatter function to work.  My fix unlocks the camera before moving it.


This addresses bug vwr-20962.
http://jira.secondlife.com/browse/vwr-20962


Diffs (updated)
-

  indra/newview/llagentcamera.cpp 0a1d00f86446 

Diff: http://codereview.secondlife.com/r/16/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: VWR-20962 CTRL-\ Last chatter

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/16/
---

(Updated 2010-12-22 11:18:45.942539)


Review request for Viewer.


Summary
---

You have to push CTRL-\(there is a menu item for this too) twice to get the 
last chatter function to work.  My fix unlocks the camera before moving it.


This addresses bug STORM-785.
http://jira.secondlife.com/browse/STORM-785


Diffs
-

  indra/newview/llagentcamera.cpp 0a1d00f86446 

Diff: http://codereview.secondlife.com/r/16/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-785 CTRL-\ Last chatter

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/16/
---

(Updated 2010-12-22 11:21:04.403780)


Review request for Viewer.


Summary (updated)
---

You have to push CTRL-\(there is a menu item for this too) twice to get the 
last chatter function to work.  My fix unlocks the camera before moving it.


This addresses bug STORM-785.
http://jira.secondlife.com/browse/STORM-785


Diffs
-

  indra/newview/llagentcamera.cpp 0a1d00f86446 

Diff: http://codereview.secondlife.com/r/16/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: VWR-24100 Settings.xml: redundant entries and unnecessary tag

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/18/
---

(Updated 2010-12-23 12:12:39.928040)


Review request for Viewer.


Summary (updated)
---

I wrote two programs that use settings.xml to produce this massive table:
http://wiki.secondlife.com/wiki/Debug_Settings

While doing this I found 4 places with duplicate entries and 1 entry that is 
repeated 4 times.  There is also a pair of unnecessary tags.

Having these cleaned out would make running my program, and thus updating the 
wiki table, easier.

I've erased all but the last entry for these redundant debug settings.


This addresses bug vwr-24100.
http://jira.secondlife.com/browse/vwr-24100


Diffs
-

  indra/newview/app_settings/settings.xml 46a990f8296f 

Diff: http://codereview.secondlife.com/r/18/diff


Testing
---


Thanks,

Jonathan

___
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

[opensource-dev] Review Request: STORM-737 Add "+" menu to Inventory/Recent

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/65/
---

Review request for Viewer.


Summary
---

This change enables the "+" menu in Inventory/Recent
It grays out "New Folder" in this menu
It enables identical menu entries when you right click on an inventory item.

Question:
 Is graying out "New Folder" best done where I am doing it now -- in 
llpanelmaininventory.cpp / LLPanelMainInventory::onAddButtonClick()


This addresses bug storm-737.
http://jira.secondlife.com/browse/storm-737


Diffs
-

  doc/contributions.txt e843e274fa58 
  indra/newview/llinventorybridge.cpp e843e274fa58 
  indra/newview/llpanelmaininventory.cpp e843e274fa58 

Diff: http://codereview.secondlife.com/r/65/diff


Testing
---

I opened up Inventory/My Inventory and used all the "New xxx" options for both 
right clicking on an inventory item and also from the "+" menu.

I then changed to the Recent tab and performed the same steps.

New items were created as expected, except "New Folder" was not an option via 
either method when the Recent tab was active.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-737 Add "+" menu to Inventory/Recent

[Jonathan Yap]

> On 2010-12-24 10:55:28, Vadim ProductEngine wrote:
> > indra/newview/llpanelmaininventory.cpp, line 509
> > <http://codereview.secondlife.com/r/65/diff/1/?file=274#file274line509>
> >
> > This line can now be removed at all.

Line 509 on the left is removed; it might be that the diff display is slightly 
confused about this matter.  Not sure what a lighter color of pink means.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/65/#review85
---


On 2010-12-23 13:25:31, Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/65/
> ---
> 
> (Updated 2010-12-23 13:25:31)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This change enables the "+" menu in Inventory/Recent
> It grays out "New Folder" in this menu
> It enables identical menu entries when you right click on an inventory item.
> 
> Question:
>  Is graying out "New Folder" best done where I am doing it now -- in 
> llpanelmaininventory.cpp / LLPanelMainInventory::onAddButtonClick()
> 
> 
> This addresses bug storm-737.
> http://jira.secondlife.com/browse/storm-737
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt e843e274fa58 
>   indra/newview/llinventorybridge.cpp e843e274fa58 
>   indra/newview/llpanelmaininventory.cpp e843e274fa58 
> 
> Diff: http://codereview.secondlife.com/r/65/diff
> 
> 
> Testing
> ---
> 
> I opened up Inventory/My Inventory and used all the "New xxx" options for 
> both right clicking on an inventory item and also from the "+" menu.
> 
> I then changed to the Recent tab and performed the same steps.
> 
> New items were created as expected, except "New Folder" was not an option via 
> either method when the Recent tab was active.
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-737 Add "+" menu to Inventory/Recent

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/65/#review89
---



indra/newview/llpanelmaininventory.cpp
<http://codereview.secondlife.com/r/65/#comment63>

I updated the code per Aleric's suggestion but the 2nd diff failed to 
upload.  There must be some unusual hg command to generate follow-up diffs in a 
format RB will be happy with.  You can see the change on bitbucket though.


- Jonathan


On 2010-12-23 13:25:31, Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/65/
> ---
> 
> (Updated 2010-12-23 13:25:31)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This change enables the "+" menu in Inventory/Recent
> It grays out "New Folder" in this menu
> It enables identical menu entries when you right click on an inventory item.
> 
> Question:
>  Is graying out "New Folder" best done where I am doing it now -- in 
> llpanelmaininventory.cpp / LLPanelMainInventory::onAddButtonClick()
> 
> 
> This addresses bug storm-737.
> http://jira.secondlife.com/browse/storm-737
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt e843e274fa58 
>   indra/newview/llinventorybridge.cpp e843e274fa58 
>   indra/newview/llpanelmaininventory.cpp e843e274fa58 
> 
> Diff: http://codereview.secondlife.com/r/65/diff
> 
> 
> Testing
> ---
> 
> I opened up Inventory/My Inventory and used all the "New xxx" options for 
> both right clicking on an inventory item and also from the "+" menu.
> 
> I then changed to the Recent tab and performed the same steps.
> 
> New items were created as expected, except "New Folder" was not an option via 
> either method when the Recent tab was active.
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

[opensource-dev] Review Request: VWR-24347 Reversion in Copy3rdPartyLibs.cmake -- cannot find msvc* files using VS 2005 Express

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/68/
---

Review request for Viewer.


Summary
---

Environment: Windows, VS 2005 Express

Back in the days of v2.1 I was able to supply the following option to 
develop.py, which would allow me to specify the location of
msvcr80.dll
msvcp80.dll
Microsoft.VC80.CRT.manifest

-DMSVC_REDIST_PATH:PATH=E:/some/path

There was a similar code path for the debug files
-DMSVC_DEBUG_REDIST_PATH=E:/some/path

This files cannot be found by the Express compiler so without a way of telling 
the compiler where to find them they have to be dropped into the build tree 
manually.

At some point Copy3rdPartyLibs.cmake was rewritten and this option was dropped.


This addresses bug vwr-24347.
http://jira.secondlife.com/browse/vwr-24347


Diffs
-

  indra/cmake/Copy3rdPartyLibs.cmake 27dae7b01a81 

Diff: http://codereview.secondlife.com/r/68/diff


Testing
---

I tried using -DMSVC_REDIST_PATH:PATH=E:/some/path with my VS 2005 Express 
compiler and obtained the desired result. 

I don't have the debug files to test -DMSVC_DEBUG_REDIST_PATH=E:/some/path


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: VWR-24100 Settings.xml: redundant entries and unnecessary tag

[Jonathan Yap]

> On Dec. 16, 2010, 8:04 a.m., Aleric Inglewood wrote:
> > indra/newview/app_settings/settings.xml, line 1165
> > <http://codereview.secondlife.com/r/18/diff/1/?file=60#file60line1165>
> >
> > Considering the setting name (CacheLocationTopFolder), isn't the 
> > deleted Comment string better than the one you left in? Thus, Controls the 
> > top folder location of the the local disk cache, rather than Controls the 
> > location of the local disk cache. Diff is ok with me, I just wondered if 
> > you saw that it wasn't an exact duplicate.

I adjusted the file per your observation.  Apparently this setting was cloned 
from the one above it (CacheLocation) twice and the comment only updated in one 
of the copies.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/18/#review32
-------


On Dec. 23, 2010, 12:12 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/18/
> ---
> 
> (Updated Dec. 23, 2010, 12:12 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> I wrote two programs that use settings.xml to produce this massive table:
> http://wiki.secondlife.com/wiki/Debug_Settings
> 
> While doing this I found 4 places with duplicate entries and 1 entry that is 
> repeated 4 times.  There is also a pair of unnecessary tags.
> 
> Having these cleaned out would make running my program, and thus updating the 
> wiki table, easier.
> 
> I've erased all but the last entry for these redundant debug settings.
> 
> 
> This addresses bug vwr-24100.
> http://jira.secondlife.com/browse/vwr-24100
> 
> 
> Diffs
> -
> 
>   indra/newview/app_settings/settings.xml 46a990f8296f 
> 
> Diff: http://codereview.secondlife.com/r/18/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

[opensource-dev] Review Request: STORM-829 Viewer 2 does not parse /me in object Instant Messages

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/71/
---

Review request for Viewer.


Summary
---

The "/me" in the lsl code below would be displayed rather than being translated 
to a name:
llInstantMessage(llGetOwner(),"/me Hello, Avatar!");


This addresses bug STORM-829.
http://jira.secondlife.com/browse/STORM-829


Diffs
-

  indra/newview/llviewermessage.cpp 845cab866155 

Diff: http://codereview.secondlife.com/r/71/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-829 Viewer 2 does not parse /me in object Instant Messages

[Jonathan Yap]

> On Jan. 5, 2011, 7:33 p.m., Wolfpup Lowenhar wrote:
> > indra/newview/llviewermessage.cpp, line 2752
> > <http://codereview.secondlife.com/r/71/diff/1/?file=327#file327line2752>
> >
> > Looks good to me, but just wondering why your checking for "/me " and 
> > "/me'" .

That is the check used in other parts of the code and as was pointed out in the 
mailing list you can type /me's happy


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/71/#review114
-------


On Jan. 5, 2011, 6:14 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/71/
> ---
> 
> (Updated Jan. 5, 2011, 6:14 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> The "/me" in the lsl code below would be displayed rather than being 
> translated to a name:
> llInstantMessage(llGetOwner(),"/me Hello, Avatar!");
> 
> 
> This addresses bug STORM-829.
> http://jira.secondlife.com/browse/STORM-829
> 
> 
> Diffs
> -
> 
>   indra/newview/llviewermessage.cpp 845cab866155 
> 
> Diff: http://codereview.secondlife.com/r/71/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-829 Viewer 2 does not parse /me in object Instant Messages

[Jonathan Yap]

> On Jan. 6, 2011, 7 a.m., Aleric Inglewood wrote:
> > What about /Me, /ME or /me followed by another punctuation? Ie, "/me?", 
> > "/me!", etc...
> > Just asking because these comparisions with just "/me " and "/me'" seem 
> > very limited,
> > almost weird. More logical would be to not check anything at ALL - and 
> > either expand
> > things, or not. What happens if you just set a flag saying "whatever is in 
> > this
> > string, don't expand /me, /who, /whois, /kick etc" without at that point 
> > checking
> > for one specific case (missing possibly many other variations).
> >

If it was me writing the original code I would not have made it case-sensitive, 
but as this is a bug fix and not a new feature I am following the current 
design of having just /me work.  


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/71/#review120
---


On Jan. 5, 2011, 6:14 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/71/
> ---
> 
> (Updated Jan. 5, 2011, 6:14 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> The "/me" in the lsl code below would be displayed rather than being 
> translated to a name:
> llInstantMessage(llGetOwner(),"/me Hello, Avatar!");
> 
> 
> This addresses bug STORM-829.
> http://jira.secondlife.com/browse/STORM-829
> 
> 
> Diffs
> -
> 
>   indra/newview/llviewermessage.cpp 845cab866155 
> 
> Diff: http://codereview.secondlife.com/r/71/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: VWR-24347 Reversion in Copy3rdPartyLibs.cmake -- cannot find msvc* files using VS 2005 Express

[Jonathan Yap]

> On Jan. 4, 2011, 4:14 p.m., Merov Linden wrote:
> > Those lines have been missing since a long time: the first version in hg is 
> > already missing them:
> > https://bitbucket.org/lindenlab/viewer-development/changeset/07304583316d

They are not in that early version, but they are (plus code for VS2008) in a 
V2.1 copy I have of that file, but at some point were taken out again.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/68/#review111
---


On Dec. 29, 2010, 3:42 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/68/
> ---
> 
> (Updated Dec. 29, 2010, 3:42 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Environment: Windows, VS 2005 Express
> 
> Back in the days of v2.1 I was able to supply the following option to 
> develop.py, which would allow me to specify the location of
> msvcr80.dll
> msvcp80.dll
> Microsoft.VC80.CRT.manifest
> 
> -DMSVC_REDIST_PATH:PATH=E:/some/path
> 
> There was a similar code path for the debug files
> -DMSVC_DEBUG_REDIST_PATH=E:/some/path
> 
> This files cannot be found by the Express compiler so without a way of 
> telling the compiler where to find them they have to be dropped into the 
> build tree manually.
> 
> At some point Copy3rdPartyLibs.cmake was rewritten and this option was 
> dropped.
> 
> 
> This addresses bug vwr-24347.
> http://jira.secondlife.com/browse/vwr-24347
> 
> 
> Diffs
> -
> 
>   indra/cmake/Copy3rdPartyLibs.cmake 27dae7b01a81 
> 
> Diff: http://codereview.secondlife.com/r/68/diff
> 
> 
> Testing
> ---
> 
> I tried using -DMSVC_REDIST_PATH:PATH=E:/some/path with my VS 2005 Express 
> compiler and obtained the desired result. 
> 
> I don't have the debug files to test -DMSVC_DEBUG_REDIST_PATH=E:/some/path
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

[opensource-dev] Review Request: STORM-830 Volume slider isn't properly remembered if set to zero

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/72/
---

Review request for Viewer.


Summary
---

There is an edge case in setMasterGain during startup which prevents 
setInternalGain from being called if the master volume setting and 
mInternalGain both equal 0.

Setting mInternalGain to a very low but non-zero value fixes this issue.


This addresses bug STORM-830.
http://jira.secondlife.com/browse/STORM-830


Diffs
-

  indra/llaudio/llaudioengine.cpp 6d44f0d85a80 

Diff: http://codereview.secondlife.com/r/72/diff


Testing
---

In Preferences / Sound & Media tested:
Buttons
Ambient
Sound Effects
Stream Music
Media
Voice Chat


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-829 Viewer 2 does not parse /me in object Instant Messages

[Jonathan Yap]

> On Jan. 6, 2011, 7 a.m., Aleric Inglewood wrote:
> > What about /Me, /ME or /me followed by another punctuation? Ie, "/me?", 
> > "/me!", etc...
> > Just asking because these comparisions with just "/me " and "/me'" seem 
> > very limited,
> > almost weird. More logical would be to not check anything at ALL - and 
> > either expand
> > things, or not. What happens if you just set a flag saying "whatever is in 
> > this
> > string, don't expand /me, /who, /whois, /kick etc" without at that point 
> > checking
> > for one specific case (missing possibly many other variations).
> >
> 
> Jonathan Yap wrote:
> If it was me writing the original code I would not have made it 
> case-sensitive, but as this is a bug fix and not a new feature I am following 
> the current design of having just /me work.
> 
> Aleric Inglewood wrote:
> I didn't suggest to make it case insensitive, I wondered what happens
> when you use /ME instead of /me with and without the patch.
> And I wonder why it is necessary at all to compare a string with "/me ".
> At the very least that indicates code duplication.
> 
> Let me clarify,
> 
> void do_it(std::string const& str)
> {
>   if (!flag && str == "/me")
> ...
>   else
> ...
> }
> 
> Bad code:
> 
> if (str == "/me")
>   flag = 1;
> do_it(str);
> 
> ---
> 
> Code that makes more sense:
> 
> flag = 1;
> do_it(str);
> 
> 
> But keep in mind that I didn't look at the actual code ;). I just looked 
> at your patch.
>

You have to check for /me with a blank after it as anything else would be 
processed as a potential gesture.  This "/me " and "/me'" testing is scattered 
throughout the chat handling code.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/71/#review120
---


On Jan. 5, 2011, 6:14 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/71/
> ---
> 
> (Updated Jan. 5, 2011, 6:14 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> The "/me" in the lsl code below would be displayed rather than being 
> translated to a name:
> llInstantMessage(llGetOwner(),"/me Hello, Avatar!");
> 
> 
> This addresses bug STORM-829.
> http://jira.secondlife.com/browse/STORM-829
> 
> 
> Diffs
> -
> 
>   indra/newview/llviewermessage.cpp 845cab866155 
> 
> Diff: http://codereview.secondlife.com/r/71/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-829 Viewer 2 does not parse /me in object Instant Messages

[Jonathan Yap]

> On Jan. 7, 2011, 9:36 a.m., Joshua Linden wrote:
> > I believe Aleric's comment is accurate. Logic testing for a prefix should 
> > be removed from the patch, and the flag should simply always be specified 
> > in this case. 
> > 
> > It is notable that the flag does trigger exactly the same test that is 
> > present in the patch (i.e. it's not case sensitive, it replicates prefix 
> > testing in several other places in the code base, etc). A more general fix 
> > might be to refactor all of the places that do prefix testing, but that 
> > wouldn't affect this specific issue. Again, the patch should be reduced to 
> > one line that simply adds the desired flag.

Joshua, if the check for "/me " and "/me'" were not present and CHAT_STYLE_IRC 
was always passed in then all llInstantMessages from objects would be treated 
as if /me had been sent.  I don't think this is what is desired.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/71/#review132
---


On Jan. 5, 2011, 6:14 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/71/
> ---
> 
> (Updated Jan. 5, 2011, 6:14 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> The "/me" in the lsl code below would be displayed rather than being 
> translated to a name:
> llInstantMessage(llGetOwner(),"/me Hello, Avatar!");
> 
> 
> This addresses bug STORM-829.
> http://jira.secondlife.com/browse/STORM-829
> 
> 
> Diffs
> -
> 
>   indra/newview/llviewermessage.cpp 845cab866155 
> 
> Diff: http://codereview.secondlife.com/r/71/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-830 Volume slider isn't properly remembered if set to zero

[Jonathan Yap]

> On Jan. 6, 2011, 5:37 p.m., Aleric Inglewood wrote:
> > This is really not how you want to deal with this bug :/.  It's a known 
> > fact that audio mixers are very bad with low volumes. Setting a volume to 0 
> > (or something really small) can put a very high load on the CPU for the 
> > audio mixer, which causes severe problems. See 
> > https://jira.secondlife.com/browse/VWR-14914
> > 
> > So, on the contrary (as VWR-14914 fixed a horrible bug that made FPS drop 
> > drastically): when the volume is set to 0 (or even close to zero) the audio 
> > channel has to be muted and not mixed, ever. Assuming that VWR-14914 is in 
> > Viewer 2, and wasn't broken in the meantime, a volume of 0 would cause the 
> > channel to be muted, but setting it to 0.01 will not cause it to be 
> > muted, but result in a high CPU load.
> >
> 
> Aleric Inglewood wrote:
> After discussion on IRC Jonathan explained to me what this is all about. 
> The patch is correct.
> Nevertheless, I was confused about the fact that this value of 0.01 
> is never going to be USED.
> Perhaps a different value and comment can avoid that in the future when 
> other coders look at it.
> mInternalGain is only ever compared with, and the whole point of this 
> patch is to make sure
> that the first comparison fails (I now understand). A common value used 
> to make sure that
> a first comparison fails is a value outside the valid range of that 
> variable. The valid
> range being [0, 1], I'd suggest using -1 and adding a comment in the 
> lines of:
> 
> // Make sure that the first call to setMasterGain will cause 
> setInternalGain to be called.
> mInternalGain = -1.f;
>
> 
> Kent Quirk wrote:
> Aleric is correct, and if the approach is technically possible it would 
> be preferred to use an out-of-range value. It appears to me that his 
> suggested fix will work, but I haven't tried it myself. Jonathan, can you try 
> it, please?
>

I changed the initial value of mInternalGain to -1, did a quick test with key 
clicks, and saw that that fixes the problem, too.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/72/#review128
---


On Jan. 6, 2011, 2:37 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/72/
> ---
> 
> (Updated Jan. 6, 2011, 2:37 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> There is an edge case in setMasterGain during startup which prevents 
> setInternalGain from being called if the master volume setting and 
> mInternalGain both equal 0.
> 
> Setting mInternalGain to a very low but non-zero value fixes this issue.
> 
> 
> This addresses bug STORM-830.
> http://jira.secondlife.com/browse/STORM-830
> 
> 
> Diffs
> -
> 
>   indra/llaudio/llaudioengine.cpp 6d44f0d85a80 
> 
> Diff: http://codereview.secondlife.com/r/72/diff
> 
> 
> Testing
> ---
> 
> In Preferences / Sound & Media tested:
> Buttons
> Ambient
> Sound Effects
> Stream Music
> Media
> Voice Chat
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

[opensource-dev] Review Request: Storm-844 "More" should be "Less" when Media Control is open

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/78/
---

Review request for Viewer.


Summary
---

"More" should be "Less" when Media Control is open

This is a trivial text change in an xml file.  The reason I am posting this 
here is due to some confusion using label_selected.  In this case having it set 
to a different value than when label is set to seems to have no effect, so I 
have made them identical.

I scanned all the xml files and there are only about 5 places where 
label_selected is different from the preceding label= value.

Is there any reason to revert back to having them set to different values?
i.e. label="More" and label_selected="Less"


This addresses bug storm-844.
http://jira.secondlife.com/browse/storm-844


Diffs
-

  doc/contributions.txt 179e0754a27d 
  indra/newview/skins/default/xui/en/panel_nearby_media.xml 179e0754a27d 

Diff: http://codereview.secondlife.com/r/78/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: Storm-844 "More" should be "Less" when Media Control is open

[Jonathan Yap]

> On Jan. 13, 2011, 8:05 a.m., Twisted Laws wrote:
> > To me, this is the wrong solution.  label_selected used to work to allow a 
> > button to display different text when it was selected, so you could have a 
> > button that said More until it was pressed or selected and displayed more 
> > informaiton.  At that time the text could change to Less indicating that 
> > pressing the button a second time would maybe reduce the information 
> > displayed.  If the button in question here doesn't change when pressing, 
> > that means that something down in the control code is wrong.

I did a test with the floater preview UI (one of the few places that label is 
not the same as label_selected.  There is a >> button there that does change to 
<< when pressed.

However, in this case, there is some code that does the button swapping.  In 
llpanelnearbymedia.cpp / void LLPanelNearByMedia::onMoreLess()

getChild("more_btn")->setVisible(!is_more);
getChild("less_btn")->setVisible(is_more);

If people think refactoring rather then just rewording is the way to go please 
say so.  I always hesitate to "fix what ain't broke".

The vast majority of button definitions in the xml files have identical entries 
for label and label_selected (is having label_selected present a requirement in 
a button definition or are all these entires superfluous?).


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/78/#review148
---


On Jan. 12, 2011, 6:02 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/78/
> ---
> 
> (Updated Jan. 12, 2011, 6:02 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> "More" should be "Less" when Media Control is open
> 
> This is a trivial text change in an xml file.  The reason I am posting this 
> here is due to some confusion using label_selected.  In this case having it 
> set to a different value than when label is set to seems to have no effect, 
> so I have made them identical.
> 
> I scanned all the xml files and there are only about 5 places where 
> label_selected is different from the preceding label= value.
> 
> Is there any reason to revert back to having them set to different values?
> i.e. label="More" and label_selected="Less"
> 
> 
> This addresses bug storm-844.
> http://jira.secondlife.com/browse/storm-844
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 179e0754a27d 
>   indra/newview/skins/default/xui/en/panel_nearby_media.xml 179e0754a27d 
> 
> Diff: http://codereview.secondlife.com/r/78/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: Storm-844 "More" should be "Less" when Media Control is open

[Jonathan Yap]

> On Jan. 13, 2011, 8:05 a.m., Twisted Laws wrote:
> > To me, this is the wrong solution.  label_selected used to work to allow a 
> > button to display different text when it was selected, so you could have a 
> > button that said More until it was pressed or selected and displayed more 
> > informaiton.  At that time the text could change to Less indicating that 
> > pressing the button a second time would maybe reduce the information 
> > displayed.  If the button in question here doesn't change when pressing, 
> > that means that something down in the control code is wrong.
> 
> Jonathan Yap wrote:
> I did a test with the floater preview UI (one of the few places that 
> label is not the same as label_selected.  There is a >> button there that 
> does change to << when pressed.
> 
> However, in this case, there is some code that does the button swapping.  
> In llpanelnearbymedia.cpp / void LLPanelNearByMedia::onMoreLess()
> 
>   getChild("more_btn")->setVisible(!is_more);
>   getChild("less_btn")->setVisible(is_more);
> 
> If people think refactoring rather then just rewording is the way to go 
> please say so.  I always hesitate to "fix what ain't broke".
> 
> The vast majority of button definitions in the xml files have identical 
> entries for label and label_selected (is having label_selected present a 
> requirement in a button definition or are all these entires superfluous?).
> 
> Twisted Laws wrote:
> I looked into this and have a patch that corrects it.  What the patch 
> does in llPanelNearByMedia::onMoreLess(), the first statement is changed to 
> bool is_more = getChild("more_btn")->getToggleState();  the two 
> statements Jonathan shows are changed to just one 
> getChild("more_btn")->setVisible(true); (may just be removed and 
> not be necessary) and in the xml file, the first more_btn gets 
> is_toggle="true" added and the less_btn is removed.  My patch file is 
> attached to STORM-844.

I applied this patch and you can view it via the link to the repository in the 
jira.


- Jonathan


-------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/78/#review148
---


On Jan. 12, 2011, 6:02 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/78/
> ---
> 
> (Updated Jan. 12, 2011, 6:02 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> "More" should be "Less" when Media Control is open
> 
> This is a trivial text change in an xml file.  The reason I am posting this 
> here is due to some confusion using label_selected.  In this case having it 
> set to a different value than when label is set to seems to have no effect, 
> so I have made them identical.
> 
> I scanned all the xml files and there are only about 5 places where 
> label_selected is different from the preceding label= value.
> 
> Is there any reason to revert back to having them set to different values?
> i.e. label="More" and label_selected="Less"
> 
> 
> This addresses bug storm-844.
> http://jira.secondlife.com/browse/storm-844
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 179e0754a27d 
>   indra/newview/skins/default/xui/en/panel_nearby_media.xml 179e0754a27d 
> 
> Diff: http://codereview.secondlife.com/r/78/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: VWR-24347 Reversion in Copy3rdPartyLibs.cmake -- cannot find msvc* files using VS 2005 Express

[Jonathan Yap]

> On Jan. 17, 2011, 7:33 a.m., Oz Linden wrote:
> > Given that we are in the process of moving to VS2010, I suggest that this 
> > is not worth doing

Oz, I think it is worth doing, but doing it it a more general and 
forward-looking way, unless VS2010 Express will not have this issue. Please see 
the comment I just wrote in https://jira.secondlife.com/browse/vwr-24347 and 
let me know what you think.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/68/#review175
---


On Dec. 29, 2010, 3:42 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/68/
> ---
> 
> (Updated Dec. 29, 2010, 3:42 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Environment: Windows, VS 2005 Express
> 
> Back in the days of v2.1 I was able to supply the following option to 
> develop.py, which would allow me to specify the location of
> msvcr80.dll
> msvcp80.dll
> Microsoft.VC80.CRT.manifest
> 
> -DMSVC_REDIST_PATH:PATH=E:/some/path
> 
> There was a similar code path for the debug files
> -DMSVC_DEBUG_REDIST_PATH=E:/some/path
> 
> This files cannot be found by the Express compiler so without a way of 
> telling the compiler where to find them they have to be dropped into the 
> build tree manually.
> 
> At some point Copy3rdPartyLibs.cmake was rewritten and this option was 
> dropped.
> 
> 
> This addresses bug vwr-24347.
> http://jira.secondlife.com/browse/vwr-24347
> 
> 
> Diffs
> -
> 
>   indra/cmake/Copy3rdPartyLibs.cmake 27dae7b01a81 
> 
> Diff: http://codereview.secondlife.com/r/68/diff
> 
> 
> Testing
> ---
> 
> I tried using -DMSVC_REDIST_PATH:PATH=E:/some/path with my VS 2005 Express 
> compiler and obtained the desired result. 
> 
> I don't have the debug files to test -DMSVC_DEBUG_REDIST_PATH=E:/some/path
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: Provide define LL_MSVC10 to customize Visual Studio 10 code submissions.

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/121/#review249
---


1) You should fill in the Bugs field with a Jira number.

2) Since VS2005 used the 80 code and VS2008 used 90, should you be using 
LL_MSVC100?

- Jonathan


On Jan. 25, 2011, 2:41 a.m., Nicky Perian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/121/
> ---
> 
> (Updated Jan. 25, 2011, 2:41 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Need additional eyes on this. My first RB.
> 
> 
> This addresses bug LL_MSVC10.
> http://jira.secondlife.com/browse/LL_MSVC10
> 
> 
> Diffs
> -
> 
>   indra/llcommon/llpreprocessor.h 26c09ad4293e 
> 
> Diff: http://codereview.secondlife.com/r/121/diff
> 
> 
> Testing
> ---
> 
> Complied with VS2010. Used with another patch around a microsoft bug. Have 
> not used with VS 2005
> 
> 
> Thanks,
> 
> Nicky
> 
>

___
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

Re: [opensource-dev] Review Request: Do not fail when no scp command is found, unless it is actually needed to fetch something

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/127/#review265
---



autobuild/common.py


Why send out a warning at this point?  Developers who will never use scp 
don't benefit from this message.  If scp is needed and is not present having 
the error sent out later should be sufficient.


- Jonathan


On Jan. 28, 2011, 5:20 a.m., Oz Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/127/
> ---
> 
> (Updated Jan. 28, 2011, 5:20 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> During initialization, if there is no scp or pscp command found then 
> autobuild fails immediately.  This is true whether or not any scp urls need 
> to be used.
> 
> This change modifies the behavior so that a warning is printed if no command 
> is found, but execution proceeds until an scp command is needed, at which 
> time execution fails with an explanatory message.
> 
> This patch can print the warning multiple times - I didn't attempt to 
> suppress the extras.
> 
> 
> Diffs
> -
> 
>   autobuild/common.py f49808fe3c07 
> 
> Diff: http://codereview.secondlife.com/r/127/diff
> 
> 
> Testing
> ---
> 
> I've tested this locally, simulating the error by temporarily modifying the 
> names of the commands to be found for scp.
> 
> I have not checked it on Windows, where the original problem was found.
> 
> 
> Thanks,
> 
> Oz
> 
>

___
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

Re: [opensource-dev] Review Request: VWR-24667; Copy3rdPartyLibs.cmake needs to account for Visual Studio 10 and Visual Studio 10 Express.

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/131/#review282
---



indra/cmake/Copy3rdPartyLibs.cmake


Examine the attached file in https://jira.secondlife.com/browse/STORM-932 
and my comment at the bottom on how to avoid duplicating code in this file.


- Jonathan


On Jan. 30, 2011, 8:55 p.m., Nicky Perian wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/131/
> ---
> 
> (Updated Jan. 30, 2011, 8:55 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Copy3rdPartyLibs.cmake need to respond correctly under Visual Studio 10 and 
> Visual Studio 10 Express Edition.
> 
>  
> 
> 
> This addresses bug VWR-24667.
> http://jira.secondlife.com/browse/VWR-24667
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 691e3941d950 
>   indra/cmake/Copy3rdPartyLibs.cmake 691e3941d950 
> 
> Diff: http://codereview.secondlife.com/r/131/diff
> 
> 
> Testing
> ---
> 
> Tested good under windows 7 64 bit and Visual Studio 10 Express Edition.
> 
> 
> Thanks,
> 
> Nicky
> 
>

___
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

[opensource-dev] Review Request: VWR-22220 Chat preferences > font size should increase size of input text as well

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/139/
---

Review request for Viewer.


Summary
---

This is a request for help.  I am trying to learn more about c++ and how 
variables in one class are accessed from another.  For someone who knows what 
they are doing this is probably a pretty easy question.

I have been able to set the font size on the chat input box when it is created 
in llbottomtray.cpp.  I would like to do the same thing when someone clicks in 
that box to input text; it is possible they have changed the font setting and I 
would like to apply the size there as well, but I am stuck on how to do this.  
I think the right place to do this is in 
llchatbar.cpp/LLChatBar::onInputEditorGainFocus().

I have tried all kinds of wrong ways but at this point am stymied.

Exact steps on how to proceed would be appreciated.


This addresses bug vwr-0.
http://jira.secondlife.com/browse/vwr-0


Diffs
-

  indra/llui/lllineeditor.h 3d2e71443c58 
  indra/llui/lllineeditor.cpp 3d2e71443c58 
  indra/newview/llbottomtray.cpp 3d2e71443c58 
  indra/newview/llchatbar.cpp 3d2e71443c58 

Diff: http://codereview.secondlife.com/r/139/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: VWR-22220 Chat preferences > font size should increase size of input text as well

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/139/
---

(Updated Feb. 5, 2011, 3:08 p.m.)


Review request for Viewer.


Changes
---

Pay no attention to anything in llchatbar -- I am told that is dead code.

The font is set in bottomtray when the chat box is initially built.  I suspect 
that code can now be deleted as the font is set every time when the chat box is 
given focus in nearbychatbar.


Summary
---

This is a request for help.  I am trying to learn more about c++ and how 
variables in one class are accessed from another.  For someone who knows what 
they are doing this is probably a pretty easy question.

I have been able to set the font size on the chat input box when it is created 
in llbottomtray.cpp.  I would like to do the same thing when someone clicks in 
that box to input text; it is possible they have changed the font setting and I 
would like to apply the size there as well, but I am stuck on how to do this.  
I think the right place to do this is in 
llchatbar.cpp/LLChatBar::onInputEditorGainFocus().

I have tried all kinds of wrong ways but at this point am stymied.

Exact steps on how to proceed would be appreciated.


This addresses bug vwr-0.
http://jira.secondlife.com/browse/vwr-0


Diffs (updated)
-

  indra/llui/lllineeditor.h 3d2e71443c58 
  indra/llui/lllineeditor.cpp 3d2e71443c58 
  indra/newview/llbottomtray.cpp 3d2e71443c58 
  indra/newview/llchatbar.h 3d2e71443c58 
  indra/newview/llchatbar.cpp 3d2e71443c58 
  indra/newview/llnearbychatbar.cpp 3d2e71443c58 

Diff: http://codereview.secondlife.com/r/139/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: VWR-22220 Chat preferences > font size should increase size of input text as well

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/139/
---

(Updated Feb. 6, 2011, 12:54 a.m.)


Review request for Viewer.


Changes
---

Removed changes to bottomtray.cpp


Summary
---

This is a request for help.  I am trying to learn more about c++ and how 
variables in one class are accessed from another.  For someone who knows what 
they are doing this is probably a pretty easy question.

I have been able to set the font size on the chat input box when it is created 
in llbottomtray.cpp.  I would like to do the same thing when someone clicks in 
that box to input text; it is possible they have changed the font setting and I 
would like to apply the size there as well, but I am stuck on how to do this.  
I think the right place to do this is in 
llchatbar.cpp/LLChatBar::onInputEditorGainFocus().

I have tried all kinds of wrong ways but at this point am stymied.

Exact steps on how to proceed would be appreciated.


This addresses bug vwr-0.
http://jira.secondlife.com/browse/vwr-0


Diffs (updated)
-

  indra/llui/lllineeditor.h 3d2e71443c58 
  indra/llui/lllineeditor.cpp 3d2e71443c58 
  indra/newview/llchatbar.h 3d2e71443c58 
  indra/newview/llchatbar.cpp 3d2e71443c58 
  indra/newview/llnearbychatbar.cpp 3d2e71443c58 

Diff: http://codereview.secondlife.com/r/139/diff


Testing
---


Thanks,

Jonathan

___
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

[opensource-dev] Review Request: STORM-974 UI button alignment issues in Landmark panel, About Land window, and Hardware Settings window

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/142/
---

Review request for Viewer.


Summary
---

Please see the jira for the referenced pictures.

In picture #1, in the preferences > graphics > hardware settings, the line of 
"gamma" setting is a little too up and it collides with the "antialiasing" line.

In picture #2 in the "places" tab under "my landmarks" tab the "profile" button 
gets cut off at the end from the rest of the sidebar's UI.
There are a number of other buttons with alignment issues with no screenshot.

In picture #3 in the Access tab in the About Land window, the remove buttons 
are off to the right a few millimetres.

Changes made were to align the Hardware Settings and About Land buttons and to 
completely go over the buttons in the Landmarks panel.  The small down-arrow at 
the bottom right corner was using an imbedded UTF-8 so I fixed that and also 
made it point up while the menu is open (there is a separate jira for reversing 
the direction it points in these two states).

This patch really needs a test viewer built to make sure all works and that 
nothing was missed in the Landmarks panel.  There are 3 other related jiras 
that I hope are taken in; a test viewer could be made incorporating all 4 jiras 
at that point.


This addresses bug STORM-974.
http://jira.secondlife.com/browse/STORM-974


Diffs
-

  indra/newview/skins/default/xui/en/floater_about_land.xml b0bceb572090 
  indra/newview/skins/default/xui/en/floater_hardware_settings.xml b0bceb572090 
  indra/newview/skins/default/xui/en/panel_edit_pick.xml b0bceb572090 
  indra/newview/skins/default/xui/en/panel_places.xml b0bceb572090 

Diff: http://codereview.secondlife.com/r/142/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-974 UI button alignment issues in Landmark panel, About Land window, and Hardware Settings window

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/142/
---

(Updated Feb. 8, 2011, 5:22 a.m.)


Review request for Viewer.


Changes
---

Took out the changes to flip the down-arrow in the bottom right of the Landmark 
panel to an up-arrow when it is pushed as that change is not within the scope 
of this jira.


Summary
---

Please see the jira for the referenced pictures.

In picture #1, in the preferences > graphics > hardware settings, the line of 
"gamma" setting is a little too up and it collides with the "antialiasing" line.

In picture #2 in the "places" tab under "my landmarks" tab the "profile" button 
gets cut off at the end from the rest of the sidebar's UI.
There are a number of other buttons with alignment issues with no screenshot.

In picture #3 in the Access tab in the About Land window, the remove buttons 
are off to the right a few millimetres.

Changes made were to align the Hardware Settings and About Land buttons and to 
completely go over the buttons in the Landmarks panel.  The small down-arrow at 
the bottom right corner was using an imbedded UTF-8 so I fixed that and also 
made it point up while the menu is open (there is a separate jira for reversing 
the direction it points in these two states).

This patch really needs a test viewer built to make sure all works and that 
nothing was missed in the Landmarks panel.  There are 3 other related jiras 
that I hope are taken in; a test viewer could be made incorporating all 4 jiras 
at that point.


This addresses bug STORM-974.
http://jira.secondlife.com/browse/STORM-974


Diffs (updated)
-

  indra/newview/skins/default/xui/en/panel_places.xml b0bceb572090 

Diff: http://codereview.secondlife.com/r/142/diff


Testing
---


Thanks,

Jonathan

___
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

[opensource-dev] Review Request: STORM-977 llmediaplugintest shows up even though -DLL_TESTS:BOOL=OFF has been used

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/144/
---

Review request for Viewer.


Summary
---

llmediaplugintest shows up to be compiled even though -DLL_TESTS:BOOL=OFF has 
been used on the command line.

This cmake file does not use the call to LL_ADD_PROJECT_UNIT_TESTS that other 
cmake files do.  

LL_ADD_PROJECT_UNIT_TESTS is usually wrapped with if(LL_TESTS)

I could not figure out which lines suppress the inclusion of 
copy_plugintest_libs and llmediaplugintest into the list of what is to be built 
so wrapped the entire file around if(LL_TESTS).  

If there is some better way to more exactly target these two items please point 
it out.


This addresses bug STORM-977.
http://jira.secondlife.com/browse/STORM-977


Diffs
-

  indra/test_apps/llplugintest/CMakeLists.txt b0bceb572090 

Diff: http://codereview.secondlife.com/r/144/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: VWR-22220 Chat preferences > font size should increase size of input text as well

[Jonathan Yap]

> On Feb. 15, 2011, 6:40 a.m., Vadim ProductEngine wrote:
> > indra/newview/llchatbar.h, line 83
> > <http://codereview.secondlife.com/r/139/diff/4/?file=790#file790line83>
> >
> > Use tabs for indentation, not spaces.
> > 
> > This note seems to apply to all modified lines in this patch.

Using spaces is specified in the coding standard:
http://wiki.secondlife.com/wiki/Coding_standard#Indentation
This preference was made explicit on Feb 3.


> On Feb. 15, 2011, 6:40 a.m., Vadim ProductEngine wrote:
> > indra/newview/llchatbar.cpp, lines 543-549
> > <http://codereview.secondlife.com/r/139/diff/4/?file=791#file791line543>
> >
> > Do you only update font size on focus changes?
> > Why?
> > That doesn't look right to me.

It might not look/feel right, but the only downside to doing it this way is the 
message "Click here to chat" is not immediately updated.

If you are in the middle of typing in the chat input box and change the size 
preference you will have to click back on the box to continue typing, at which 
point the new font size is updated in the box.

The upside to doing it this way is a test for a font change is not being 
performed every frame or every keystroke.  Oz is happy with it working this way.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/139/#review362
---


On Feb. 6, 2011, 12:54 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/139/
> ---
> 
> (Updated Feb. 6, 2011, 12:54 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This is a request for help.  I am trying to learn more about c++ and how 
> variables in one class are accessed from another.  For someone who knows what 
> they are doing this is probably a pretty easy question.
> 
> I have been able to set the font size on the chat input box when it is 
> created in llbottomtray.cpp.  I would like to do the same thing when someone 
> clicks in that box to input text; it is possible they have changed the font 
> setting and I would like to apply the size there as well, but I am stuck on 
> how to do this.  I think the right place to do this is in 
> llchatbar.cpp/LLChatBar::onInputEditorGainFocus().
> 
> I have tried all kinds of wrong ways but at this point am stymied.
> 
> Exact steps on how to proceed would be appreciated.
> 
> 
> This addresses bug vwr-0.
> http://jira.secondlife.com/browse/vwr-0
> 
> 
> Diffs
> -
> 
>   indra/llui/lllineeditor.h 3d2e71443c58 
>   indra/llui/lllineeditor.cpp 3d2e71443c58 
>   indra/newview/llchatbar.h 3d2e71443c58 
>   indra/newview/llchatbar.cpp 3d2e71443c58 
>   indra/newview/llnearbychatbar.cpp 3d2e71443c58 
> 
> Diff: http://codereview.secondlife.com/r/139/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: VWR-22220 Chat preferences > font size should increase size of input text as well

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/139/
---

(Updated Feb. 15, 2011, 10:04 a.m.)


Review request for Viewer.


Changes
---

During testing found that when the viewer is started the font size in the chat 
input box is not being set, so changed llbottomtray to set it.


Summary
---

This is a request for help.  I am trying to learn more about c++ and how 
variables in one class are accessed from another.  For someone who knows what 
they are doing this is probably a pretty easy question.

I have been able to set the font size on the chat input box when it is created 
in llbottomtray.cpp.  I would like to do the same thing when someone clicks in 
that box to input text; it is possible they have changed the font setting and I 
would like to apply the size there as well, but I am stuck on how to do this.  
I think the right place to do this is in 
llchatbar.cpp/LLChatBar::onInputEditorGainFocus().

I have tried all kinds of wrong ways but at this point am stymied.

Exact steps on how to proceed would be appreciated.


This addresses bug vwr-0.
http://jira.secondlife.com/browse/vwr-0


Diffs (updated)
-

  indra/newview/llbottomtray.h 3d2e71443c58 
  indra/newview/llbottomtray.cpp 3d2e71443c58 

Diff: http://codereview.secondlife.com/r/139/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-977 llmediaplugintest shows up even though -DLL_TESTS:BOOL=OFF has been used

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/144/
---

(Updated Feb. 16, 2011, 11:32 a.m.)


Review request for Viewer.


Changes
---

Updated code per Merov's request and Boroondas' suggestion.


Summary
---

llmediaplugintest shows up to be compiled even though -DLL_TESTS:BOOL=OFF has 
been used on the command line.

This cmake file does not use the call to LL_ADD_PROJECT_UNIT_TESTS that other 
cmake files do.  

LL_ADD_PROJECT_UNIT_TESTS is usually wrapped with if(LL_TESTS)

I could not figure out which lines suppress the inclusion of 
copy_plugintest_libs and llmediaplugintest into the list of what is to be built 
so wrapped the entire file around if(LL_TESTS).  

If there is some better way to more exactly target these two items please point 
it out.


This addresses bug STORM-977.
http://jira.secondlife.com/browse/STORM-977


Diffs (updated)
-

  indra/CMakeLists.txt b0bceb572090 
  indra/test_apps/llplugintest/CMakeLists.txt b0bceb572090 

Diff: http://codereview.secondlife.com/r/144/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-977 llmediaplugintest shows up even though -DLL_TESTS:BOOL=OFF has been used

[Jonathan Yap]

> On Feb. 8, 2011, 10:41 a.m., Boroondas Gupte wrote:
> > > If there is some better way to more exactly target these two items please 
> > > point it out.
> > 
> > You should be able to get the same effect when wrapping the only place 
> > where this file is referenced in a LL_TESTS condition, i.e., change 
> > https://bitbucket.org/lindenlab/viewer-development/src/b0bceb572090/indra/CMakeLists.txt#cl-74
> >  from
> > if (NOT LINUX)
> > to
> > if (LL_TESTS AND NOT LINUX)
> > (and the same for the endif, of course)
> > 
> > Whether that's a better place, I don't know.
> > 
> > Though, I think LL_TESTS is the wrong conditional here, anyway. LL_TESTS is 
> > for enabling unit and integration tests. llplugintest however, I have 
> > learned on IRC today, seems to be a fully separate program based on and 
> > similar to uBrowser that could be used to load and test individual 
> > llmediaplugins, would it communicate with them in the same way the viewer 
> > does. (Which, according to MichelleZ, it doesn't, thus potentially 
> > misleading developers of new plugins.)
> > 
> > It should probably not be built unless explicitly requested, thus a new 
> > variable, defaulting to OFF and different from LL_TESTS would suit this 
> > much better. Or, just delete the referencing at 
> > https://bitbucket.org/lindenlab/viewer-development/src/b0bceb572090/indra/CMakeLists.txt#cl-75
> >  completely, thus unconditionally removing it from the dependencies of the 
> > viewer itself, and have those that want to build it explicitly state it as 
> > a build target.
> 
> Merov Linden wrote:
> I'm afraid that, if that app is not built by default, it will rapidly rot 
> and become unmaintainable. There is value having it built (even if not used) 
> by LL devs and TC at least (who all build with LL_TESTS on): make sure we 
> don't add viewer dependencies where we don't need any and maintain a clean 
> separation of concerns between plugins and hosts.
> 
> Boroondas Gupte wrote:
> I can see that point. Though, I'd still like a separate variable, if 
> alone to avoid people from mistaking llplugintest for a unit or integration 
> test, like I initially did. Just have that (new) variable default on ON, 
> then, rather than OFF.

New variable name aside, would it make it clearer what this module does if it 
were renamed to something more obvious?  One person pointed out to me that 
lltestplugin might be a better choice.


- Jonathan


-------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/144/#review347
---


On Feb. 16, 2011, 11:32 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/144/
> ---
> 
> (Updated Feb. 16, 2011, 11:32 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> llmediaplugintest shows up to be compiled even though -DLL_TESTS:BOOL=OFF has 
> been used on the command line.
> 
> This cmake file does not use the call to LL_ADD_PROJECT_UNIT_TESTS that other 
> cmake files do.  
> 
> LL_ADD_PROJECT_UNIT_TESTS is usually wrapped with if(LL_TESTS)
> 
> I could not figure out which lines suppress the inclusion of 
> copy_plugintest_libs and llmediaplugintest into the list of what is to be 
> built so wrapped the entire file around if(LL_TESTS).  
> 
> If there is some better way to more exactly target these two items please 
> point it out.
> 
> 
> This addresses bug STORM-977.
> http://jira.secondlife.com/browse/STORM-977
> 
> 
> Diffs
> -
> 
>   indra/CMakeLists.txt b0bceb572090 
>   indra/test_apps/llplugintest/CMakeLists.txt b0bceb572090 
> 
> Diff: http://codereview.secondlife.com/r/144/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-977 llmediaplugintest shows up even though -DLL_TESTS:BOOL=OFF has been used

[Jonathan Yap]

> On Feb. 16, 2011, 12:07 p.m., Merov Linden wrote:
> > Cleaner! :) I'm assuming you built on Windows and it doesn't create weird 
> > errors down the road.

I have built it on windows, started the viewer up, walked/flew around a bit.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/144/#review368
---


On Feb. 16, 2011, 11:32 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/144/
> ---
> 
> (Updated Feb. 16, 2011, 11:32 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> llmediaplugintest shows up to be compiled even though -DLL_TESTS:BOOL=OFF has 
> been used on the command line.
> 
> This cmake file does not use the call to LL_ADD_PROJECT_UNIT_TESTS that other 
> cmake files do.  
> 
> LL_ADD_PROJECT_UNIT_TESTS is usually wrapped with if(LL_TESTS)
> 
> I could not figure out which lines suppress the inclusion of 
> copy_plugintest_libs and llmediaplugintest into the list of what is to be 
> built so wrapped the entire file around if(LL_TESTS).  
> 
> If there is some better way to more exactly target these two items please 
> point it out.
> 
> 
> This addresses bug STORM-977.
> http://jira.secondlife.com/browse/STORM-977
> 
> 
> Diffs
> -
> 
>   indra/CMakeLists.txt b0bceb572090 
>   indra/test_apps/llplugintest/CMakeLists.txt b0bceb572090 
> 
> Diff: http://codereview.secondlife.com/r/144/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: VWR-22220 Chat preferences > font size should increase size of input text as well

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/139/
---

(Updated Feb. 18, 2011, 7:11 a.m.)


Review request for Viewer.


Changes
---

Changed indentation from spaces to tabs per request.


Summary
---

This is a request for help.  I am trying to learn more about c++ and how 
variables in one class are accessed from another.  For someone who knows what 
they are doing this is probably a pretty easy question.

I have been able to set the font size on the chat input box when it is created 
in llbottomtray.cpp.  I would like to do the same thing when someone clicks in 
that box to input text; it is possible they have changed the font setting and I 
would like to apply the size there as well, but I am stuck on how to do this.  
I think the right place to do this is in 
llchatbar.cpp/LLChatBar::onInputEditorGainFocus().

I have tried all kinds of wrong ways but at this point am stymied.

Exact steps on how to proceed would be appreciated.


This addresses bug vwr-0.
http://jira.secondlife.com/browse/vwr-0


Diffs (updated)
-

  indra/newview/llbottomtray.h 3d2e71443c58 
  indra/newview/llbottomtray.cpp 3d2e71443c58 
  indra/newview/llchatbar.h 3d2e71443c58 
  indra/newview/llchatbar.cpp 3d2e71443c58 

Diff: http://codereview.secondlife.com/r/139/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: VWR-22220 Chat preferences > font size should increase size of input text as well

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/139/#review376
---


My internet connection went dead while in the middle of updating the diffs.  
They are corrupted and I don't know how to fix them.

- Jonathan


On Feb. 18, 2011, 7:11 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/139/
> ---
> 
> (Updated Feb. 18, 2011, 7:11 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This is a request for help.  I am trying to learn more about c++ and how 
> variables in one class are accessed from another.  For someone who knows what 
> they are doing this is probably a pretty easy question.
> 
> I have been able to set the font size on the chat input box when it is 
> created in llbottomtray.cpp.  I would like to do the same thing when someone 
> clicks in that box to input text; it is possible they have changed the font 
> setting and I would like to apply the size there as well, but I am stuck on 
> how to do this.  I think the right place to do this is in 
> llchatbar.cpp/LLChatBar::onInputEditorGainFocus().
> 
> I have tried all kinds of wrong ways but at this point am stymied.
> 
> Exact steps on how to proceed would be appreciated.
> 
> 
> This addresses bug vwr-0.
> http://jira.secondlife.com/browse/vwr-0
> 
> 
> Diffs
> -
> 
>   indra/newview/llbottomtray.h 3d2e71443c58 
>   indra/newview/llbottomtray.cpp 3d2e71443c58 
>   indra/newview/llchatbar.h 3d2e71443c58 
>   indra/newview/llchatbar.cpp 3d2e71443c58 
> 
> Diff: http://codereview.secondlife.com/r/139/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

[opensource-dev] Review Request: Add optional range ring to the mini-map -- one centered on you with a radius of 20m to show local chat range

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/197/
---

Review request for Viewer.


Summary
---

Add optional range ring to the mini-map -- one centered on you with a radius of 
20m to show local chat range.

By default the range ring is off.

To turn it on you right click on the mini-map and pick the menu entry "Range 
Ring".


This addresses bug Storm-1068.
http://jira.secondlife.com/browse/Storm-1068


Diffs
-

  doc/contributions.txt aed94e854443 
  indra/newview/app_settings/settings.xml aed94e854443 
  indra/newview/llnetmap.cpp aed94e854443 
  indra/newview/skins/default/colors.xml aed94e854443 
  indra/newview/skins/default/xui/en/menu_mini_map.xml aed94e854443 

Diff: http://codereview.secondlife.com/r/197/diff


Testing
---

Tested with another avatar.  When they are outside the ring they cannot see my 
local chat and when they are inside it they can, so the ring's radius is set 
correctly.

Flew over various types of land to make sure my color choice (blue @10%) was 
always visible.

Enabled range ring, logged out and back on; range ring is still present on 
mini-map.

Panned mini-map, range ring remains centered over avatar.

Used mouse wheel to zoom in and out as far as possible.  Size of range ring 
varied with change in zoom level.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-971 'Stop Tracking' menu item is still enabled in Mini-map floater after you stopped tracking in Nearby mini-map

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/133/#review442
---



indra/newview/llnetmap.cpp


Why is a 0 (or in some other places of the code, a NULL) being passed here? 
 It seems the definition of isTracking() does nothing with this and it could be 
eliminated from the definition in the .h file and the code that calls it.


- Jonathan


On Feb. 3, 2011, 12:43 p.m., Twisted Laws wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/133/
> ---
> 
> (Updated Feb. 3, 2011, 12:43 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Sets "Stop Tracking" enabled based on tracking if true or false instead of 
> only when true.  Also made it check for menu pointer validity to remove a 
> remote crash possiblity.
> 
> 
> This addresses bug STORM-971.
> http://jira.secondlife.com/browse/STORM-971
> 
> 
> Diffs
> -
> 
>   indra/newview/llnetmap.cpp ec4a9fd30688 
> 
> Diff: http://codereview.secondlife.com/r/133/diff
> 
> 
> Testing
> ---
> 
> My testing was to open the side panel nearby, and verify that the state of 
> the menu was correct when tracking by the different methods and disabled.   
> Then leaving that open, openned a mini-map instance and repeated testing its 
> state tracking and not.  Also tried the reverse. Verified that every 
> possibility I could think of, the state of both menus was always correct.  
> 
> Correct state is Stop Tracking is enabled anytime tracking is on for avatars 
> or landmarks and is not enabled when Tracking has been stopped no matter 
> where you stopped tracking (i.e, clicking on red arrow in view window, and 
> using the various menues).
> 
> 
> Thanks,
> 
> Twisted
> 
>

___
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

Re: [opensource-dev] Review Request: Add optional range ring to the mini-map -- one centered on you with a radius of 20m to show local chat range

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/197/
---

(Updated March 12, 2011, 7:51 a.m.)


Review request for Viewer.


Changes
---

Added 2nd ring at 100 meters.
Changed menu wording.
Internally, made the ring drawing code into a separate function.


Summary
---

Add optional range ring to the mini-map -- one centered on you with a radius of 
20m to show local chat range.

By default the range ring is off.

To turn it on you right click on the mini-map and pick the menu entry "Range 
Ring".


This addresses bug Storm-1068.
http://jira.secondlife.com/browse/Storm-1068


Diffs (updated)
-

  indra/newview/app_settings/settings.xml aed94e854443 
  indra/newview/llnetmap.h aed94e854443 
  indra/newview/llnetmap.cpp aed94e854443 
  indra/newview/skins/default/colors.xml aed94e854443 
  indra/newview/skins/default/xui/en/menu_mini_map.xml aed94e854443 

Diff: http://codereview.secondlife.com/r/197/diff


Testing
---

Tested with another avatar.  When they are outside the ring they cannot see my 
local chat and when they are inside it they can, so the ring's radius is set 
correctly.

Flew over various types of land to make sure my color choice (blue @10%) was 
always visible.

Enabled range ring, logged out and back on; range ring is still present on 
mini-map.

Panned mini-map, range ring remains centered over avatar.

Used mouse wheel to zoom in and out as far as possible.  Size of range ring 
varied with change in zoom level.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: Add optional range ring to the mini-map -- one centered on you with a radius of 20m to show local chat range

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/197/
---

(Updated March 12, 2011, 7:52 a.m.)


Review request for Viewer.


Changes
---

Slight tweak to menu name.


Summary
---

Add optional range ring to the mini-map -- one centered on you with a radius of 
20m to show local chat range.

By default the range ring is off.

To turn it on you right click on the mini-map and pick the menu entry "Range 
Ring".


This addresses bug Storm-1068.
http://jira.secondlife.com/browse/Storm-1068


Diffs (updated)
-

  indra/newview/skins/default/xui/en/menu_mini_map.xml aed94e854443 

Diff: http://codereview.secondlife.com/r/197/diff


Testing
---

Tested with another avatar.  When they are outside the ring they cannot see my 
local chat and when they are inside it they can, so the ring's radius is set 
correctly.

Flew over various types of land to make sure my color choice (blue @10%) was 
always visible.

Enabled range ring, logged out and back on; range ring is still present on 
mini-map.

Panned mini-map, range ring remains centered over avatar.

Used mouse wheel to zoom in and out as far as possible.  Size of range ring 
varied with change in zoom level.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: Add optional range ring to the mini-map -- one centered on you with a radius of 20m to show local chat range

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/197/
---

(Updated March 12, 2011, 8 a.m.)


Review request for Viewer.


Changes
---

Trouble with incremental diffs.  Applying all diffs as one file.


Summary
---

Add optional range ring to the mini-map -- one centered on you with a radius of 
20m to show local chat range.

By default the range ring is off.

To turn it on you right click on the mini-map and pick the menu entry "Range 
Ring".


This addresses bug Storm-1068.
http://jira.secondlife.com/browse/Storm-1068


Diffs (updated)
-

  doc/contributions.txt aed94e854443 
  indra/newview/app_settings/settings.xml aed94e854443 
  indra/newview/llnetmap.h aed94e854443 
  indra/newview/llnetmap.cpp aed94e854443 
  indra/newview/skins/default/colors.xml aed94e854443 
  indra/newview/skins/default/xui/en/menu_mini_map.xml aed94e854443 

Diff: http://codereview.secondlife.com/r/197/diff


Testing
---

Tested with another avatar.  When they are outside the ring they cannot see my 
local chat and when they are inside it they can, so the ring's radius is set 
correctly.

Flew over various types of land to make sure my color choice (blue @10%) was 
always visible.

Enabled range ring, logged out and back on; range ring is still present on 
mini-map.

Panned mini-map, range ring remains centered over avatar.

Used mouse wheel to zoom in and out as far as possible.  Size of range ring 
varied with change in zoom level.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: VWR-22220 Chat preferences > font size should increase size of input text as well

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/139/
---

(Updated March 12, 2011, 4:33 p.m.)


Review request for Viewer.


Summary
---

This is a request for help.  I am trying to learn more about c++ and how 
variables in one class are accessed from another.  For someone who knows what 
they are doing this is probably a pretty easy question.

I have been able to set the font size on the chat input box when it is created 
in llbottomtray.cpp.  I would like to do the same thing when someone clicks in 
that box to input text; it is possible they have changed the font setting and I 
would like to apply the size there as well, but I am stuck on how to do this.  
I think the right place to do this is in 
llchatbar.cpp/LLChatBar::onInputEditorGainFocus().

I have tried all kinds of wrong ways but at this point am stymied.

Exact steps on how to proceed would be appreciated.


This addresses bug storm-1025.
http://jira.secondlife.com/browse/storm-1025


Diffs
-

  indra/newview/llbottomtray.h 3d2e71443c58 
  indra/newview/llbottomtray.cpp 3d2e71443c58 
  indra/newview/llchatbar.h 3d2e71443c58 
  indra/newview/llchatbar.cpp 3d2e71443c58 

Diff: http://codereview.secondlife.com/r/139/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-1025 Chat preferences > font size should increase size of input text as well

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/139/
---

(Updated March 12, 2011, 4:42 p.m.)


Review request for Viewer.


Changes
---

Ignore this and the previous message--just changing the jira number from vwr to 
storm.  And yes, the diffs are messed up and I don't know how to fix that.


Summary (updated)
---

This is a request for help.  I am trying to learn more about c++ and how 
variables in one class are accessed from another.  For someone who knows what 
they are doing this is probably a pretty easy question.

I have been able to set the font size on the chat input box when it is created 
in llbottomtray.cpp.  I would like to do the same thing when someone clicks in 
that box to input text; it is possible they have changed the font setting and I 
would like to apply the size there as well, but I am stuck on how to do this.  
I think the right place to do this is in 
llchatbar.cpp/LLChatBar::onInputEditorGainFocus().

I have tried all kinds of wrong ways but at this point am stymied.

Exact steps on how to proceed would be appreciated.


This addresses bug storm-1025.
http://jira.secondlife.com/browse/storm-1025


Diffs
-

  indra/newview/llbottomtray.h 3d2e71443c58 
  indra/newview/llbottomtray.cpp 3d2e71443c58 
  indra/newview/llchatbar.h 3d2e71443c58 
  indra/newview/llchatbar.cpp 3d2e71443c58 

Diff: http://codereview.secondlife.com/r/139/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: Add optional range ring to the mini-map -- one centered on you with a radius of 20m to show local chat range

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/197/#review480
---


The code that creates a ring @ 100 meters has been removed and I changed the 
mystery constant 20.0 to CHAT_NORMAL_RADIUS.  Struggling with the diff hence 
that part is not updated.

- Jonathan


On March 12, 2011, 8 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/197/
> ---
> 
> (Updated March 12, 2011, 8 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Add optional range ring to the mini-map -- one centered on you with a radius 
> of 20m to show local chat range.
> 
> By default the range ring is off.
> 
> To turn it on you right click on the mini-map and pick the menu entry "Range 
> Ring".
> 
> 
> This addresses bug Storm-1068.
> http://jira.secondlife.com/browse/Storm-1068
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt aed94e854443 
>   indra/newview/app_settings/settings.xml aed94e854443 
>   indra/newview/llnetmap.h aed94e854443 
>   indra/newview/llnetmap.cpp aed94e854443 
>   indra/newview/skins/default/colors.xml aed94e854443 
>   indra/newview/skins/default/xui/en/menu_mini_map.xml aed94e854443 
> 
> Diff: http://codereview.secondlife.com/r/197/diff
> 
> 
> Testing
> ---
> 
> Tested with another avatar.  When they are outside the ring they cannot see 
> my local chat and when they are inside it they can, so the ring's radius is 
> set correctly.
> 
> Flew over various types of land to make sure my color choice (blue @10%) was 
> always visible.
> 
> Enabled range ring, logged out and back on; range ring is still present on 
> mini-map.
> 
> Panned mini-map, range ring remains centered over avatar.
> 
> Used mouse wheel to zoom in and out as far as possible.  Size of range ring 
> varied with change in zoom level.
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

[opensource-dev] Review Request: Storm-1077 Change "Voice Enabled/Disabled" to "Speak Button"

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/229/
---

Review request for Viewer.


Summary
---

1) Change Voice Enabled to Speak Button when you right click on the bottom bar 
and get a menu.

2) Eliminate the separator bar in this menu.

3) Have a hint appear for the Speak button (see image in jira) the first time 
you use the viewer, have voice on (the default), and successfully connect to a 
voice server.


This addresses bug STORM-1077.
http://jira.secondlife.com/browse/STORM-1077


Diffs
-

  doc/contributions.txt b761ed94eb26 
  indra/newview/app_settings/ignorable_dialogs.xml b761ed94eb26 
  indra/newview/llbottomtray.cpp b761ed94eb26 
  indra/newview/llfirstuse.h b761ed94eb26 
  indra/newview/llfirstuse.cpp b761ed94eb26 
  indra/newview/skins/default/xui/en/menu_bottomtray.xml b761ed94eb26 
  indra/newview/skins/default/xui/en/notifications.xml b761ed94eb26 

Diff: http://codereview.secondlife.com/r/229/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: Storm-1077 Change "Voice Enabled/Disabled" to "Speak Button"

[Jonathan Yap]

> On March 24, 2011, 6:04 a.m., Vadim ProductEngine wrote:
> > indra/newview/skins/default/xui/en/notifications.xml, line 6616
> > <http://codereview.secondlife.com/r/229/diff/1/?file=1319#file1319line6616>
> >
> > I guess you meant that *voice* is enabled by default, not microphone.
> > 
> > Mic only gets enabled explicitly by pressing the Speak button or middle 
> > mouse button.

My message about the mic being on by default is correct.  I asked Q about this 
and he said it was a design decision.  Every new user has their mic on when 
they first start the viewer.  Then they wonder why no one writes back when they 
are using text chat; they have been muted.

See the xml code block that shows the mic is on by default in one of my jira 
comments for this issue.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/229/#review496
-------


On March 23, 2011, 6:06 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/229/
> ---
> 
> (Updated March 23, 2011, 6:06 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> 1) Change Voice Enabled to Speak Button when you right click on the bottom 
> bar and get a menu.
> 
> 2) Eliminate the separator bar in this menu.
> 
> 3) Have a hint appear for the Speak button (see image in jira) the first time 
> you use the viewer, have voice on (the default), and successfully connect to 
> a voice server.
> 
> 
> This addresses bug STORM-1077.
> http://jira.secondlife.com/browse/STORM-1077
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt b761ed94eb26 
>   indra/newview/app_settings/ignorable_dialogs.xml b761ed94eb26 
>   indra/newview/llbottomtray.cpp b761ed94eb26 
>   indra/newview/llfirstuse.h b761ed94eb26 
>   indra/newview/llfirstuse.cpp b761ed94eb26 
>   indra/newview/skins/default/xui/en/menu_bottomtray.xml b761ed94eb26 
>   indra/newview/skins/default/xui/en/notifications.xml b761ed94eb26 
> 
> Diff: http://codereview.secondlife.com/r/229/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: Storm-1077 Change "Voice Enabled/Disabled" to "Speak Button"

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/229/
---

(Updated March 24, 2011, 9:10 a.m.)


Review request for Viewer.


Changes
---

Updated hint wording -- the speak button is NOT on by default.

Disable hint if speak or flyout button is pressed.


Summary
---

1) Change Voice Enabled to Speak Button when you right click on the bottom bar 
and get a menu.

2) Eliminate the separator bar in this menu.

3) Have a hint appear for the Speak button (see image in jira) the first time 
you use the viewer, have voice on (the default), and successfully connect to a 
voice server.


This addresses bug STORM-1077.
http://jira.secondlife.com/browse/STORM-1077


Diffs (updated)
-

  indra/newview/llcallfloater.cpp b761ed94eb26 
  indra/newview/llspeakbutton.cpp b761ed94eb26 
  indra/newview/skins/default/xui/en/notifications.xml b761ed94eb26 

Diff: http://codereview.secondlife.com/r/229/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: Storm-1077 Change "Voice Enabled/Disabled" to "Speak Button"

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/229/#review500
---


I deleted the line in the mouse-up speak button code that turns off the speak 
button hint.  That line was there when I incorrectly though the speak button 
might be on at viewer startup.  I could not get the diff to update so am 
commenting in here.

- Jonathan


On March 24, 2011, 9:10 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/229/
> ---
> 
> (Updated March 24, 2011, 9:10 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> 1) Change Voice Enabled to Speak Button when you right click on the bottom 
> bar and get a menu.
> 
> 2) Eliminate the separator bar in this menu.
> 
> 3) Have a hint appear for the Speak button (see image in jira) the first time 
> you use the viewer, have voice on (the default), and successfully connect to 
> a voice server.
> 
> 
> This addresses bug STORM-1077.
> http://jira.secondlife.com/browse/STORM-1077
> 
> 
> Diffs
> -
> 
>   indra/newview/llcallfloater.cpp b761ed94eb26 
>   indra/newview/llspeakbutton.cpp b761ed94eb26 
>   indra/newview/skins/default/xui/en/notifications.xml b761ed94eb26 
> 
> Diff: http://codereview.secondlife.com/r/229/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: Storm-1077 Change "Voice Enabled/Disabled" to "Speak Button"

[Jonathan Yap]

> On March 24, 2011, 11:04 a.m., Jonathan Yap wrote:
> > I deleted the line in the mouse-up speak button code that turns off the 
> > speak button hint.  That line was there when I incorrectly though the speak 
> > button might be on at viewer startup.  I could not get the diff to update 
> > so am commenting in here.

You can look at the bitbucket repo to see the change.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/229/#review500
---


On March 24, 2011, 9:10 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/229/
> ---
> 
> (Updated March 24, 2011, 9:10 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> 1) Change Voice Enabled to Speak Button when you right click on the bottom 
> bar and get a menu.
> 
> 2) Eliminate the separator bar in this menu.
> 
> 3) Have a hint appear for the Speak button (see image in jira) the first time 
> you use the viewer, have voice on (the default), and successfully connect to 
> a voice server.
> 
> 
> This addresses bug STORM-1077.
> http://jira.secondlife.com/browse/STORM-1077
> 
> 
> Diffs
> -
> 
>   indra/newview/llcallfloater.cpp b761ed94eb26 
>   indra/newview/llspeakbutton.cpp b761ed94eb26 
>   indra/newview/skins/default/xui/en/notifications.xml b761ed94eb26 
> 
> Diff: http://codereview.secondlife.com/r/229/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

[opensource-dev] Review Request: STORM-1095 Chat preferences > font size should increase size of input text in the chat box

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/244/
---

Review request for Viewer.


Summary
---

 Chat preferences > font size should increase size of input text in the chat box


This addresses bug STORM-1095.
http://jira.secondlife.com/browse/STORM-1095


Diffs
-

  doc/contributions.txt 65ff7415f171 
  indra/llui/lllineeditor.h 65ff7415f171 
  indra/llui/lllineeditor.cpp 65ff7415f171 
  indra/newview/llbottomtray.cpp 65ff7415f171 
  indra/newview/llfloaterpreference.cpp 65ff7415f171 
  indra/newview/llnearbychatbar.h 65ff7415f171 
  indra/newview/llnearbychatbar.cpp 65ff7415f171 
  indra/newview/llviewerchat.h 65ff7415f171 
  indra/newview/llviewerchat.cpp 65ff7415f171 

Diff: http://codereview.secondlife.com/r/244/diff


Testing
---

Change font size in preferences and see
1) Font size in chat input box changes to new size immediately
2) Font size is set to selected size when viewer is restarted


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-1095 Chat preferences > font size should increase size of input text in the chat box

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/244/
---

(Updated March 30, 2011, 5:43 a.m.)


Review request for Viewer.


Changes
---

Cleanup of where a few .h files were included.


Summary
---

 Chat preferences > font size should increase size of input text in the chat box


This addresses bug STORM-1095.
http://jira.secondlife.com/browse/STORM-1095


Diffs (updated)
-

  doc/contributions.txt 65ff7415f171 
  indra/llui/lllineeditor.h 65ff7415f171 
  indra/llui/lllineeditor.cpp 65ff7415f171 
  indra/newview/llbottomtray.cpp 65ff7415f171 
  indra/newview/llfloaterpreference.cpp 65ff7415f171 
  indra/newview/llnearbychatbar.h 65ff7415f171 
  indra/newview/llnearbychatbar.cpp 65ff7415f171 
  indra/newview/llviewerchat.h 65ff7415f171 
  indra/newview/llviewerchat.cpp 65ff7415f171 

Diff: http://codereview.secondlife.com/r/244/diff


Testing
---

Change font size in preferences and see
1) Font size in chat input box changes to new size immediately
2) Font size is set to selected size when viewer is restarted


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-1095 Chat preferences > font size should increase size of input text in the chat box

[Jonathan Yap]

> On March 30, 2011, 3:16 p.m., Boroondas Gupte wrote:
> > indra/llui/lllineeditor.cpp, lines 2294-2297
> > <http://codereview.secondlife.com/r/244/diff/2/?file=1361#file1361line2294>
> >
> > ... this simple setter isn't implemented right at the declaration (i.e. 
> > in indra/llui/lllineeditor.h)?

Unfortunately this identical change is also in Storm-1094 and I don't want to 
make the MM crazy.  As I do not have much internet at the moment if someone 
would like to adjust both of these jiras to match please do so, but I don't see 
that this is a showstopper if not addressed.


> On March 30, 2011, 3:16 p.m., Boroondas Gupte wrote:
> > indra/newview/llbottomtray.cpp, lines 554-555
> > <http://codereview.secondlife.com/r/244/diff/2/?file=1362#file1362line554>
> >
> > If font isn't used later, this could be written as
> > 
> > mNearbyChatBar->getChatBox()->setFont(LLViewerChat::getChatFont());
> > 
> > (Not sure which variant is more readable.)

I was doing a bit of code stealing of my changes in other files, hence the two 
lines made more sense in this instance.


> On March 30, 2011, 3:16 p.m., Boroondas Gupte wrote:
> > indra/newview/llviewerchat.cpp, lines 38-39
> > <http://codereview.secondlife.com/r/244/diff/2/?file=1367#file1367line38>
> >
> > Whatever the reason for that comment is, I guess the idea is that you 
> > put new members below it, not above.
> > 
> > Also, should this be initialized to null here ...

This was fixed as well.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/244/#review523
---


On March 30, 2011, 5:43 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/244/
> ---
> 
> (Updated March 30, 2011, 5:43 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
>  Chat preferences > font size should increase size of input text in the chat 
> box
> 
> 
> This addresses bug STORM-1095.
> http://jira.secondlife.com/browse/STORM-1095
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 65ff7415f171 
>   indra/llui/lllineeditor.h 65ff7415f171 
>   indra/llui/lllineeditor.cpp 65ff7415f171 
>   indra/newview/llbottomtray.cpp 65ff7415f171 
>   indra/newview/llfloaterpreference.cpp 65ff7415f171 
>   indra/newview/llnearbychatbar.h 65ff7415f171 
>   indra/newview/llnearbychatbar.cpp 65ff7415f171 
>   indra/newview/llviewerchat.h 65ff7415f171 
>   indra/newview/llviewerchat.cpp 65ff7415f171 
> 
> Diff: http://codereview.secondlife.com/r/244/diff
> 
> 
> Testing
> ---
> 
> Change font size in preferences and see
> 1) Font size in chat input box changes to new size immediately
> 2) Font size is set to selected size when viewer is restarted
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-1095 Chat preferences > font size should increase size of input text in the chat box

[Jonathan Yap]

> On April 3, 2011, 8:46 p.m., Merov Linden wrote:
> > indra/newview/llviewerchat.cpp, lines 38-39
> > <http://codereview.secondlife.com/r/244/diff/2/?file=1367#file1367line38>
> >
> > Should be initialized to NULL.

I fixed that initialization to NULL issue and that change was in the last PO 
test build.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/244/#review539
---


On March 30, 2011, 5:43 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/244/
> ---
> 
> (Updated March 30, 2011, 5:43 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
>  Chat preferences > font size should increase size of input text in the chat 
> box
> 
> 
> This addresses bug STORM-1095.
> http://jira.secondlife.com/browse/STORM-1095
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 65ff7415f171 
>   indra/llui/lllineeditor.h 65ff7415f171 
>   indra/llui/lllineeditor.cpp 65ff7415f171 
>   indra/newview/llbottomtray.cpp 65ff7415f171 
>   indra/newview/llfloaterpreference.cpp 65ff7415f171 
>   indra/newview/llnearbychatbar.h 65ff7415f171 
>   indra/newview/llnearbychatbar.cpp 65ff7415f171 
>   indra/newview/llviewerchat.h 65ff7415f171 
>   indra/newview/llviewerchat.cpp 65ff7415f171 
> 
> Diff: http://codereview.secondlife.com/r/244/diff
> 
> 
> Testing
> ---
> 
> Change font size in preferences and see
> 1) Font size in chat input box changes to new size immediately
> 2) Font size is set to selected size when viewer is restarted
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-1095 Chat preferences > font size should increase size of input text in the chat box

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/244/
---

(Updated April 13, 2011, 6:40 a.m.)


Review request for Viewer.


Summary
---

 Chat preferences > font size should increase size of input text in the chat box


This addresses bug STORM-1095.
http://jira.secondlife.com/browse/STORM-1095


Diffs (updated)
-

  doc/contributions.txt a8f868007986 
  indra/llui/lllineeditor.h a8f868007986 
  indra/llui/lllineeditor.cpp a8f868007986 
  indra/newview/llbottomtray.cpp a8f868007986 
  indra/newview/llfloaterpreference.cpp a8f868007986 
  indra/newview/llnearbychatbar.h a8f868007986 
  indra/newview/llnearbychatbar.cpp a8f868007986 
  indra/newview/llviewerchat.h a8f868007986 
  indra/newview/llviewerchat.cpp a8f868007986 

Diff: http://codereview.secondlife.com/r/244/diff


Testing
---

Change font size in preferences and see
1) Font size in chat input box changes to new size immediately
2) Font size is set to selected size when viewer is restarted


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-1095 Chat preferences > font size should increase size of input text in the chat box

[Jonathan Yap]

> On April 13, 2011, 7:19 a.m., Boroondas Gupte wrote:
> > indra/newview/llviewerchat.h, lines 37-39
> > <http://codereview.secondlife.com/r/244/diff/2-3/?file=1366#file1366line37>
> >
> > Why repeat the 'public:' ?

Vadim's RB comments and other LL code implied this was the way to go.  I've 
just eliminated the extra public: line as it's only for one item.


> On April 13, 2011, 7:19 a.m., Boroondas Gupte wrote:
> > indra/newview/llviewerchat.h, line 38
> > <http://codereview.secondlife.com/r/244/diff/2-3/?file=1366#file1366line38>
> >
> > Indented with 4 spaces instead of 1 tab like the other lines.

Fixed.


> On April 13, 2011, 7:19 a.m., Boroondas Gupte wrote:
> > doc/contributions.txt, line 646
> > <http://codereview.secondlife.com/r/244/diff/3/?file=1447#file1447line646>
> >
> > I guess the STORM-1077 and STORM-1019 entries were added in error? Or 
> > why are you replacing them, rather than just adding STORM-1095?

I am not the best Merge Monkey and must have made an error when merging these 
changesets into viewer-development when the MM sent this back.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/244/#review589
---


On April 13, 2011, 6:40 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/244/
> ---
> 
> (Updated April 13, 2011, 6:40 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
>  Chat preferences > font size should increase size of input text in the chat 
> box
> 
> 
> This addresses bug STORM-1095.
> http://jira.secondlife.com/browse/STORM-1095
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a8f868007986 
>   indra/llui/lllineeditor.h a8f868007986 
>   indra/llui/lllineeditor.cpp a8f868007986 
>   indra/newview/llbottomtray.cpp a8f868007986 
>   indra/newview/llfloaterpreference.cpp a8f868007986 
>   indra/newview/llnearbychatbar.h a8f868007986 
>   indra/newview/llnearbychatbar.cpp a8f868007986 
>   indra/newview/llviewerchat.h a8f868007986 
>   indra/newview/llviewerchat.cpp a8f868007986 
> 
> Diff: http://codereview.secondlife.com/r/244/diff
> 
> 
> Testing
> ---
> 
> Change font size in preferences and see
> 1) Font size in chat input box changes to new size immediately
> 2) Font size is set to selected size when viewer is restarted
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

[opensource-dev] Review Request: Storm-1128 Sort the results of using search in the World Map

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/262/
---

Review request for Viewer.


Summary
---

The results of using the World Map search option are sorted.


This addresses bug Storm-1128.
http://jira.secondlife.com/browse/Storm-1128


Diffs
-

  doc/contributions.txt a8f868007986 
  indra/newview/llfloaterworldmap.cpp a8f868007986 

Diff: http://codereview.secondlife.com/r/262/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-1095 Chat preferences > font size should increase size of input text in the chat box

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/244/
---

(Updated April 13, 2011, 12:32 p.m.)


Review request for Viewer.


Changes
---

Apply some formatting changes suggested in the RB comments.


Summary
---

 Chat preferences > font size should increase size of input text in the chat box


This addresses bug STORM-1095.
http://jira.secondlife.com/browse/STORM-1095


Diffs (updated)
-

  doc/contributions.txt a8f868007986 
  indra/llui/lllineeditor.h a8f868007986 
  indra/llui/lllineeditor.cpp a8f868007986 
  indra/newview/llbottomtray.cpp a8f868007986 
  indra/newview/llfloaterpreference.cpp a8f868007986 
  indra/newview/llnearbychatbar.h a8f868007986 
  indra/newview/llnearbychatbar.cpp a8f868007986 
  indra/newview/llviewerchat.h a8f868007986 
  indra/newview/llviewerchat.cpp a8f868007986 

Diff: http://codereview.secondlife.com/r/244/diff


Testing
---

Change font size in preferences and see
1) Font size in chat input box changes to new size immediately
2) Font size is set to selected size when viewer is restarted


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-1095 Chat preferences > font size should increase size of input text in the chat box

[Jonathan Yap]

> On April 13, 2011, 10:24 a.m., Vadim ProductEngine wrote:
> > indra/newview/llbottomtray.cpp, lines 560-561
> > <http://codereview.secondlife.com/r/244/diff/3/?file=1450#file1450line560>
> >
> > Any reason not to move this to LLNearbyChatBar::postBuild() ?

Yes, there is a reason, though not necessarily a good one -- the same change 
was made for Storm-1094 which would probably have the merge monkey send it back 
for me to re-merge.  Since I cannot compile with autobuild I  would not be able 
to test this change and thus it would languish for a month+ until I have time 
to work on getting my autobuild issue resolved.


> On April 13, 2011, 10:24 a.m., Vadim ProductEngine wrote:
> > indra/newview/llnearbychatbar.h, line 39
> > <http://codereview.secondlife.com/r/244/diff/3/?file=1452#file1452line39>
> >
> > again the redundant change

I don't know why hg diff is picking up this blank line.


> On April 13, 2011, 10:24 a.m., Vadim ProductEngine wrote:
> > indra/newview/llnearbychatbar.cpp, line 444
> > <http://codereview.secondlife.com/r/244/diff/3/?file=1453#file1453line444>
> >
> > RB seems to be set up to ignore whitespace changes and thus doesn't 
> > show this... but you've removed the leading tab here.
> > 
> > Please avoid irrelevant changes.

Fixed.  Thanks for catching this.


> On April 13, 2011, 10:24 a.m., Vadim ProductEngine wrote:
> > indra/newview/llviewerchat.h, line 39
> > <http://codereview.secondlife.com/r/244/diff/3/?file=1454#file1454line39>
> >
> > Replace the redundant "public:" marker with an empty line.

Also fixed, though in other LL code, where there are larger blocks to organize, 
multiple public: lines are found. 


> On April 13, 2011, 10:24 a.m., Vadim ProductEngine wrote:
> > indra/newview/llviewerchat.h, line 47
> > <http://codereview.secondlife.com/r/244/diff/3/?file=1454#file1454line47>
> >
> > CS: remove spaces near parenthesis.

Done.


> On April 13, 2011, 10:24 a.m., Vadim ProductEngine wrote:
> > indra/newview/llviewerchat.cpp, line 261
> > <http://codereview.secondlife.com/r/244/diff/3/?file=1455#file1455line261>
> >
> > CS: extra spaces near parenthesis

Done.


- Jonathan


-------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/244/#review591
---


On April 13, 2011, 12:32 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/244/
> ---
> 
> (Updated April 13, 2011, 12:32 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
>  Chat preferences > font size should increase size of input text in the chat 
> box
> 
> 
> This addresses bug STORM-1095.
> http://jira.secondlife.com/browse/STORM-1095
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a8f868007986 
>   indra/llui/lllineeditor.h a8f868007986 
>   indra/llui/lllineeditor.cpp a8f868007986 
>   indra/newview/llbottomtray.cpp a8f868007986 
>   indra/newview/llfloaterpreference.cpp a8f868007986 
>   indra/newview/llnearbychatbar.h a8f868007986 
>   indra/newview/llnearbychatbar.cpp a8f868007986 
>   indra/newview/llviewerchat.h a8f868007986 
>   indra/newview/llviewerchat.cpp a8f868007986 
> 
> Diff: http://codereview.secondlife.com/r/244/diff
> 
> 
> Testing
> ---
> 
> Change font size in preferences and see
> 1) Font size in chat input box changes to new size immediately
> 2) Font size is set to selected size when viewer is restarted
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-1095 Chat preferences > font size should increase size of input text in the chat box

[Jonathan Yap]

> On April 13, 2011, 1:13 p.m., Vadim ProductEngine wrote:
> > doc/contributions.txt, line 416
> > <http://codereview.secondlife.com/r/244/diff/4/?file=1463#file1463line416>
> >
> > fix this before committing

Fixed


> On April 13, 2011, 1:13 p.m., Vadim ProductEngine wrote:
> > doc/contributions.txt, line 646
> > <http://codereview.secondlife.com/r/244/diff/4/?file=1463#file1463line646>
> >
> > ditto

Fixed


> On April 13, 2011, 1:13 p.m., Vadim ProductEngine wrote:
> > indra/newview/llviewerchat.cpp, line 38
> > <http://codereview.secondlife.com/r/244/diff/4/?file=1471#file1471line38>
> >
> > ditto

Fixed.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/244/#review596
-------


On April 13, 2011, 12:32 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/244/
> ---
> 
> (Updated April 13, 2011, 12:32 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
>  Chat preferences > font size should increase size of input text in the chat 
> box
> 
> 
> This addresses bug STORM-1095.
> http://jira.secondlife.com/browse/STORM-1095
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a8f868007986 
>   indra/llui/lllineeditor.h a8f868007986 
>   indra/llui/lllineeditor.cpp a8f868007986 
>   indra/newview/llbottomtray.cpp a8f868007986 
>   indra/newview/llfloaterpreference.cpp a8f868007986 
>   indra/newview/llnearbychatbar.h a8f868007986 
>   indra/newview/llnearbychatbar.cpp a8f868007986 
>   indra/newview/llviewerchat.h a8f868007986 
>   indra/newview/llviewerchat.cpp a8f868007986 
> 
> Diff: http://codereview.secondlife.com/r/244/diff
> 
> 
> Testing
> ---
> 
> Change font size in preferences and see
> 1) Font size in chat input box changes to new size immediately
> 2) Font size is set to selected size when viewer is restarted
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-1095 Chat preferences > font size should increase size of input text in the chat box

[Jonathan Yap]

> On April 13, 2011, 10:24 a.m., Vadim ProductEngine wrote:
> > indra/newview/llbottomtray.cpp, lines 560-561
> > <http://codereview.secondlife.com/r/244/diff/3/?file=1450#file1450line560>
> >
> > Any reason not to move this to LLNearbyChatBar::postBuild() ?
> 
> Jonathan Yap wrote:
> Yes, there is a reason, though not necessarily a good one -- the same 
> change was made for Storm-1094 which would probably have the merge monkey 
> send it back for me to re-merge.  Since I cannot compile with autobuild I  
> would not be able to test this change and thus it would languish for a month+ 
> until I have time to work on getting my autobuild issue resolved.
> 
> Vadim ProductEngine wrote:
> I could do the merge for you.

The bitbucket repo for this is current -- if you want to make this change and 
test it that would be wonderful.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/244/#review591
-------


On April 13, 2011, 12:32 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/244/
> ---
> 
> (Updated April 13, 2011, 12:32 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
>  Chat preferences > font size should increase size of input text in the chat 
> box
> 
> 
> This addresses bug STORM-1095.
> http://jira.secondlife.com/browse/STORM-1095
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a8f868007986 
>   indra/llui/lllineeditor.h a8f868007986 
>   indra/llui/lllineeditor.cpp a8f868007986 
>   indra/newview/llbottomtray.cpp a8f868007986 
>   indra/newview/llfloaterpreference.cpp a8f868007986 
>   indra/newview/llnearbychatbar.h a8f868007986 
>   indra/newview/llnearbychatbar.cpp a8f868007986 
>   indra/newview/llviewerchat.h a8f868007986 
>   indra/newview/llviewerchat.cpp a8f868007986 
> 
> Diff: http://codereview.secondlife.com/r/244/diff
> 
> 
> Testing
> ---
> 
> Change font size in preferences and see
> 1) Font size in chat input box changes to new size immediately
> 2) Font size is set to selected size when viewer is restarted
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: Storm-1128 Sort the results of using search in the World Map

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/262/
---

(Updated April 15, 2011, 11:33 a.m.)


Review request for Viewer.


Changes
---

Cleaned up code per some codereview comments.


Summary
---

The results of using the World Map search option are sorted.


This addresses bug Storm-1128.
http://jira.secondlife.com/browse/Storm-1128


Diffs (updated)
-

  doc/contributions.txt a8f868007986 
  indra/newview/llfloaterworldmap.cpp a8f868007986 

Diff: http://codereview.secondlife.com/r/262/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: Storm-1128 Sort the results of using search in the World Map

[Jonathan Yap]

> On April 13, 2011, 1:26 p.m., Vadim ProductEngine wrote:
> > indra/newview/llfloaterworldmap.cpp, line 1497
> > <http://codereview.secondlife.com/r/262/diff/1/?file=1462#file1462line1497>
> >
> > CS: sim_info_vec

Changed.


> On April 13, 2011, 1:26 p.m., Vadim ProductEngine wrote:
> > indra/newview/llfloaterworldmap.cpp, lines 1500-1501
> > <http://codereview.secondlife.com/r/262/diff/1/?file=1462#file1462line1500>
> >
> > I'd move the iterator declaration inside the "for" clause to limit its 
> > scope.

Changed.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/262/#review598
-------


On April 15, 2011, 11:33 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/262/
> ---
> 
> (Updated April 15, 2011, 11:33 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> The results of using the World Map search option are sorted.
> 
> 
> This addresses bug Storm-1128.
> http://jira.secondlife.com/browse/Storm-1128
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a8f868007986 
>   indra/newview/llfloaterworldmap.cpp a8f868007986 
> 
> Diff: http://codereview.secondlife.com/r/262/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: Storm-1128 Sort the results of using search in the World Map

[Jonathan Yap]

> On April 13, 2011, 2:31 p.m., Boroondas Gupte wrote:
> > indra/newview/llfloaterworldmap.cpp, lines 99-101
> > <http://codereview.secondlife.com/r/262/diff/1/?file=1462#file1462line99>
> >
> > Why do you make this a functor rather than a plain normal function? 
> > It's not like we have to keep any internal state or something.

Moved to inside of routine.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/262/#review595
---


On April 15, 2011, 11:33 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/262/
> ---
> 
> (Updated April 15, 2011, 11:33 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> The results of using the World Map search option are sorted.
> 
> 
> This addresses bug Storm-1128.
> http://jira.secondlife.com/browse/Storm-1128
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a8f868007986 
>   indra/newview/llfloaterworldmap.cpp a8f868007986 
> 
> Diff: http://codereview.secondlife.com/r/262/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: Storm-1128 Sort the results of using search in the World Map

[Jonathan Yap]

> On April 13, 2011, 2:31 p.m., Boroondas Gupte wrote:
> > indra/newview/llfloaterworldmap.cpp, lines 99-101
> > <http://codereview.secondlife.com/r/262/diff/1/?file=1462#file1462line99>
> >
> > Why do you make this a functor rather than a plain normal function? 
> > It's not like we have to keep any internal state or something.
> 
> Jonathan Yap wrote:
> Moved to inside of routine.
> 
> Boroondas Gupte wrote:
> That's a good idea, too, but not what I meant. I was wondering why you 
> are using a class (or struct) with an operator() instead of just a function. 
> std::sort can take either a functor (i.e. a callable object like you're using 
> here) or a function pointer.

Nothing else in this class would make use of this  function so it seemed to 
make sense to go the struct route (plus when I was trying to figure out how to 
code this initially the example I found elsewhere in the code was done this 
way).


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/262/#review595
-------


On April 15, 2011, 11:33 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/262/
> ---
> 
> (Updated April 15, 2011, 11:33 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> The results of using the World Map search option are sorted.
> 
> 
> This addresses bug Storm-1128.
> http://jira.secondlife.com/browse/Storm-1128
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a8f868007986 
>   indra/newview/llfloaterworldmap.cpp a8f868007986 
> 
> Diff: http://codereview.secondlife.com/r/262/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: Storm-1128 Sort the results of using search in the World Map

[Jonathan Yap]

> On April 13, 2011, 2:31 p.m., Boroondas Gupte wrote:
> > indra/newview/llfloaterworldmap.cpp, lines 99-101
> > <http://codereview.secondlife.com/r/262/diff/1/?file=1462#file1462line99>
> >
> > Why do you make this a functor rather than a plain normal function? 
> > It's not like we have to keep any internal state or something.
> 
> Jonathan Yap wrote:
> Moved to inside of routine.
> 
> Boroondas Gupte wrote:
> That's a good idea, too, but not what I meant. I was wondering why you 
> are using a class (or struct) with an operator() instead of just a function. 
> std::sort can take either a functor (i.e. a callable object like you're using 
> here) or a function pointer.
> 
> Jonathan Yap wrote:
> Nothing else in this class would make use of this  function so it seemed 
> to make sense to go the struct route (plus when I was trying to figure out 
> how to code this initially the example I found elsewhere in the code was done 
> this way).
> 
> Boroondas Gupte wrote:
> Ah, right. I forgot local function aren't allowed in C++. Otherwise one 
> could have done something like
>   inline bool sortRegionNames(std::pair & _left, 
> std::pair & _right)
>   {
>   
> return(LLStringUtil::compareInsensitive(_left.second->getName(),_right.second->getName())
>  < 0);
>   }
> 
>   std::vector > 
> sim_info_vec(LLWorldMap::getInstance()->getRegionMap().begin(), 
> LLWorldMap::getInstance()->getRegionMap().end());
>   std::sort(sim_info_vec.begin(), sim_info_vec.end(), &sortRegionNames);
> 
> So we probably have to either stick to your struct or make 
> sortRegionNames a method or free function.

After fixing the >> to > > issues are you able to compile if the struct I moved 
from the top of the code into this routine is moved back to the top again?  The 
other example I found had it that way, and now I am wondering if it was like 
that to allow linux to compile.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/262/#review595
---


On April 15, 2011, 11:33 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/262/
> ---
> 
> (Updated April 15, 2011, 11:33 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> The results of using the World Map search option are sorted.
> 
> 
> This addresses bug Storm-1128.
> http://jira.secondlife.com/browse/Storm-1128
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a8f868007986 
>   indra/newview/llfloaterworldmap.cpp a8f868007986 
> 
> Diff: http://codereview.secondlife.com/r/262/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: Storm-1128 Sort the results of using search in the World Map

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/262/
---

(Updated April 16, 2011, 6:24 a.m.)


Review request for Viewer.


Changes
---

To fix some issues with gcc made some formatting changes and moved the compare 
routine back to the top of the code.


Summary
---

The results of using the World Map search option are sorted.


This addresses bug Storm-1128.
http://jira.secondlife.com/browse/Storm-1128


Diffs (updated)
-

  doc/contributions.txt a8f868007986 
  indra/newview/llfloaterworldmap.cpp a8f868007986 

Diff: http://codereview.secondlife.com/r/262/diff


Testing
---


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: Storm-1128 Sort the results of using search in the World Map

[Jonathan Yap]

> On April 16, 2011, 12:37 p.m., Boroondas Gupte wrote:
> > indra/newview/llfloaterworldmap.cpp, line 92
> > <http://codereview.secondlife.com/r/262/diff/3/?file=1486#file1486line92>
> >
> > Consider replacing the type notation
> > const std::pair &
> > be the (equivalent!) notation
> > std::pair  const&
> > which, although less 'traditional', is IMO easier to understand. (See 
> > http://www.xs4all.nl/~carlo17/cpp/const.qualifier.html )

Changed.


> On April 16, 2011, 12:37 p.m., Boroondas Gupte wrote:
> > indra/newview/llfloaterworldmap.cpp, line 1500
> > <http://codereview.secondlife.com/r/262/diff/3/?file=1486#file1486line1500>
> >
> > If we use std::sort, we really should directly #include  in 
> > this file, even if it already gets included indirectly (via other 
> > includes). The only exception I'd make would be if  was already 
> > (directly) included by the corresponding header file, 
> > indra/newview/llfloaterworldmap.h, but that isn't the case here.

Good observation.  Changed.


> On April 16, 2011, 12:37 p.m., Boroondas Gupte wrote:
> > indra/newview/llfloaterworldmap.cpp, line 88
> > <http://codereview.secondlife.com/r/262/diff/3/?file=1486#file1486line88>
> >
> > remove trailing whitespace

Changed.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/262/#review629
---


On April 16, 2011, 6:24 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/262/
> ---
> 
> (Updated April 16, 2011, 6:24 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> The results of using the World Map search option are sorted.
> 
> 
> This addresses bug Storm-1128.
> http://jira.secondlife.com/browse/Storm-1128
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a8f868007986 
>   indra/newview/llfloaterworldmap.cpp a8f868007986 
> 
> Diff: http://codereview.secondlife.com/r/262/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: Storm-1128 Sort the results of using search in the World Map

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/262/
---

(Updated April 16, 2011, 1:52 p.m.)


Review request for Viewer.


Changes
---

New diff uploaded.  Made a few changes suggested in Boroondas' comments.


Summary
---

The results of using the World Map search option are sorted.


This addresses bug Storm-1128.
http://jira.secondlife.com/browse/Storm-1128


Diffs (updated)
-

  doc/contributions.txt a8f868007986 
  indra/newview/llfloaterworldmap.cpp a8f868007986 

Diff: http://codereview.secondlife.com/r/262/diff


Testing
---


Thanks,

Jonathan

___
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

[opensource-dev] Review Request: STORM-956 Ability to mute dialogs by muting object (or object owner)

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/275/
---

Review request for Viewer.


Summary
---

>From the jira Description:
Griefers are still able to spam you with dialogs, such as dialogs, web site 
popups, and avatar animation requests even after you mute the owner of the 
object. Muting does not stop these at all. This is an ideal path for griefers 
to attack and they have been doing so for as long as I've been on Second Life. 
I believe that muting the object or it's owner should stop dialogs of all types 
from appearing from the object.

Note:
The changes to message_template.msg are not mine; that file was pushed from the 
server source into viewer-development.  I have no control over what goes into 
that file or how it is formatted, etc.


This addresses bug Storm-956.
http://jira.secondlife.com/browse/Storm-956


Diffs
-

  doc/contributions.txt ec4ad7e3ecca 
  indra/newview/llviewermessage.cpp ec4ad7e3ecca 
  scripts/messages/message_template.msg ec4ad7e3ecca 

Diff: http://codereview.secondlife.com/r/275/diff


Testing
---

See lengthy Test Plan in the jira.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-859 : winres.h isn't present within VC++ Express includes. Must be manually copied from win7 sdk example code.

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/304/#review698
---



indra/newview/skins/default/xui/en/notifications.xml


I think you have uploaded the wrong diff for this code review.


- Jonathan


On May 15, 2011, 7:14 p.m., Wolfpup Lowenhar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/304/
> ---
> 
> (Updated May 15, 2011, 7:14 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This is for switching from using winres.h to windows.h so that the viewer can 
> be build no matter whether your using and Express or Professional version of 
> Visual Studio.
> 
> 
> This addresses bug STORM-859.
> http://jira.secondlife.com/browse/STORM-859
> 
> 
> Diffs
> -
> 
>   indra/newview/skins/default/xui/en/notifications.xml e9d656e4620e 
> 
> Diff: http://codereview.secondlife.com/r/304/diff
> 
> 
> Testing
> ---
> 
> I have built viewer-development locally with this modification using Visual 
> Studio 2010 Ultimate and have seen no ill affects as yet to the built viewer. 
> It logs in to SL just fine.
> I have done the following with the built viewer:
> 1. edited or created objects with no ill affects.
> 2. carried on conversations in Near By, Group and P2P with no ill affects.
> 3. Teleported to various sims and levels in those sims with no ill affect.
> 4. carried on Voice conversations with no ill affect.
> 5. moved things around in my inventory as well as rezed and taken objects 
> from and to my inventory with no ill affects.
> 
> 
> Thanks,
> 
> Wolfpup
> 
>

___
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

[opensource-dev] Review Request: STORM-49 As a Content Creator, I have to select a regular prim type and than choose sculpt from a drop-down menu in order to create a sculpted prim.

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/317/
---

Review request for Viewer.


Summary
---

As a Content Creator, I have to select a regular prim type and than choose 
sculpt from a drop-down menu in order to create a sculpted prim.

I have added a new Sculpt icon to the list of available object types that can 
be selected on the build menu.  You can now rez a sculpt the same way you do a 
cube.

Possible issue: I made up a new Pcode used only by the viewer.


This addresses bug STORM-49.
http://jira.secondlife.com/browse/STORM-49


Diffs
-

  doc/contributions.txt a36a329e77cc 
  indra/llmath/llvolume.h a36a329e77cc 
  indra/llprimitive/llprimitive.cpp a36a329e77cc 
  indra/newview/llfloatertools.cpp a36a329e77cc 
  indra/newview/lltoolplacer.cpp a36a329e77cc 
  indra/newview/llviewerobjectlist.h a36a329e77cc 
  indra/newview/llviewerobjectlist.cpp a36a329e77cc 
  indra/newview/skins/default/textures/build/Object_Sculpt.png a36a329e77cc 
  indra/newview/skins/default/textures/build/Object_Sculpt_Selected.png 
a36a329e77cc 
  indra/newview/skins/default/textures/textures.xml a36a329e77cc 
  indra/newview/skins/default/xui/en/floater_tools.xml a36a329e77cc 

Diff: http://codereview.secondlife.com/r/317/diff


Testing
---

Rezzed a sculpt both alone and with someone watching.

Rezzed sculpts as fast as I could click (poor mans load test).


Thanks,

Jonathan

___
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

[opensource-dev] Review Request: VWR-25896 [Regression] Bulk uploads do not adhere to default permissions

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/323/
---

Review request for Viewer.


Summary
---

Something in the big mesh branch merge (build 230088) appears to have broken 
bulk uploading.

* On inventory tab, click "+" menu -> Upload -> Set Default Permissions
* Set your default permissions to enable Next owner Modify, Copy, and 
Resell/Giveaway and click OK
* Click "+" menu -> Upload -> Bulk and select a group of multiple images
* After upload completes, check permissions of each uploaded item

Expected behavior is that all the bulk uploaded items would have 
Modify/Copy/Transfer permissions.
Actual results is that first item upload has Modify/Copy/Transfer permissions, 
all others have no perms at all.

I examined this area of code before the mesh merge and saw a small but 
significant difference.  It is not clear to me that this is the only change 
necessary; in an email Oz said a server-side change might be needed too.  


This addresses bug VWR-25896.
http://jira.secondlife.com/browse/VWR-25896


Diffs
-

  doc/contributions.txt e4ef43d63d55 
  indra/newview/llassetuploadresponders.cpp e4ef43d63d55 

Diff: http://codereview.secondlife.com/r/323/diff


Testing
---

Repeated bulk uploading of two textures, altering the settings in the bulk 
upload floater for each test; their uploaded properties followed the settings I 
had set in the floater.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: VWR-25896 [Regression] Bulk uploads do not adhere to default permissions

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/323/
---

(Updated June 5, 2011, 8:11 a.m.)


Review request for Viewer.


Summary
---

Something in the big mesh branch merge (build 230088) appears to have broken 
bulk uploading.

* On inventory tab, click "+" menu -> Upload -> Set Default Permissions
* Set your default permissions to enable Next owner Modify, Copy, and 
Resell/Giveaway and click OK
* Click "+" menu -> Upload -> Bulk and select a group of multiple images
* After upload completes, check permissions of each uploaded item

Expected behavior is that all the bulk uploaded items would have 
Modify/Copy/Transfer permissions.
Actual results is that first item upload has Modify/Copy/Transfer permissions, 
all others have no perms at all.

I examined this area of code before the mesh merge and saw a small but 
significant difference.  It is not clear to me that this is the only change 
necessary; in an email Oz said a server-side change might be needed too.  


This addresses bug VWR-25896.
http://jira.secondlife.com/browse/VWR-25896


Diffs (updated)
-

  doc/contributions.txt e4ef43d63d55 
  indra/newview/llassetuploadresponders.cpp e4ef43d63d55 

Diff: http://codereview.secondlife.com/r/323/diff


Testing
---

Repeated bulk uploading of two textures, altering the settings in the bulk 
upload floater for each test; their uploaded properties followed the settings I 
had set in the floater.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: VWR-25896 [Regression] Bulk uploads do not adhere to default permissions

[Jonathan Yap]

> On June 5, 2011, 8:03 a.m., Boroondas Gupte wrote:
> > indra/newview/llassetuploadresponders.cpp, lines 384-386
> > <http://codereview.secondlife.com/r/323/diff/1/?file=2919#file2919line384>
> >
> > While you're at it, re-complete this comment that was (probably 
> > accidentally) clipped in the same commit 
> > (https://bitbucket.org/lindenlab/viewer-development/changeset/8871cfd6eaf5#chg_indra/newview/llassetuploadresponders.cpp_oldline328
> >  )

Added back line in clipped comment.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/323/#review720
-------


On June 5, 2011, 8:11 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/323/
> ---
> 
> (Updated June 5, 2011, 8:11 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Something in the big mesh branch merge (build 230088) appears to have broken 
> bulk uploading.
> 
> * On inventory tab, click "+" menu -> Upload -> Set Default Permissions
> * Set your default permissions to enable Next owner Modify, Copy, and 
> Resell/Giveaway and click OK
> * Click "+" menu -> Upload -> Bulk and select a group of multiple images
> * After upload completes, check permissions of each uploaded item
> 
> Expected behavior is that all the bulk uploaded items would have 
> Modify/Copy/Transfer permissions.
> Actual results is that first item upload has Modify/Copy/Transfer 
> permissions, all others have no perms at all.
> 
> I examined this area of code before the mesh merge and saw a small but 
> significant difference.  It is not clear to me that this is the only change 
> necessary; in an email Oz said a server-side change might be needed too.  
> 
> 
> This addresses bug VWR-25896.
> http://jira.secondlife.com/browse/VWR-25896
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt e4ef43d63d55 
>   indra/newview/llassetuploadresponders.cpp e4ef43d63d55 
> 
> Diff: http://codereview.secondlife.com/r/323/diff
> 
> 
> Testing
> ---
> 
> Repeated bulk uploading of two textures, altering the settings in the bulk 
> upload floater for each test; their uploaded properties followed the settings 
> I had set in the floater.
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

[opensource-dev] Review Request: STORM-899 'No attachments worn' text on blank 'Attachments' accordion remains in English for all locales

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/326/
---

Review request for Viewer.


Summary
---

1. Launch viewer, change language to German for example.
2. Re-login, open COF for edit.
3. Open 'Attachments' accordion, delete all objects.
===>
Actual: 'No attachments worn' text appears in English.


This addresses bug STORM-899.
http://jira.secondlife.com/browse/STORM-899


Diffs
-

  doc/contributions.txt c0c940514b74 
  indra/newview/llcofwearables.cpp c0c940514b74 
  indra/newview/skins/default/xui/en/panel_cof_wearables.xml c0c940514b74 
  indra/newview/skins/default/xui/en/strings.xml c0c940514b74 

Diff: http://codereview.secondlife.com/r/326/diff


Testing
---

Opened this tab with the viewer in English and also in French.  I saw the same 
message, but that is because there is no translation for French, but it shows 
the call for a translation is working.

What I am not sure of is if I made fix in the right way.


Thanks,

Jonathan

___
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

[opensource-dev] Review Request: VWR-25923 Unnecessary capability request spam

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/333/
---

Review request for Viewer.


Summary
---

This is a patch by ArminWeatherHax.  I am creating the request to help speed 
this fix along in the system.



Ways to reproduce: log into a simulator.
Reproduces: always
Affects: any version supported, probably all 3rd party viewers but Kokua (and 
Imprudence, soon).

What happens:
In each idle cycle the voice client requests the "ParcelVoiceInfoRequest" 
capability, thats each time a HTTP GET.
See LLVivoxVoiceClient::stateMachine() after comment // Check for parcel 
boundary crossing

Expected:
On parcel/region change request the capability once. It's not the region that 
rezzes in, but the avatar, so do the request for the capability not earlier 
than the agents region signals capabilitiesReceived() true. After that you are 
sure if the region returns an empty url you can give up for that region.

Not sure about the impact on lag - requesting and returning an url is not much 
data transmitted, though its a pretty big number of people doing it over and 
over per second (no matter if they have voice on or off).




going once again through llviewerregion I see its fortunately not each time a 
HTTP GET, just once when the agent connects to the region. Though the patch 
still saves all the lookup if the cap is there while it can't be possibly.


This addresses bug VWR-25923.
http://jira.secondlife.com/browse/VWR-25923


Diffs
-

  doc/contributions.txt a36a329e77cc 
  indra/newview/llvoicevivox.h a36a329e77cc 
  indra/newview/llvoicevivox.cpp a36a329e77cc 

Diff: http://codereview.secondlife.com/r/333/diff


Testing
---


Thanks,

Jonathan

___
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

[opensource-dev] Review Request: STORM-787 Mute Gestures Button

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/336/
---

Review request for Viewer.


Summary
---

Added a checkbox control in Preferences->Sound and Media (see attached image in 
jira) to enable/disable sounds coming from gestures.

There is a long writeup in the jira for reasons to have this.

This control is linked to 1) the master volume control and 2) the sound effects 
control.  If either of these is disabled (speaker has a red line through it) 
the checkbox is disabled.


This addresses bug STORM-787.
http://jira.secondlife.com/browse/STORM-787


Diffs
-

  doc/contributions.txt 6a3e7e403bd1 
  indra/newview/app_settings/settings.xml 6a3e7e403bd1 
  indra/newview/llfloaterpreference.h 6a3e7e403bd1 
  indra/newview/llfloaterpreference.cpp 6a3e7e403bd1 
  indra/newview/llviewermessage.cpp 6a3e7e403bd1 
  indra/newview/skins/default/xui/en/panel_preferences_sound.xml 6a3e7e403bd1 

Diff: http://codereview.secondlife.com/r/336/diff


Testing
---

Tested per test plan in jira.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-899 'No attachments worn' text on blank 'Attachments' accordion remains in English for all locales

[Jonathan Yap]

> On June 15, 2011, 9:11 a.m., Vadim ProductEngine wrote:
> > indra/newview/skins/default/xui/en/strings.xml, line 2095
> > <http://codereview.secondlife.com/r/326/diff/1/?file=2929#file2929line2095>
> >
> > The section this string is in seems inappropriate.

I've moved this a bit farther down and created its own section, as per Vadim's 
jira comment other similar flatlist fixes may be necessary.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/326/#review751
-------


On June 7, 2011, 2:32 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/326/
> ---
> 
> (Updated June 7, 2011, 2:32 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> 1. Launch viewer, change language to German for example.
> 2. Re-login, open COF for edit.
> 3. Open 'Attachments' accordion, delete all objects.
> ===>
> Actual: 'No attachments worn' text appears in English.
> 
> 
> This addresses bug STORM-899.
> http://jira.secondlife.com/browse/STORM-899
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt c0c940514b74 
>   indra/newview/llcofwearables.cpp c0c940514b74 
>   indra/newview/skins/default/xui/en/panel_cof_wearables.xml c0c940514b74 
>   indra/newview/skins/default/xui/en/strings.xml c0c940514b74 
> 
> Diff: http://codereview.secondlife.com/r/326/diff
> 
> 
> Testing
> ---
> 
> Opened this tab with the viewer in English and also in French.  I saw the 
> same message, but that is because there is no translation for French, but it 
> shows the call for a translation is working.
> 
> What I am not sure of is if I made fix in the right way.
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-899 'No attachments worn' text on blank 'Attachments' accordion remains in English for all locales

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/326/
---

(Updated June 15, 2011, 10:04 a.m.)


Review request for Viewer.


Changes
---

Made changes per Vadim's comments.


Summary
---

1. Launch viewer, change language to German for example.
2. Re-login, open COF for edit.
3. Open 'Attachments' accordion, delete all objects.
===>
Actual: 'No attachments worn' text appears in English.


This addresses bug STORM-899.
http://jira.secondlife.com/browse/STORM-899


Diffs (updated)
-

  doc/contributions.txt c0c940514b74 
  indra/newview/llcofwearables.cpp c0c940514b74 
  indra/newview/skins/default/xui/en/panel_cof_wearables.xml c0c940514b74 
  indra/newview/skins/default/xui/en/strings.xml c0c940514b74 

Diff: http://codereview.secondlife.com/r/326/diff


Testing
---

Opened this tab with the viewer in English and also in French.  I saw the same 
message, but that is because there is no translation for French, but it shows 
the call for a translation is working.

What I am not sure of is if I made fix in the right way.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-899 'No attachments worn' text on blank 'Attachments' accordion remains in English for all locales

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/326/
---

(Updated June 15, 2011, 12:50 p.m.)


Review request for Viewer.


Changes
---

Changed tab to space per Boroondas' comment.


Summary
---

1. Launch viewer, change language to German for example.
2. Re-login, open COF for edit.
3. Open 'Attachments' accordion, delete all objects.
===>
Actual: 'No attachments worn' text appears in English.


This addresses bug STORM-899.
http://jira.secondlife.com/browse/STORM-899


Diffs (updated)
-

  doc/contributions.txt c0c940514b74 
  indra/newview/llcofwearables.cpp c0c940514b74 
  indra/newview/skins/default/xui/en/panel_cof_wearables.xml c0c940514b74 
  indra/newview/skins/default/xui/en/strings.xml c0c940514b74 

Diff: http://codereview.secondlife.com/r/326/diff


Testing
---

Opened this tab with the viewer in English and also in French.  I saw the same 
message, but that is because there is no translation for French, but it shows 
the call for a translation is working.

What I am not sure of is if I made fix in the right way.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-1320 Create a 3p-libndofdev-linux repo based on version 0.3 of Jan Ciger's linux libndofdev.

[Jonathan Yap]

> On June 16, 2011, 4:59 p.m., Boroondas Gupte wrote:
> > libndofdev/CMakeLists.txt, lines 27-30
> > 
> >
> > Might be worth mentioning the non-linux libndofdev (and where to find 
> > it) in the error message.

On windows I already get 4 lines such as: 
package pcre has no installation information configured for platform windows
which is plenty of information for a package I have no need of; let's not put 
too much unnecessary text on the screen.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/340/#review760
---


On June 16, 2011, 1:32 p.m., Log Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/340/
> ---
> 
> (Updated June 16, 2011, 1:32 p.m.)
> 
> 
> Review request for Viewer, Oz Linden, Boroondas Gupte, and Altair Memo.
> 
> 
> Summary
> ---
> 
> Checked in version 0.3 of Jan Ciger's libndofdev drop-in replacement for 
> linux.
> * Added cmake build configuration.
> * Added autobuild package configuration.
> * Created libndofdev.txt license file from ndofdev.c file header.
> * Added README to explain that this is only for use in the linux viewer.
> 
> BUGFIXES:
> * OPEN-21 STORM-312 This version of libndofdev supports kernel versions >= 
> 2.6.33.
> 
> When reviewing, please provide extra scrutiny to autobuild.xml and 
> CMakeLists.txt, since those are the files I actually edited.
> 
> 
> This addresses bugs OPEN-21, STORM-1320 and STORM-312.
> http://jira.secondlife.com/browse/OPEN-21
> http://jira.secondlife.com/browse/STORM-1320
> http://jira.secondlife.com/browse/STORM-312
> 
> 
> Diffs
> -
> 
>   autobuild.xml PRE-CREATION 
>   libndofdev/CHANGELOG PRE-CREATION 
>   libndofdev/CMakeLists.txt PRE-CREATION 
>   libndofdev/LICENSES/libndofdev.txt PRE-CREATION 
>   libndofdev/README PRE-CREATION 
>   libndofdev/include/ndofdev_external.h PRE-CREATION 
>   libndofdev/ndofdev.c PRE-CREATION 
> 
> Diff: http://codereview.secondlife.com/r/340/diff
> 
> 
> Testing
> ---
> 
> This built successfully on TeamCity and the packaged library worked correctly 
> when I extracted it into the packages directory of the viewer build tree ( 
> build-linux-i686/packages ).  My spacenavigator, which hasn't worked in six 
> months, started working with the new build.
> 
> 
> Thanks,
> 
> Log
> 
>

___
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

[opensource-dev] Review Request: STORM-1392 Add Nearby Voice to the Communicate menu

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/346/
---

Review request for Viewer.


Summary
---

Add Nearby Voice to the Communicate menu.  This is currently activated via the 
flyout button next to the Speak button.


This addresses bug STORM-1392.
http://jira.secondlife.com/browse/STORM-1392


Diffs
-

  doc/contributions.txt dc5af272d23f 
  indra/newview/llbottomtray.cpp dc5af272d23f 
  indra/newview/skins/default/xui/en/menu_viewer.xml dc5af272d23f 

Diff: http://codereview.secondlife.com/r/346/diff


Testing
---

Clicking menu entry toggles appearance/disappearance of Nearby Voice floater.

Noted that this menu entry is grayed out when voice is off or otherwise not 
available.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-1392 Add Nearby Voice to the Communicate menu

[Jonathan Yap]

> On June 18, 2011, 4:24 a.m., Boroondas Gupte wrote:
> > indra/newview/llbottomtray.cpp, lines 570-574
> > <http://codereview.secondlife.com/r/346/diff/1/?file=2998#file2998line570>
> >
> > Wouldn't it be better to set the initial disabledness for all three 
> > items here in the XUI-XML with 
> > https://wiki.secondlife.com/wiki/Skinning_HowTo/Common_XUI_attributes#enabled
> >  rather than in the CPP code?
> > 
> > (Assuming that subsequent setEnabled(voice_status) will override it in 
> > either case, so that it can still be toggled.)

Code refactoring this area is a good idea but is outside the narrow scope of 
this jira.  If some Linden thinks this should be done I will make the changes.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/346/#review771
-------


On June 18, 2011, 4:12 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/346/
> ---
> 
> (Updated June 18, 2011, 4:12 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Add Nearby Voice to the Communicate menu.  This is currently activated via 
> the flyout button next to the Speak button.
> 
> 
> This addresses bug STORM-1392.
> http://jira.secondlife.com/browse/STORM-1392
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt dc5af272d23f 
>   indra/newview/llbottomtray.cpp dc5af272d23f 
>   indra/newview/skins/default/xui/en/menu_viewer.xml dc5af272d23f 
> 
> Diff: http://codereview.secondlife.com/r/346/diff
> 
> 
> Testing
> ---
> 
> Clicking menu entry toggles appearance/disappearance of Nearby Voice floater.
> 
> Noted that this menu entry is grayed out when voice is off or otherwise not 
> available.
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: VWR-25923 Unnecessary capability request spam

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/333/
---

(Updated June 24, 2011, 4:45 a.m.)


Review request for Viewer.


Summary
---

This is a patch by ArminWeatherHax.  I am creating the request to help speed 
this fix along in the system.



Ways to reproduce: log into a simulator.
Reproduces: always
Affects: any version supported, probably all 3rd party viewers but Kokua (and 
Imprudence, soon).

What happens:
In each idle cycle the voice client requests the "ParcelVoiceInfoRequest" 
capability, thats each time a HTTP GET.
See LLVivoxVoiceClient::stateMachine() after comment // Check for parcel 
boundary crossing

Expected:
On parcel/region change request the capability once. It's not the region that 
rezzes in, but the avatar, so do the request for the capability not earlier 
than the agents region signals capabilitiesReceived() true. After that you are 
sure if the region returns an empty url you can give up for that region.

Not sure about the impact on lag - requesting and returning an url is not much 
data transmitted, though its a pretty big number of people doing it over and 
over per second (no matter if they have voice on or off).




going once again through llviewerregion I see its fortunately not each time a 
HTTP GET, just once when the agent connects to the region. Though the patch 
still saves all the lookup if the cap is there while it can't be possibly.


This addresses bug VWR-25923.
http://jira.secondlife.com/browse/VWR-25923


Diffs (updated)
-

  doc/contributions.txt 04e2a3ddca51 
  indra/newview/llviewerregion.h 04e2a3ddca51 
  indra/newview/llviewerregion.cpp 04e2a3ddca51 
  indra/newview/llvoicevivox.h 04e2a3ddca51 
  indra/newview/llvoicevivox.cpp 04e2a3ddca51 

Diff: http://codereview.secondlife.com/r/333/diff


Testing
---


Thanks,

Jonathan

___
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

[opensource-dev] Review Request: STORM-1459 "Wearing Tab" - Add ability to copy displayed inventory names to clipboard

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/370/
---

Review request for Viewer.


Summary
---

Add a feature on the "Wearing TAB" where users could copy to the clipboard 
everything you see in the "Wearing TAB". This would make the blogging 
communities life soo much easier instead of having to type out all that 
information.

The label on this button needs input from someone on the XD team.

I would like to know if my code for adding a CR to the end of every line but 
the last one could be done in a more elegant way.


This addresses bug STORM-1459.
http://jira.secondlife.com/browse/STORM-1459


Diffs
-

  doc/contributions.txt f9864a43ddf0 
  indra/newview/llpanelwearing.h f9864a43ddf0 
  indra/newview/llpanelwearing.cpp f9864a43ddf0 
  indra/newview/skins/default/xui/en/panel_outfits_wearing.xml f9864a43ddf0 

Diff: http://codereview.secondlife.com/r/370/diff


Testing
---

Clicked on Send to Clipboard button and was able to paste results into an 
editor.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-1459 "Wearing Tab" - Add ability to copy displayed inventory names to clipboard

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/370/
---

(Updated June 29, 2011, 12:14 p.m.)


Review request for Viewer.


Changes
---

Made changes requested by Oz and Vadim and with help from Alexandrea Fride.


Summary
---

Add a feature on the "Wearing TAB" where users could copy to the clipboard 
everything you see in the "Wearing TAB". This would make the blogging 
communities life soo much easier instead of having to type out all that 
information.

The label on this button needs input from someone on the XD team.

I would like to know if my code for adding a CR to the end of every line but 
the last one could be done in a more elegant way.


This addresses bug STORM-1459.
http://jira.secondlife.com/browse/STORM-1459


Diffs (updated)
-

  doc/contributions.txt f9864a43ddf0 
  indra/newview/llpanelwearing.h f9864a43ddf0 
  indra/newview/llpanelwearing.cpp f9864a43ddf0 
  indra/newview/skins/default/xui/en/menu_wearing_gear.xml f9864a43ddf0 

Diff: http://codereview.secondlife.com/r/370/diff


Testing
---

Clicked on Send to Clipboard button and was able to paste results into an 
editor.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-1459 "Wearing Tab" - Add ability to copy displayed inventory names to clipboard

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/370/
---

(Updated June 30, 2011, 1:27 a.m.)


Review request for Viewer.


Changes
---

Updated comment per Nicky's suggestion


Summary
---

Add a feature on the "Wearing TAB" where users could copy to the clipboard 
everything you see in the "Wearing TAB". This would make the blogging 
communities life soo much easier instead of having to type out all that 
information.

The label on this button needs input from someone on the XD team.

I would like to know if my code for adding a CR to the end of every line but 
the last one could be done in a more elegant way.


This addresses bug STORM-1459.
http://jira.secondlife.com/browse/STORM-1459


Diffs (updated)
-

  doc/contributions.txt f9864a43ddf0 
  indra/newview/llpanelwearing.h f9864a43ddf0 
  indra/newview/llpanelwearing.cpp f9864a43ddf0 
  indra/newview/skins/default/xui/en/menu_wearing_gear.xml f9864a43ddf0 

Diff: http://codereview.secondlife.com/r/370/diff


Testing
---

Clicked on Send to Clipboard button and was able to paste results into an 
editor.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: Change pre-login viewer display URL to prepare for new community information display

[Jonathan Yap]

> On July 16, 2011, 6:48 a.m., Thickbrick Sleaford wrote:
> > indra/newview/app_settings/settings.xml, line 4875
> > 
> >
> > Setting the default value to something other than an empty string would 
> > override the selected grid's login page in LLGridManager::getGridInfo and 
> > LLGridManager::getLoginPage, which is probably not the desired result. In 
> > fact, I wonder why this setting is persistent at all. This should probably 
> > be renamed to CmdLineLoginPage, to be consistent with CmdLineLoginURI etc.
> 
> Oz Linden wrote:
> That may be true (have not checked), but this same URL is the login page 
> for all our grids, it doesn't matter.
> 
> I confirmed that overriding on the command line does work.
>

Would this change affect anything when connecting to a non-LL grid?


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/399/#review883
---


On July 16, 2011, 6:12 a.m., Oz Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/399/
> ---
> 
> (Updated July 16, 2011, 6:12 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This changes the URL from which the upper part of the viewer window is loaded 
> prior to login.
> 
> At the moment, the new URL is internally redirected to the same content as 
> before, but shortly it will have new community and activity information on 
> what's going on in Second Life.
> 
> 
> This addresses bug storm-1510.
> http://jira.secondlife.com/browse/storm-1510
> 
> 
> Diffs
> -
> 
>   indra/newview/app_settings/settings.xml c6f2f4af65e5 
>   indra/newview/llviewernetwork.cpp c6f2f4af65e5 
>   indra/newview/tests/llviewernetwork_test.cpp c6f2f4af65e5 
> 
> Diff: http://codereview.secondlife.com/r/399/diff
> 
> 
> Testing
> ---
> 
> Confirmed using the log file that the new URL is being requested.
> Changed the settings file locally to point to an internal prototype of the 
> new screen and confirmed that it is displayed correctly.
> 
> 
> Thanks,
> 
> Oz
> 
>

___
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

Re: [opensource-dev] Review Request: (STORM-1221) [HARD CODED] ALL LANGS Several strings are untranslated under Group Profile Land/Assets (French viewer)

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/400/#review889
---



indra/newview/lldateutil.cpp


Fix spelling of with (h is missing)


- Jonathan


On July 18, 2011, 4:01 p.m., Seth ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/400/
> ---
> 
> (Updated July 18, 2011, 4:01 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Hard coded dates made localizable under Group Profile Land/Assets.
> 
> Added a function for parsing a date string of specific format.
> Added strings defining the date format in Group Profile Land/Assets that 
> should be localized.
> 
> 
> This addresses bug STORM-1221.
> http://jira.secondlife.com/browse/STORM-1221
> 
> 
> Diffs
> -
> 
>   indra/newview/lldateutil.h d8c37b383028 
>   indra/newview/lldateutil.cpp d8c37b383028 
>   indra/newview/llpanelgrouplandmoney.cpp d8c37b383028 
>   indra/newview/skins/default/xui/en/strings.xml d8c37b383028 
> 
> Diff: http://codereview.secondlife.com/r/400/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Seth
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-49 As a Content Creator, I have to select a regular prim type and than choose sculpt from a drop-down menu in order to create a sculpted prim.

[Jonathan Yap]

> On July 20, 2011, 11:14 a.m., Boroondas Gupte wrote:
> > indra/newview/lltoolplacer.cpp, lines 391-393
> > <http://codereview.secondlife.com/r/317/diff/1/?file=2852#file2852line391>
> >
> > Remove the empty line.

Done


> On July 20, 2011, 11:14 a.m., Boroondas Gupte wrote:
> > indra/newview/lltoolplacer.cpp, lines 394-404
> > <http://codereview.secondlife.com/r/317/diff/1/?file=2852#file2852line394>
> >
> > Where are these numbers coming from? Are they used elsewhere in the 
> > code?

These numbers are copied from another place in the code dealing with converting 
an object to the sculpt type.  In file \newview\llpanelobject.cpp see the end 
of getVolumeParams.


> On July 20, 2011, 11:14 a.m., Boroondas Gupte wrote:
> > indra/newview/llviewerobjectlist.h, lines 252-255
> > <http://codereview.secondlife.com/r/317/diff/1/?file=2853#file2853line252>
> >
> > Please re-introduce the empty line before
> > // Inlines

Done.


> On July 20, 2011, 11:14 a.m., Boroondas Gupte wrote:
> > indra/newview/llviewerobjectlist.cpp, line 96
> > <http://codereview.secondlife.com/r/317/diff/1/?file=2854#file2854line96>
> >
> > Is a global variable really the way to go here? Also, please add a 
> > short comment explaining the semantics of this variable.

Comment added.  Robin Cornelius suggested using a global variable.  If you can 
think of a better way please let me know.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/317/#review895
---


On June 2, 2011, 2:03 p.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/317/
> ---
> 
> (Updated June 2, 2011, 2:03 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> As a Content Creator, I have to select a regular prim type and than choose 
> sculpt from a drop-down menu in order to create a sculpted prim.
> 
> I have added a new Sculpt icon to the list of available object types that can 
> be selected on the build menu.  You can now rez a sculpt the same way you do 
> a cube.
> 
> Possible issue: I made up a new Pcode used only by the viewer.
> 
> 
> This addresses bug STORM-49.
> http://jira.secondlife.com/browse/STORM-49
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a36a329e77cc 
>   indra/llmath/llvolume.h a36a329e77cc 
>   indra/llprimitive/llprimitive.cpp a36a329e77cc 
>   indra/newview/llfloatertools.cpp a36a329e77cc 
>   indra/newview/lltoolplacer.cpp a36a329e77cc 
>   indra/newview/llviewerobjectlist.h a36a329e77cc 
>   indra/newview/llviewerobjectlist.cpp a36a329e77cc 
>   indra/newview/skins/default/textures/build/Object_Sculpt.png a36a329e77cc 
>   indra/newview/skins/default/textures/build/Object_Sculpt_Selected.png 
> a36a329e77cc 
>   indra/newview/skins/default/textures/textures.xml a36a329e77cc 
>   indra/newview/skins/default/xui/en/floater_tools.xml a36a329e77cc 
> 
> Diff: http://codereview.secondlife.com/r/317/diff
> 
> 
> Testing
> ---
> 
> Rezzed a sculpt both alone and with someone watching.
> 
> Rezzed sculpts as fast as I could click (poor mans load test).
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-49 As a Content Creator, I have to select a regular prim type and than choose sculpt from a drop-down menu in order to create a sculpted prim.

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/317/
---

(Updated Aug. 3, 2011, 11:21 a.m.)


Review request for Viewer.


Changes
---

Made changes per RB suggestions by Boroondas.


Summary
---

As a Content Creator, I have to select a regular prim type and than choose 
sculpt from a drop-down menu in order to create a sculpted prim.

I have added a new Sculpt icon to the list of available object types that can 
be selected on the build menu.  You can now rez a sculpt the same way you do a 
cube.

Possible issue: I made up a new Pcode used only by the viewer.


This addresses bug STORM-49.
http://jira.secondlife.com/browse/STORM-49


Diffs (updated)
-

  doc/contributions.txt a36a329e77cc 
  indra/llmath/llvolume.h a36a329e77cc 
  indra/llprimitive/llprimitive.cpp a36a329e77cc 
  indra/newview/llfloatertools.cpp a36a329e77cc 
  indra/newview/lltoolplacer.cpp a36a329e77cc 
  indra/newview/llviewerobjectlist.h a36a329e77cc 
  indra/newview/llviewerobjectlist.cpp a36a329e77cc 
  indra/newview/skins/default/textures/build/Object_Sculpt.png a36a329e77cc 
  indra/newview/skins/default/textures/build/Object_Sculpt_Selected.png 
a36a329e77cc 
  indra/newview/skins/default/textures/textures.xml a36a329e77cc 
  indra/newview/skins/default/xui/en/floater_tools.xml a36a329e77cc 

Diff: http://codereview.secondlife.com/r/317/diff


Testing
---

Rezzed a sculpt both alone and with someone watching.

Rezzed sculpts as fast as I could click (poor mans load test).


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-49 As a Content Creator, I have to select a regular prim type and than choose sculpt from a drop-down menu in order to create a sculpted prim.

[Jonathan Yap]

> On July 20, 2011, 11:14 a.m., Boroondas Gupte wrote:
> > indra/newview/llviewerobjectlist.cpp, line 96
> > <http://codereview.secondlife.com/r/317/diff/1/?file=2854#file2854line96>
> >
> > Is a global variable really the way to go here? Also, please add a 
> > short comment explaining the semantics of this variable.
> 
> Jonathan Yap wrote:
> Comment added.  Robin Cornelius suggested using a global variable.  If 
> you can think of a better way please let me know.
> 
> Boroondas Gupte wrote:
> A static class member variable would avoid littering the global 
> namespace. Though an issue I'm more worried about would remain: We might end 
> up converting the wrong object to a sculpty.
> 
> I think one could trigger a race condition by clicking the new sculpty 
> button, then very quickly clicking inworld, clicking one of the 
> non-scuplty prim buttons and clicking inworld again. In that case, it might 
> be undefined which of the two new objects become sculpties. Then again, users 
> who do insane clicking like that might not care either way, anyway.

I don't think rapid clicking as you suggest (even if the time was extremely 
close to the rezz time of the about to be created sculpt) is an issue -- the 
packet sent to the server to request the sculpt to be created and rezzed is 
going to generate a packet coming from the server with the initial object 
parameters.  I don't see how you could "get in front" of this return server 
packet with additional clicking.  I'm assuming the server processes packets 
coming from the viewer sequentially.


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/317/#review895
---


On Aug. 3, 2011, 11:21 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/317/
> ---
> 
> (Updated Aug. 3, 2011, 11:21 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> As a Content Creator, I have to select a regular prim type and than choose 
> sculpt from a drop-down menu in order to create a sculpted prim.
> 
> I have added a new Sculpt icon to the list of available object types that can 
> be selected on the build menu.  You can now rez a sculpt the same way you do 
> a cube.
> 
> Possible issue: I made up a new Pcode used only by the viewer.
> 
> 
> This addresses bug STORM-49.
> http://jira.secondlife.com/browse/STORM-49
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a36a329e77cc 
>   indra/llmath/llvolume.h a36a329e77cc 
>   indra/llprimitive/llprimitive.cpp a36a329e77cc 
>   indra/newview/llfloatertools.cpp a36a329e77cc 
>   indra/newview/lltoolplacer.cpp a36a329e77cc 
>   indra/newview/llviewerobjectlist.h a36a329e77cc 
>   indra/newview/llviewerobjectlist.cpp a36a329e77cc 
>   indra/newview/skins/default/textures/build/Object_Sculpt.png a36a329e77cc 
>   indra/newview/skins/default/textures/build/Object_Sculpt_Selected.png 
> a36a329e77cc 
>   indra/newview/skins/default/textures/textures.xml a36a329e77cc 
>   indra/newview/skins/default/xui/en/floater_tools.xml a36a329e77cc 
> 
> Diff: http://codereview.secondlife.com/r/317/diff
> 
> 
> Testing
> ---
> 
> Rezzed a sculpt both alone and with someone watching.
> 
> Rezzed sculpts as fast as I could click (poor mans load test).
> 
> 
> Thanks,
> 
> Jonathan
> 
>

___
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

Re: [opensource-dev] Review Request: STORM-49 As a Content Creator, I have to select a regular prim type and than choose sculpt from a drop-down menu in order to create a sculpted prim.

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/317/
---

(Updated Aug. 14, 2011, 9:07 a.m.)


Review request for Viewer.


Changes
---

Converted global variable ConvertToSculpt from a global to a static class 
member per code review suggestion and pushed changes to bitbucket.


Summary
---

As a Content Creator, I have to select a regular prim type and than choose 
sculpt from a drop-down menu in order to create a sculpted prim.

I have added a new Sculpt icon to the list of available object types that can 
be selected on the build menu.  You can now rez a sculpt the same way you do a 
cube.

Possible issue: I made up a new Pcode used only by the viewer.


This addresses bug STORM-49.
http://jira.secondlife.com/browse/STORM-49


Diffs (updated)
-

  doc/contributions.txt a36a329e77cc 
  indra/llmath/llvolume.h a36a329e77cc 
  indra/llprimitive/llprimitive.cpp a36a329e77cc 
  indra/newview/llfloatertools.cpp a36a329e77cc 
  indra/newview/lltoolplacer.cpp a36a329e77cc 
  indra/newview/llviewerobjectlist.h a36a329e77cc 
  indra/newview/llviewerobjectlist.cpp a36a329e77cc 
  indra/newview/skins/default/textures/build/Object_Sculpt.png a36a329e77cc 
  indra/newview/skins/default/textures/build/Object_Sculpt_Selected.png 
a36a329e77cc 
  indra/newview/skins/default/textures/textures.xml a36a329e77cc 
  indra/newview/skins/default/xui/en/floater_tools.xml a36a329e77cc 

Diff: http://codereview.secondlife.com/r/317/diff


Testing
---

Rezzed a sculpt both alone and with someone watching.

Rezzed sculpts as fast as I could click (poor mans load test).


Thanks,

Jonathan

___
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

[opensource-dev] Review Request: STORM-1567 Mute button for llDialog popup

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/449/
---

Review request for Viewer.


Summary
---

Add a block button to popups from llDialog calls.  Clicking on the Block button 
adds the object to the block list.  The type of block is Object.  Changing the 
object's name does not defeat this block as the block entry is stored as a UUID.

Note: The object's name in the block list does not change if the object is 
renamed, though the block does work.  This may need to be addressed in a 
separate SVC jira once this change is incorporated into viewer-development.


This addresses bug STORM-1567.
http://jira.secondlife.com/browse/STORM-1567


Diffs
-

  doc/contributions.txt df4801993ea4 
  indra/newview/lltoastnotifypanel.cpp df4801993ea4 
  indra/newview/llviewermessage.cpp df4801993ea4 
  indra/newview/skins/default/xui/en/notifications.xml df4801993ea4 

Diff: http://codereview.secondlife.com/r/449/diff


Testing
---

Tested per Test Plan jira entry.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-1567 Mute button for llDialog popup

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/449/
---

(Updated Aug. 23, 2011, 4:07 a.m.)


Review request for Viewer.


Changes
---

Original diff applied to current version of viewer-development per Vadim's 
request.  New diff based on this changed applied here.


Summary
---

Add a block button to popups from llDialog calls.  Clicking on the Block button 
adds the object to the block list.  The type of block is Object.  Changing the 
object's name does not defeat this block as the block entry is stored as a UUID.

Note: The object's name in the block list does not change if the object is 
renamed, though the block does work.  This may need to be addressed in a 
separate SVC jira once this change is incorporated into viewer-development.


This addresses bug STORM-1567.
http://jira.secondlife.com/browse/STORM-1567


Diffs (updated)
-

  doc/contributions.txt 4ebbd04efd93 
  indra/newview/lltoastnotifypanel.cpp 4ebbd04efd93 
  indra/newview/llviewermessage.cpp 4ebbd04efd93 
  indra/newview/skins/default/xui/en/notifications.xml 4ebbd04efd93 

Diff: http://codereview.secondlife.com/r/449/diff


Testing
---

Tested per Test Plan jira entry.


Thanks,

Jonathan

___
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

Re: [opensource-dev] Review Request: STORM-1567 Mute button for llDialog popup

[Jonathan Yap]

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/449/
---

(Updated Aug. 23, 2011, 1:41 p.m.)


Review request for Viewer.


Summary
---

Add a block button to popups from llDialog calls.  Clicking on the Block button 
adds the object to the block list.  The type of block is Object.  Changing the 
object's name does not defeat this block as the block entry is stored as a UUID.

Note: The object's name in the block list does not change if the object is 
renamed, though the block does work.  This may need to be addressed in a 
separate SVC jira once this change is incorporated into viewer-development.


This addresses bug STORM-1567.
http://jira.secondlife.com/browse/STORM-1567


Diffs
-

  doc/contributions.txt 4ebbd04efd93 
  indra/newview/lltoastnotifypanel.cpp 4ebbd04efd93 
  indra/newview/llviewermessage.cpp 4ebbd04efd93 
  indra/newview/skins/default/xui/en/notifications.xml 4ebbd04efd93 

Diff: http://codereview.secondlife.com/r/449/diff


Testing
---

Tested per Test Plan jira entry.


Thanks,

Jonathan

___
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