Re: [opensource-dev] Review Request: (STORM-721) Information about resident is displayed incorrectly in mini-inspector if there are any resident or group SLURLs

[Richard Nelson]

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



indra/llui/lltextbase.cpp


this will still display views that are partially outside of the visible rect

what you want to do iscall mDocumentView->draw() inside the block that has 
the clip operation and then hide the mDocumentView when calling LLUICtrl::draw.




- Richard


On March 1, 2011, 1:53 p.m., Seth ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/169/
> ---
> 
> (Updated March 1, 2011, 1:53 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Fixed text editor to display the embedded widgets only if they are in the 
> currently visible area of a text document.
> 
> 
> This addresses bug STORM-721.
> http://jira.secondlife.com/browse/STORM-721
> 
> 
> Diffs
> -
> 
>   indra/llui/lltextbase.cpp 767feb16f05f 
> 
> Diff: http://codereview.secondlife.com/r/169/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-721) Information about resident is displayed incorrectly in mini-inspector if there are any resident or group SLURLs

[Richard Nelson]

> On March 3, 2011, 11:45 a.m., Richard Nelson wrote:
> > indra/llui/lltextbase.cpp, line 604
> > <http://codereview.secondlife.com/r/169/diff/1/?file=1014#file1014line604>
> >
> > this will still display views that are partially outside of the visible 
> > rect
> > 
> > what you want to do iscall mDocumentView->draw() inside the block that 
> > has the clip operation and then hide the mDocumentView when calling 
> > LLUICtrl::draw.
> > 
> >
> 
> Seth ProductEngine wrote:
> The clip rect in the block with drawText() call is not enabled for the 
> editors without a scroller and even enabling it and calling 
> mDocumentView->draw() inside that block cuts off the bottom of the last 
> visible line of text (see screenshot at 
> https://jira.secondlife.com/secure/attachment/48565/clipped_text.png).
> 
> With allow_scroll="false" & clip_partial="true" settings for the editor 
> we lose one text line and if there are icons on the first line they will be 
> clipped anyway because the top of such icons exceeds the top limit of the 
> visible rect.

With allow_scroll="false" and clip_partial="true", you shouldn't lose a line of 
text, so that sounds like the bug to fix.  If icons on the first line are being 
clipped, you could increase the v_pad attribute to leave enough room, although 
the size of the icon should be accounted for in the text layout, so that is 
either another bug or related to the first bug above.

I'd strongly suggest fixing those issues instead of using your proposed fix.  
Visibility of the text and widgets embedded within the text should be handled 
in the same way if possible.

I'll let you know if I find any obvious source of these problems.


- Richard


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


On March 4, 2011, 10:30 a.m., Seth ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/169/
> ---
> 
> (Updated March 4, 2011, 10:30 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> Fixed text editor to display the embedded widgets only if they are in the 
> currently visible area of a text document.
> 
> 
> This addresses bug STORM-721.
> http://jira.secondlife.com/browse/STORM-721
> 
> 
> Diffs
> -
> 
>   indra/llui/lltextbase.cpp 767feb16f05f 
> 
> Diff: http://codereview.secondlife.com/r/169/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-721) Information about resident is displayed incorrectly in mini-inspector if there are any resident or group SLURLs

[Richard Nelson]

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


So I've found some underlying problems with text extents measurement and 
clipping and I'm working on a fix now...hopefully I'll have something tomorrow

- Richard


On March 4, 2011, 10:30 a.m., Seth ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/169/
> ---
> 
> (Updated March 4, 2011, 10:30 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> Fixed text editor to display the embedded widgets only if they are in the 
> currently visible area of a text document.
> 
> 
> This addresses bug STORM-721.
> http://jira.secondlife.com/browse/STORM-721
> 
> 
> Diffs
> -
> 
>   indra/llui/lltextbase.cpp 767feb16f05f 
> 
> Diff: http://codereview.secondlife.com/r/169/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-320) Script Editor in Viewer 2.0+ "tabs" incorrectly

[Richard Nelson]

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

Ship it!


seems reasonable

- Richard


On April 19, 2011, 3:16 p.m., Seth ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/274/
> ---
> 
> (Updated April 19, 2011, 3:16 p.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> Fixed navigation with arrow keys through the text with enabled word-wrapping.
> 
> Steps to repro at 
> https://jira.secondlife.com/browse/STORM-320?focusedCommentId=244113&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-244113
> 
> 
> This addresses bug STORM-320.
> http://jira.secondlife.com/browse/STORM-320
> 
> 
> Diffs
> -
> 
>   indra/llui/lltextbase.cpp 9c0506d10226 
> 
> Diff: http://codereview.secondlife.com/r/274/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-229) Loading Scripts takes a long time and stalls Viewer

[Richard Nelson]

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

Ship it!


makes sense. ship it.

- Richard


On April 18, 2011, 11:25 a.m., Seth ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/273/
> ---
> 
> (Updated April 18, 2011, 11:25 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> Partial fix for selected text indentation in script editor stalling the 
> viewer: disabled updating text segments until all indentation commands are 
> executed.
> Looks like the viewer is not stalled but the indentation still works rather 
> slow.
> 
> 
> Diffs
> -
> 
>   indra/llui/lltexteditor.cpp 9c0506d10226 
> 
> Diff: http://codereview.secondlife.com/r/273/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: VWR-25609 crash on shutdown in LLGLNamePool::sInstances destructor

[Richard Nelson]

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

Ship it!


looks good.

- Richard


On May 2, 2011, 4:30 p.m., Brad Kittenbrink wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/279/
> ---
> 
> (Updated May 2, 2011, 4:30 p.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> The error is due to the fact that the instances of subclasses of LLGLNamePool 
> (e.g. LLVertexBuffer::sStreamVBOPool) were having their destructors run after 
> the destructor for LLGLNamePool::sInstances, yet accessing the members of the 
> already destructed vector, causing a crash.
> 
> The fix is to use the LLInstanceTracker class which has already solved this 
> problem in a general way, rather than hand-rolling our own custom solution in 
> LLGLNamePool.
> 
> 
> This addresses bug VWR-25609.
> http://jira.secondlife.com/browse/VWR-25609
> 
> 
> Diffs
> -
> 
>   indra/llrender/llgl.h UNKNOWN 
>   indra/llrender/llgl.cpp UNKNOWN 
>   indra/llrender/llvertexbuffer.cpp UNKNOWN 
>   indra/newview/llspatialpartition.cpp UNKNOWN 
> 
> Diff: http://codereview.secondlife.com/r/279/diff
> 
> 
> Testing
> ---
> 
> Tested in debug builds with the Microsoft Debug heap and confirmed that this 
> patch allows shutdown to complete without error.
> 
> 
> Thanks,
> 
> Brad
> 
>

___
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-25862 Fix viewer caches not being cleared.

[Richard Nelson]

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

Ship it!


looks good to me.

- Richard


On May 27, 2011, 1:04 p.m., Log Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/315/
> ---
> 
> (Updated May 27, 2011, 1:04 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> This is a patch to fix the issues with clearing viewer caches documented in 
> VWR-25862. I removed the leading delimiter in the place where each of the 
> three caches are cleared.  I also removed the mac specific deleteFilesInDir() 
> method, which should have been removed for STORM-477. I removed unneeded 
> calls to getDirDelimiter() in llvocache.cpp.
> 
> 
> This addresses bugs vwr-25681 and vwr-25862.
> http://jira.secondlife.com/browse/vwr-25681
> http://jira.secondlife.com/browse/vwr-25862
> 
> 
> Diffs
> -
> 
>   indra/llvfs/lldir_mac.h UNKNOWN 
>   indra/llvfs/lldir_mac.cpp UNKNOWN 
>   indra/newview/llappviewer.cpp UNKNOWN 
>   indra/newview/lltexturecache.cpp UNKNOWN 
>   indra/newview/llvocache.cpp UNKNOWN 
> 
> Diff: http://codereview.secondlife.com/r/315/diff
> 
> 
> Testing
> ---
> 
> Test plan: https://jira.secondlife.com/browse/VWR-25862?#comment-262800
> 
> I built and tested on Mac, Windows and Linux and saw the correct behavior 
> when clicking the clear history button and the move cache button.
> 
> 
> 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

Re: [opensource-dev] Review Request: Enable watchdog timer for Beta crash hunting.

[Richard Nelson]

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

Ship it!


- Richard


On June 14, 2011, 2:50 p.m., Jenn wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/337/
> ---
> 
> (Updated June 14, 2011, 2:50 p.m.)
> 
> 
> Review request for Viewer, Alain Linden, Richard Nelson, and Nat Goodspeed.
> 
> 
> Summary
> ---
> 
> Enable watchdog timer for Beta release to get tracebacks for classes of 
> crashes for which we currently have little data.
> 
> Setting timer to 20 seconds. Will either disable or make longer for 
> deployment to the Release channel.
> 
> 
> Diffs
> -
> 
>   indra/newview/app_settings/settings.xml a7c507c7e0f8 
> 
> Diff: http://codereview.secondlife.com/r/337/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jenn
> 
>

___
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-1460 (Cursor doesn't go to the input field on the Find Floater due to lack of window focus)

[Richard Nelson]

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


Please don't use code to manually set focus unless you know for sure that 
xui-based focus control doesn't work.

In this case, just add tab_stop="true" to the web_browser widget inside 
floater_search.xml

- Richard


On July 8, 2011, 5:16 a.m., Paul ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/386/
> ---
> 
> (Updated July 8, 2011, 5:16 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Reason:
> Focus didn't go to the browser
> 
> Solution:
> Set the focus to the browser if a page is loaded and a floater containing a 
> browser has a focus
> 
> 
> This addresses bug storm-1460.
> http://jira.secondlife.com/browse/storm-1460
> 
> 
> Diffs
> -
> 
>   indra/newview/llfloatersearch.h 68ad362920c1 
>   indra/newview/llfloatersearch.cpp 68ad362920c1 
> 
> Diff: http://codereview.secondlife.com/r/386/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Paul
> 
>

___
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-1234) In Nearby Chat, Group Chat and IM Chat, URL selection frequently fails to grab last character

[Richard Nelson]

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

Ship it!


once again all the -1's start creeping back in :)  At least it's clearly 
documented here.

- Richard


On July 13, 2011, 9:06 a.m., Seth ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/394/
> ---
> 
> (Updated July 13, 2011, 9:06 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> Fixed text editor selection which was skipping the last character in line.
> 
> Added handling of the wrapped lines case when calculating the starting 
> position for selection.
> 
> The bug is a regression caused by the fix of STORM-320 which was dealing with 
> calculating the position of the input cursor while navigating through the 
> lines of text with word wrapping enabled.
> 
> 
> This addresses bug STORM-1234.
> http://jira.secondlife.com/browse/STORM-1234
> 
> 
> Diffs
> -
> 
>   indra/llui/lltextbase.cpp 68ad362920c1 
> 
> Diff: http://codereview.secondlife.com/r/394/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: Change pre-login viewer display URL to prepare for new community information display

[Richard Nelson]

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

Ship it!


Looks good, let's try it without the change to settings.xml, though.

- Richard


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-1521 [previously VWR-25588]: Zi's proposed fix for FIRE-543 - Hovertext renders as overlay on top of everything else

[Richard Nelson]

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

Ship it!


Looks good

- Richard


On July 21, 2011, 2:55 a.m., Boroondas Gupte wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/405/
> ---
> 
> (Updated July 21, 2011, 2:55 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Set the z (depth) coordinate of hovertext such that the hovertext gets 
> occluded by stuff in front of it.
> 
> 
> This addresses bugs SH-489, VWR-24017 and VWR-25588.
> http://jira.secondlife.com/browse/SH-489
> http://jira.secondlife.com/browse/VWR-24017
> http://jira.secondlife.com/browse/VWR-25588
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 46c7b36793c9 
>   indra/llrender/llfontgl.cpp 46c7b36793c9 
> 
> Diff: http://codereview.secondlife.com/r/405/diff
> 
> 
> Testing
> ---
> 
> Merged this change with e67da2c6e312 (as I still cannot build 
> viewer-development tip) and rebuilt. Verified that my nametag and an objects 
> llSetText hovertext were correctly occluded by another object in front of 
> them.
> 
> 
> Thanks,
> 
> Boroondas
> 
>

___
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-632 URL-like resident display name is shown as clickable HTTP URL in various places (additional fix)

[Richard Nelson]

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



indra/llui/llurlentry.cpp
<http://codereview.secondlife.com/r/433/#comment990>

This regexp will not allow nested tags.  I'm not sure if we can handle or 
use nested tags, but it would seem to be better to use:

boost::regex(".?")

this will consume as few characters as possible between the opening and 
closing tags, thus allowing multiple "nolink"s


- Richard


On Aug. 10, 2011, 8:25 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/433/
> ---
> 
> (Updated Aug. 10, 2011, 8:25 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> Disallow showing URL-like avatar display names as Web links in the 
> ShareItemsConfirmation notification.
> 
> Updating the  tag parsing regexp to enable multiple 
> ... clauses in a single textbox.
> 
> 
> This addresses bug STORM-632.
> http://jira.secondlife.com/browse/STORM-632
> 
> 
> Diffs
> -
> 
>   indra/llui/llurlentry.cpp 99a6889a65ec 
>   indra/newview/skins/default/xui/en/notifications.xml 99a6889a65ec 
> 
> Diff: http://codereview.secondlife.com/r/433/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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-632 URL-like resident display name is shown as clickable HTTP URL in various places (additional fix)

[Richard Nelson]

> On Aug. 11, 2011, 12:29 p.m., Richard Nelson wrote:
> > indra/llui/llurlentry.cpp, line 1137
> > <http://codereview.secondlife.com/r/433/diff/1/?file=6909#file6909line1137>
> >
> > This regexp will not allow nested tags.  I'm not sure if we can handle 
> > or use nested tags, but it would seem to be better to use:
> > 
> > boost::regex(".?")
> > 
> > this will consume as few characters as possible between the opening and 
> > closing tags, thus allowing multiple "nolink"s
> 
> Vadim ProductEngine wrote:
> If I'm not mistaken, this regexp would not allow more than one character 
> between the tags.

You are right, I meant .*?


- Richard


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


On Aug. 10, 2011, 8:25 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/433/
> -------
> 
> (Updated Aug. 10, 2011, 8:25 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> Disallow showing URL-like avatar display names as Web links in the 
> ShareItemsConfirmation notification.
> 
> Updating the  tag parsing regexp to enable multiple 
> ... clauses in a single textbox.
> 
> 
> This addresses bug STORM-632.
> http://jira.secondlife.com/browse/STORM-632
> 
> 
> Diffs
> -
> 
>   indra/llui/llurlentry.cpp 99a6889a65ec 
>   indra/newview/skins/default/xui/en/notifications.xml 99a6889a65ec 
> 
> Diff: http://codereview.secondlife.com/r/433/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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-56] As Builder, I want more decimal places allowed in the Build tool so that I can more precisely align small prims.

[Richard Nelson]

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


We picked the precision values for those spinners based on what would survive 
the roundtrip via our precision-limited object update packets.  Adding more 
precision will possibly result in confusion when the values don't respond the 
way you expect them to.  This is particularly a problem for rotation...position 
and scale could probably survive another decimal point...I haven't done the 
math.

- Richard


On July 29, 2011, 4:40 p.m., Kadah Coba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/424/
> ---
> 
> (Updated July 29, 2011, 4:40 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Adds 2 features:
> 
> Adds the ability to use modifier keys to change the defined increment on 
> LLSpinCtrl.
> Alt: increment x10
> Ctrl: increment x0.1
> Shift: increment x0.01
> 
> Adds extra decimal places to position, size, and rotation on build floater.
> Also changes LLPanelObject::sendPosition and LLPanelObject::sendScale to set 
> smaller changes in the associated values, no change was needed for 
> LLPanelObject::sendRotation. There should be no ill effects from this, 
> changes are still only sent to sim on button release.
> 
> https://bitbucket.org/Kadah_Coba/storm-56
> 
> 
> This addresses bug STORM-56.
> http://jira.secondlife.com/browse/STORM-56
> 
> 
> Diffs
> -
> 
>   indra/llui/llspinctrl.cpp UNKNOWN 
>   indra/newview/llpanelobject.cpp UNKNOWN 
>   indra/newview/skins/default/xui/en/floater_tools.xml UNKNOWN 
> 
> Diff: http://codereview.secondlife.com/r/424/diff
> 
> 
> Testing
> ---
> 
> Same code has been in use on FS beta for some time. I have built and tested 
> this myself on v-d.
> 
> 
> Thanks,
> 
> Kadah
> 
>

___
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-632 URL-like resident display name is shown as clickable HTTP URL in various places (additional fix)

[Richard Nelson]

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

Ship it!


looks good!

- Richard


On Aug. 12, 2011, 5:38 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/433/
> ---
> 
> (Updated Aug. 12, 2011, 5:38 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> Disallow showing URL-like avatar display names as Web links in the 
> ShareItemsConfirmation notification.
> 
> Updating the  tag parsing regexp to enable multiple 
> ... clauses in a single textbox.
> 
> 
> This addresses bug STORM-632.
> http://jira.secondlife.com/browse/STORM-632
> 
> 
> Diffs
> -
> 
>   indra/llui/llurlentry.cpp 99a6889a65ec 
>   indra/newview/skins/default/xui/en/notifications.xml 99a6889a65ec 
> 
> Diff: http://codereview.secondlife.com/r/433/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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-1268 Viewer update resets some viewer settings

[Richard Nelson]

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

Ship it!


good catch!

- Richard


On Aug. 19, 2011, 3:55 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/443/
> ---
> 
> (Updated Aug. 19, 2011, 3:55 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> Bug:
> Settings for ignorable dialogs were reset during deferred auto-upgrade.
> 
> Reason:
> In case of deferred upgrade (i.e. when you select "Later...") the defaults 
> for notifications settings are not loaded,
> so when the viewer exits after launching the updater, it incorrectly re-saves 
> notifications settings.
> 
> Fix:
> Initialize settings earlier, so that viewer picks them up in update mode.
> 
> 
> This addresses bug STORM-1268.
> http://jira.secondlife.com/browse/STORM-1268
> 
> 
> Diffs
> -
> 
>   indra/newview/llappviewer.cpp 478aabd2813b 
> 
> Diff: http://codereview.secondlife.com/r/443/diff
> 
> 
> Testing
> ---
> 
> See acceptance criteria in the JIRA ticket.
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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-1543 During busy mode inventory offers get silently deleted instead of thrown into trash

[Richard Nelson]

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



indra/newview/llviewermessage.cpp
<http://codereview.secondlife.com/r/444/#comment1005>

since we don't set IOR_BUSY anywhere (and its not a valid response in any 
notification), let's delete the IOR_BUSY value altogether and replace this line 
with

BOOL busy = gAgent.getBusy();

That will be a lot less confusing to the next person who needs to touch 
this code.


- Richard


On Aug. 22, 2011, 4:10 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/444/
> ---
> 
> (Updated Aug. 22, 2011, 4:10 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> In busy mode offered inventory items are now moved to trash.
> 
> There were two bugs:
> 
> 1. When auto-discarding inventory offers we looked up missing Busy button
> (i.e. a button having index=3) in the inventory offer notification dialog
> template. Failure to find the button resulted in ignoring inventory offers.
> 
> Fixed that by "auto-clicking" the existing Discard button.
> 
> 2. It turned out impossible to properly remove an inventory item
> from within LLDiscardAgentOffer::done(), because that would lead to
> nested LLInventoryModel::notifyObservers() calls.
> 
> Fixed that by deferring removal until the next LLAppViewer::idle() iteration.
> 
> 
> This addresses bug STORM-1543.
> http://jira.secondlife.com/browse/STORM-1543
> 
> 
> Diffs
> -
> 
>   indra/newview/llappviewer.h 7dbd8eaefaec 
>   indra/newview/llappviewer.cpp 7dbd8eaefaec 
>   indra/newview/llviewermessage.cpp 7dbd8eaefaec 
> 
> Diff: http://codereview.secondlife.com/r/444/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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-1543 During busy mode inventory offers get silently deleted instead of thrown into trash

[Richard Nelson]

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

Ship it!


great!

- Richard


On Aug. 25, 2011, 2:23 p.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/444/
> ---
> 
> (Updated Aug. 25, 2011, 2:23 p.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> In busy mode offered inventory items are now moved to trash.
> 
> There were two bugs:
> 
> 1. When auto-discarding inventory offers we looked up missing Busy button
> (i.e. a button having index=3) in the inventory offer notification dialog
> template. Failure to find the button resulted in ignoring inventory offers.
> 
> Fixed that by "auto-clicking" the existing Discard button.
> 
> 2. It turned out impossible to properly remove an inventory item
> from within LLDiscardAgentOffer::done(), because that would lead to
> nested LLInventoryModel::notifyObservers() calls.
> 
> Fixed that by deferring removal until the next LLAppViewer::idle() iteration.
> 
> 
> This addresses bug STORM-1543.
> http://jira.secondlife.com/browse/STORM-1543
> 
> 
> Diffs
> -
> 
>   indra/newview/llappviewer.h 7dbd8eaefaec 
>   indra/newview/llappviewer.cpp 7dbd8eaefaec 
>   indra/newview/llviewermessage.h 7dbd8eaefaec 
>   indra/newview/llviewermessage.cpp 7dbd8eaefaec 
> 
> Diff: http://codereview.secondlife.com/r/444/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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-638 "Object Return" doesn't return distant objects

[Richard Nelson]

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



indra/newview/llviewermenu.cpp


This is dangerous!  Never hold on to raw pointers to llviewerobject, as 
they can be deleted from under you.  Use LLPointer instead.

I looked into fixing the underlying problem, but that involves revisiting 
the design of selections.  We always deselect objects beyond a certain distance 
in order to keep your avatar proximate to the objects you can affect (for 
various technical and social reasons).  This workaround is fine for now, if not 
ideal.


- Richard


On Aug. 26, 2011, 8:38 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/451/
> ---
> 
> (Updated Aug. 26, 2011, 8:38 a.m.)
> 
> 
> Review request for Viewer and Seth ProductEngine.
> 
> 
> Summary
> ---
> 
> Reason: Showing the confirmation dialog resets object selection,
> thus there is nothing to derez.
> 
> Fix: Save selection until user answers in the confirmation dialog.
> 
> I didn't investigate why the bug occurred only for distant object
> (must be some internal LLSelectMgr magic).
> 
> 
> This addresses bug STORM-638.
> http://jira.secondlife.com/browse/STORM-638
> 
> 
> Diffs
> -
> 
>   indra/newview/llviewermenu.cpp 64ed6f9362af 
> 
> Diff: http://codereview.secondlife.com/r/451/diff
> 
> 
> Testing
> ---
> 
> See acceptance criteria in the JIRA ticket.
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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-638 "Object Return" doesn't return distant objects

[Richard Nelson]

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


do not ship until fixing the pointer problem

- Richard


On Aug. 26, 2011, 8:38 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/451/
> ---
> 
> (Updated Aug. 26, 2011, 8:38 a.m.)
> 
> 
> Review request for Viewer and Seth ProductEngine.
> 
> 
> Summary
> ---
> 
> Reason: Showing the confirmation dialog resets object selection,
> thus there is nothing to derez.
> 
> Fix: Save selection until user answers in the confirmation dialog.
> 
> I didn't investigate why the bug occurred only for distant object
> (must be some internal LLSelectMgr magic).
> 
> 
> This addresses bug STORM-638.
> http://jira.secondlife.com/browse/STORM-638
> 
> 
> Diffs
> -
> 
>   indra/newview/llviewermenu.cpp 64ed6f9362af 
> 
> Diff: http://codereview.secondlife.com/r/451/diff
> 
> 
> Testing
> ---
> 
> See acceptance criteria in the JIRA ticket.
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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-638 "Object Return" doesn't return distant objects

[Richard Nelson]

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

Ship it!


thanks

- Richard


On Aug. 27, 2011, 3:01 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/451/
> ---
> 
> (Updated Aug. 27, 2011, 3:01 a.m.)
> 
> 
> Review request for Viewer and Seth ProductEngine.
> 
> 
> Summary
> ---
> 
> Reason: Showing the confirmation dialog resets object selection,
> thus there is nothing to derez.
> 
> Fix: Save selection until user answers in the confirmation dialog.
> 
> I didn't investigate why the bug occurred only for distant object
> (must be some internal LLSelectMgr magic).
> 
> 
> This addresses bug STORM-638.
> http://jira.secondlife.com/browse/STORM-638
> 
> 
> Diffs
> -
> 
>   indra/newview/llviewermenu.cpp 00b00b942134 
> 
> Diff: http://codereview.secondlife.com/r/451/diff
> 
> 
> Testing
> ---
> 
> See acceptance criteria in the JIRA ticket.
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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-1112 Support SOCKS 5 proxy in the viewer (take 2)

[Richard Nelson]

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



indra/llcommon/llsingleton.h


why are we getting the data from within the destructor of the singleton 
itself?  Why not just do if (mInitState != DELETED) instead?  Seems like it 
would be less confusing.



indra/llmessage/llcurl.h


maybe a little comment as to what "Easy" is in this context.  I know you 
just moved the code to the header, but now that it is more than an 
implementation detail of LLCurl, I think our expectation of documentation has 
increased.



indra/llmessage/llcurl.cpp


if time_out is unused now, let's remove it from the signature



indra/llmessage/llproxy.h


do all these #defines and socks structs need to be in the header?



indra/llmessage/llproxy.h


I think we need something a bit more obvious to call out locking methods, 
maybe just a heavier comment header?



indra/llmessage/llproxy.cpp


control-flow wise, it might be clearer to move the final check for status 
to a separate if, with appropriate comment along the lines of "if any of the 
above failed..."



indra/llmessage/llproxy.cpp


can we reflect the potentially long blocking period in the name of the 
method?

tcp_wait_for_handshake or similar?


- Richard


On Aug. 25, 2011, 11:15 a.m., Log Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/374/
> ---
> 
> (Updated Aug. 25, 2011, 11:15 a.m.)
> 
> 
> Review request for Viewer, Oz Linden, Monty Brandenberg, and Stone Linden.
> 
> 
> Summary
> ---
> 
> This is a continuation of Robin Cornelius's SOCKS 5 contribution, shown in 
> https://codereview.secondlife.com/r/232/ .  I have tried to address all of 
> the comments on that code review and do as much cleanup as possible. The diff 
> includes everything that was submitted by Robin, as well as my work.
> Major changes since I started working:
> * Changed SOCKS 5 proxy control channel to use the existing LLSocket class, 
> which is a thin wrapper around APR sockets.
> * Worked with the Linden Lab UX team to revamp the proxy controls.
> * Proxy credentials are now stored in the LLSecAPI password storage, which is 
> the same that is used for users' Second Life Credentials instead of as being 
> stored in the clear as a preference.
> 
> 
> This addresses bug STORM-1112.
> http://jira.secondlife.com/browse/STORM-1112
> 
> 
> Diffs
> -
> 
>   indra/llcommon/llerror.h 4ebbd04efd93 
>   indra/llcommon/llerror.cpp 4ebbd04efd93 
>   indra/llcommon/llsingleton.h 4ebbd04efd93 
>   indra/llmessage/CMakeLists.txt 4ebbd04efd93 
>   indra/llmessage/llcurl.h 4ebbd04efd93 
>   indra/llmessage/llcurl.cpp 4ebbd04efd93 
>   indra/llmessage/llhttpassetstorage.cpp 4ebbd04efd93 
>   indra/llmessage/llhttpclient.cpp 4ebbd04efd93 
>   indra/llmessage/lliosocket.h 4ebbd04efd93 
>   indra/llmessage/lliosocket.cpp 4ebbd04efd93 
>   indra/llmessage/llpacketring.h 4ebbd04efd93 
>   indra/llmessage/llpacketring.cpp 4ebbd04efd93 
>   indra/llmessage/llproxy.h PRE-CREATION 
>   indra/llmessage/llproxy.cpp PRE-CREATION 
>   indra/llmessage/llurlrequest.cpp 4ebbd04efd93 
>   indra/llmessage/net.h 4ebbd04efd93 
>   indra/llmessage/net.cpp 4ebbd04efd93 
>   indra/llui/llfunctorregistry.h 4ebbd04efd93 
>   indra/newview/app_settings/settings.xml 4ebbd04efd93 
>   indra/newview/llappviewer.cpp 4ebbd04efd93 
>   indra/newview/llfloaterpreference.h 4ebbd04efd93 
>   indra/newview/llfloaterpreference.cpp 4ebbd04efd93 
>   indra/newview/llloginhandler.cpp 4ebbd04efd93 
>   indra/newview/llpanellogin.h 4ebbd04efd93 
>   indra/newview/llsecapi.h 4ebbd04efd93 
>   indra/newview/llstartup.h 4ebbd04efd93 
>   indra/newview/llstartup.cpp 4ebbd04efd93 
>   indra/newview/llviewerfloaterreg.cpp 4ebbd04efd93 
>   indra/newview/llxmlrpctransaction.cpp 4ebbd04efd93 
>   indra/newview/skins/default/xui/en/floater_preferences_proxy.xml 
> PRE-CREATION 
>   indra/newview/skins/default/xui/en/notifications.xml 4ebbd04efd93 
>   indra/newview/skins/default/xui/en/panel_cof_wearables.xml 4ebbd04efd93 
>   indra/newview/skins/default/xui/en/panel_preferences_privacy.xml 
> 4ebbd04efd93 
>   indra/newview/skins/default/xui/en/panel_preferences_setup.xml 4ebbd04efd93 
> 
> Diff: http://codereview.secondlife.com/r/374/diff
> 
> 
> Testing
> ---
> 
> I've t

Re: [opensource-dev] Review Request: STORM-1112 Support SOCKS 5 proxy in the viewer (take 2)

[Richard Nelson]

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

Ship it!


my comments were mostly about style and making the code a bit more self 
documenting.  I couldn't find any obvious logic errors, memory leaks, buffer 
overruns, data races, etc.  So, as far as I'm concerned, it's shippable.

- Richard


On Aug. 25, 2011, 11:15 a.m., Log Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/374/
> ---
> 
> (Updated Aug. 25, 2011, 11:15 a.m.)
> 
> 
> Review request for Viewer, Oz Linden, Monty Brandenberg, and Stone Linden.
> 
> 
> Summary
> ---
> 
> This is a continuation of Robin Cornelius's SOCKS 5 contribution, shown in 
> https://codereview.secondlife.com/r/232/ .  I have tried to address all of 
> the comments on that code review and do as much cleanup as possible. The diff 
> includes everything that was submitted by Robin, as well as my work.
> Major changes since I started working:
> * Changed SOCKS 5 proxy control channel to use the existing LLSocket class, 
> which is a thin wrapper around APR sockets.
> * Worked with the Linden Lab UX team to revamp the proxy controls.
> * Proxy credentials are now stored in the LLSecAPI password storage, which is 
> the same that is used for users' Second Life Credentials instead of as being 
> stored in the clear as a preference.
> 
> 
> This addresses bug STORM-1112.
> http://jira.secondlife.com/browse/STORM-1112
> 
> 
> Diffs
> -
> 
>   indra/llcommon/llerror.h 4ebbd04efd93 
>   indra/llcommon/llerror.cpp 4ebbd04efd93 
>   indra/llcommon/llsingleton.h 4ebbd04efd93 
>   indra/llmessage/CMakeLists.txt 4ebbd04efd93 
>   indra/llmessage/llcurl.h 4ebbd04efd93 
>   indra/llmessage/llcurl.cpp 4ebbd04efd93 
>   indra/llmessage/llhttpassetstorage.cpp 4ebbd04efd93 
>   indra/llmessage/llhttpclient.cpp 4ebbd04efd93 
>   indra/llmessage/lliosocket.h 4ebbd04efd93 
>   indra/llmessage/lliosocket.cpp 4ebbd04efd93 
>   indra/llmessage/llpacketring.h 4ebbd04efd93 
>   indra/llmessage/llpacketring.cpp 4ebbd04efd93 
>   indra/llmessage/llproxy.h PRE-CREATION 
>   indra/llmessage/llproxy.cpp PRE-CREATION 
>   indra/llmessage/llurlrequest.cpp 4ebbd04efd93 
>   indra/llmessage/net.h 4ebbd04efd93 
>   indra/llmessage/net.cpp 4ebbd04efd93 
>   indra/llui/llfunctorregistry.h 4ebbd04efd93 
>   indra/newview/app_settings/settings.xml 4ebbd04efd93 
>   indra/newview/llappviewer.cpp 4ebbd04efd93 
>   indra/newview/llfloaterpreference.h 4ebbd04efd93 
>   indra/newview/llfloaterpreference.cpp 4ebbd04efd93 
>   indra/newview/llloginhandler.cpp 4ebbd04efd93 
>   indra/newview/llpanellogin.h 4ebbd04efd93 
>   indra/newview/llsecapi.h 4ebbd04efd93 
>   indra/newview/llstartup.h 4ebbd04efd93 
>   indra/newview/llstartup.cpp 4ebbd04efd93 
>   indra/newview/llviewerfloaterreg.cpp 4ebbd04efd93 
>   indra/newview/llxmlrpctransaction.cpp 4ebbd04efd93 
>   indra/newview/skins/default/xui/en/floater_preferences_proxy.xml 
> PRE-CREATION 
>   indra/newview/skins/default/xui/en/notifications.xml 4ebbd04efd93 
>   indra/newview/skins/default/xui/en/panel_cof_wearables.xml 4ebbd04efd93 
>   indra/newview/skins/default/xui/en/panel_preferences_privacy.xml 
> 4ebbd04efd93 
>   indra/newview/skins/default/xui/en/panel_preferences_setup.xml 4ebbd04efd93 
> 
> Diff: http://codereview.secondlife.com/r/374/diff
> 
> 
> Testing
> ---
> 
> I've tested exclusively on Linux so far.  I'm working on a more extensive 
> test plan that includes setting up a gateway with a restrictive firewall to 
> verify that all traffic is going through the proxy. 
> 
> Test builds and screenshots of the changed UI elements are available from the 
> project page, located here:
> https://wiki.secondlife.com/wiki/User:Log_Linden/Socks5Viewer
> 
> 
> 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

Re: [opensource-dev] Review Request: LLProxy bugfixes and cleanup.

[Richard Nelson]

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

Ship it!


Looks good!

- Richard


On Sept. 7, 2011, 7:49 a.m., Log Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/463/
> ---
> 
> (Updated Sept. 7, 2011, 7:49 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> The LLProxy code was merged before I could get all the recommended code 
> review changes implemented, so these changes are all bugfixes to the code 
> that I changed in https://codereview.secondlife.com/r/374/, which is already 
> in viewer-development. I also added a simple unit test of the LLSingleton 
> template to test the functionality that I added.
> 
> 
> Diffs
> -
> 
>   indra/llcommon/CMakeLists.txt 04642a178228 
>   indra/llcommon/llsingleton.h 04642a178228 
>   indra/llcommon/tests/llsingleton_test.cpp PRE-CREATION 
>   indra/llmessage/llcurl.h 04642a178228 
>   indra/llmessage/llcurl.cpp 04642a178228 
>   indra/llmessage/llpacketring.h 04642a178228 
>   indra/llmessage/llpacketring.cpp 04642a178228 
>   indra/llmessage/llproxy.h 04642a178228 
>   indra/llmessage/llproxy.cpp 04642a178228 
> 
> Diff: http://codereview.secondlife.com/r/463/diff
> 
> 
> Testing
> ---
> 
> Builds with this code are available on the project page: 
> https://wiki.secondlife.com/wiki/User:Log_Linden/Socks5Viewer .
> 
> 
> 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

Re: [opensource-dev] Review Request: STORM-1578, STORM-1589: Away improvements

[Richard Nelson]

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

Ship it!


great!

- Richard


On Sept. 14, 2011, 12:20 p.m., Oz Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/473/
> ---
> 
> (Updated Sept. 14, 2011, 12:20 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Remove the viewer side automatic logout after extended away time.
> Note, however, that the server still logs you out after 30 minutes
> away: see SVC 7251.
> Allow more time (10 seconds vs 2 seconds) after initiating Away status
> during which mouse movements do not remove the Away status.
> 
> 
> This addresses bug storm-1578.
> http://jira.secondlife.com/browse/storm-1578
> 
> 
> Diffs
> -
> 
>   indra/newview/app_settings/settings.xml ffa9260a0b27 
>   indra/newview/llagent.h ffa9260a0b27 
>   indra/newview/llagent.cpp ffa9260a0b27 
>   indra/newview/llappviewer.cpp ffa9260a0b27 
>   indra/newview/llviewerjoystick.cpp ffa9260a0b27 
>   indra/newview/llviewerwindow.cpp ffa9260a0b27 
> 
> Diff: http://codereview.secondlife.com/r/473/diff
> 
> 
> Testing
> ---
> 
> I've done local testing with these changes.
> 
> For some reason I can't figure out, adding just these changes to the 
> then-current viewer-development tip increased the likelihood (to something 
> very close to 100%) of crashing due to problems with multi-threaded curl use. 
>   There is a separate proposed fix for that (see STORM-1562), and integrating 
> that proposed fix eliminated the problem and allowed me to test these changes.
> 
> 
> 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-1600 Error in focus while naming a new item - may lead to content loss

[Richard Nelson]

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

Ship it!


seems reasonable

- Richard


On Sept. 16, 2011, 9:14 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/478/
> ---
> 
> (Updated Sept. 16, 2011, 9:14 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> Folder view stole "Delete" key presses from the line editor when renaming a 
> newly created inventory item.
> 
> Reason:
> LLFocusMgr::setKeyboardFocus() which is called from LLLineEditor::setFocus()
> makes the folder view the edit menu handler, thus it receives the Delete key
> presses instead of the line editor.
> 
> Fix:
> Make sure the line editor becomes the edit menu handler whenever it's focused,
> no matter is it a child of a folder view or not.
> 
> 
> This addresses bug STORM-1600.
> http://jira.secondlife.com/browse/STORM-1600
> 
> 
> Diffs
> -
> 
>   indra/llui/lllineeditor.cpp 656d988266e8 
> 
> Diff: http://codereview.secondlife.com/r/478/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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: Texture Picker: Making the preview "widget" a little more flexible.

[Richard Nelson]

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


Please do not use localization strings to parameterize a widget.  Better to use 
a place holder  and then call getChildView on it and use the rect of that 
view for displaying the texture preview.

- Richard


On Oct. 4, 2011, 7:26 a.m., Vaalith Jinn wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/474/
> ---
> 
> (Updated Oct. 4, 2011, 7:26 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> Update: would appreciate a LL yes or no to this, as it's one of the issues 
> that's holding back STORM-64.
> 
> 
> Texture picker's preview box was never a real widget. It had no XUI presence 
> so the XUI designers had no control over it.
> Also, it's height depended on the it's parent floater's min size which meant 
> that increasing the min size distorted the preview box,
> which must be at a 1:1 ratio.
> 
> I have modified the code in a way that
> a) makes the preview box no longer reliant on any of the floater's own 
> parameters.
> b) gives the XUI designers three parameters that affect the preview box 
> specifically: left, top, and size.
> (size is both the size from left to right, and from top to bottom, keeping it 
> locked to 1:1)
> 
> The changes are only noticeable to coders and xui designers, not the users.
> note: It is still not a real widget and it still lacks a follow, layout, 
> delta and any other widget specific abilities.
> 
> 
> Diffs
> -
> 
>   indra/newview/lltexturectrl.cpp d36e49ee2651 
>   indra/newview/skins/default/xui/en/floater_texture_ctrl.xml d36e49ee2651 
> 
> Diff: http://codereview.secondlife.com/r/474/diff
> 
> 
> Testing
> ---
> 
> The box seems to behave exactly as it did before.
> Changing the indicated left, top, and size values in the appropriate xml file 
> does affect the relevant properties. 
> 
> 
> Thanks,
> 
> Vaalith
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: Texture Picker: Making the preview "widget" a little more flexible.

[Richard Nelson]

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

Ship it!


thanks, looks good!

- Richard


On Oct. 4, 2011, 8:11 p.m., Vaalith Jinn wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/474/
> ---
> 
> (Updated Oct. 4, 2011, 8:11 p.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> Update: would appreciate a LL yes or no to this, as it's one of the issues 
> that's holding back STORM-64.
> 
> 
> Texture picker's preview box was never a real widget. It had no XUI presence 
> so the XUI designers had no control over it.
> Also, it's height depended on the it's parent floater's min size which meant 
> that increasing the min size distorted the preview box,
> which must be at a 1:1 ratio.
> 
> I have modified the code in a way that
> a) makes the preview box no longer reliant on any of the floater's own 
> parameters.
> b) gives the XUI designers three parameters that affect the preview box 
> specifically: left, top, and size.
> (size is both the size from left to right, and from top to bottom, keeping it 
> locked to 1:1)
> 
> The changes are only noticeable to coders and xui designers, not the users.
> note: It is still not a real widget and it still lacks a follow, layout, 
> delta and any other widget specific abilities.
> 
> 
> Diffs
> -
> 
>   indra/newview/lltexturectrl.cpp d36e49ee2651 
>   indra/newview/skins/default/xui/en/floater_texture_ctrl.xml d36e49ee2651 
> 
> Diff: http://codereview.secondlife.com/r/474/diff
> 
> 
> Testing
> ---
> 
> The box seems to behave exactly as it did before.
> Changing the indicated left, top, and size values in the appropriate xml file 
> does affect the relevant properties. 
> 
> 
> Thanks,
> 
> Vaalith
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: EXP-625 New user remains a cloud for an extended period on first login in Advanced mode

[Richard Nelson]

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

Ship it!


good catch!

- Richard


On Oct. 10, 2011, 5:05 p.m., Stone Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/489/
> ---
> 
> (Updated Oct. 10, 2011, 5:05 p.m.)
> 
> 
> Review request for Viewer, Oz Linden, Nyx Linden, Jenn, Richard Nelson, Aura 
> Linden, and Brad Payne.
> 
> 
> Summary
> ---
> 
> A new user logging in for the first time will remain a cloud for an 
> unfortunate period of time. Turns out this is because we were failing to 
> request the Library items corresponding to the user's desired initial outfit. 
> The bug could be worked around by taking some other Inventory action, which 
> would trigger a background fetch. This fix directly addresses the problem and 
> allows the fetch of items to take place immediately.
> 
> 
> This addresses bug EXP-625.
> http://jira.secondlife.com/browse/EXP-625
> 
> 
> Diffs
> -
> 
>   indra/newview/llinventoryobserver.cpp 88cf7d9a9d31 
> 
> Diff: http://codereview.secondlife.com/r/489/diff
> 
> 
> Testing
> ---
> 
> Compiles and reproduceably alleviates the bug. Tested with several 
> newly-created accounts.
> 
> 
> Thanks,
> 
> Stone
> 
>

___
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-1615 Please update language support for Viewer

[Richard Nelson]

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



indra/newview/skins/minimal/xui/ru/menu_script_chiclet.xml
<http://codereview.secondlife.com/r/490/#comment1057>

why are you manually setting these values?


- Richard


On Oct. 11, 2011, 10:48 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/490/
> ---
> 
> (Updated Oct. 11, 2011, 10:48 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> - Added Traditional Chinese to the Language dropdown menu in Preferences / 
> General.
>   The language is already supported, it just wasn't in the menu.
> - Added support for Russian, Simplified Chinese and Turkish.
>   * Added the new languages to the list in Preferences / General.
>   * Added a couple of "translated" XUI files for each language to create the 
> necessary folders.
> - Removed Dutch from the available languages list.
>   Dutch translations are still there, so you can still use Dutch by running 
> viewer with "--set Language nl".
> 
> Language codes: ru (Russian), tr (Turkish), zh_CN (Simplified Chinese), zh 
> (Traditional Chinese).
> 
> 
> This addresses bug STORM-1615.
> http://jira.secondlife.com/browse/STORM-1615
> 
> 
> Diffs
> -
> 
>   indra/newview/skins/default/xui/de/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/en/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/es/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/fr/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/it/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/ja/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/nl/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/pl/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/pt/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/ru/floater_buy_currency_html.xml 
> PRE-CREATION 
>   indra/newview/skins/default/xui/tr/floater_buy_currency_html.xml 
> PRE-CREATION 
>   indra/newview/skins/default/xui/zh/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/zh_CN/floater_buy_currency_html.xml 
> PRE-CREATION 
>   indra/newview/skins/minimal/xui/ru/menu_script_chiclet.xml PRE-CREATION 
>   indra/newview/skins/minimal/xui/tr/menu_script_chiclet.xml PRE-CREATION 
>   indra/newview/skins/minimal/xui/zh/menu_script_chiclet.xml PRE-CREATION 
>   indra/newview/skins/minimal/xui/zh_CN/menu_script_chiclet.xml PRE-CREATION 
> 
> Diff: http://codereview.secondlife.com/r/490/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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-1615 Please update language support for Viewer

[Richard Nelson]

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

Ship it!


- Richard


On Oct. 11, 2011, 10:48 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/490/
> ---
> 
> (Updated Oct. 11, 2011, 10:48 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Summary
> ---
> 
> - Added Traditional Chinese to the Language dropdown menu in Preferences / 
> General.
>   The language is already supported, it just wasn't in the menu.
> - Added support for Russian, Simplified Chinese and Turkish.
>   * Added the new languages to the list in Preferences / General.
>   * Added a couple of "translated" XUI files for each language to create the 
> necessary folders.
> - Removed Dutch from the available languages list.
>   Dutch translations are still there, so you can still use Dutch by running 
> viewer with "--set Language nl".
> 
> Language codes: ru (Russian), tr (Turkish), zh_CN (Simplified Chinese), zh 
> (Traditional Chinese).
> 
> 
> This addresses bug STORM-1615.
> http://jira.secondlife.com/browse/STORM-1615
> 
> 
> Diffs
> -
> 
>   indra/newview/skins/default/xui/de/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/en/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/es/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/fr/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/it/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/ja/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/nl/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/pl/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/pt/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/ru/floater_buy_currency_html.xml 
> PRE-CREATION 
>   indra/newview/skins/default/xui/tr/floater_buy_currency_html.xml 
> PRE-CREATION 
>   indra/newview/skins/default/xui/zh/panel_preferences_general.xml 
> 3af8218d32f1 
>   indra/newview/skins/default/xui/zh_CN/floater_buy_currency_html.xml 
> PRE-CREATION 
>   indra/newview/skins/minimal/xui/ru/menu_script_chiclet.xml PRE-CREATION 
>   indra/newview/skins/minimal/xui/tr/menu_script_chiclet.xml PRE-CREATION 
>   indra/newview/skins/minimal/xui/zh/menu_script_chiclet.xml PRE-CREATION 
>   indra/newview/skins/minimal/xui/zh_CN/menu_script_chiclet.xml PRE-CREATION 
> 
> Diff: http://codereview.secondlife.com/r/490/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vadim
> 
>

___
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: As a music fan, I want audio to fade in gently so my immersion is increased

[Richard Nelson]

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


Quick comment regarding behavior...we really should not play any audio during 
the teleport blackout as it is disconcerting to feel like you are still in an 
area (hearing the audio) but not be able to see or interact with anything.  
Everything else sounds great.

- Richard


On Nov. 23, 2011, 5:59 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/520/
> ---
> 
> (Updated Nov. 23, 2011, 5:59 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Audio fading in has been added for when a music stream starts.  Audio fading 
> out has been added for when there is a change in the music stream that is 
> playing.
> 
> Two dead files have been eliminated.
> 
> An existing bug in how music was not being paused correctly has been fixed.
> 
> When you are teleporting you will hear the music stream from the place you 
> are leaving.  This is a change in behavior; previously the music stream was 
> stopped when the teleport progress bar was being displayed.
> 
> This code change affects several areas where music is started or stopped.  
> The new code has evolved significantly, so please look for things that might 
> not "make sense."
> 
> 
> This addresses bug STORM-591.
> http://jira.secondlife.com/browse/STORM-591
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 8b455c1b7a5e 
>   indra/newview/lloverlaybar.h 8b455c1b7a5e 
>   indra/newview/lloverlaybar.cpp 8b455c1b7a5e 
>   indra/newview/llpanelnearbymedia.h 8b455c1b7a5e 
>   indra/newview/llpanelnearbymedia.cpp 8b455c1b7a5e 
>   indra/newview/llvieweraudio.h 8b455c1b7a5e 
>   indra/newview/llvieweraudio.cpp 8b455c1b7a5e 
>   indra/newview/llviewermedia.cpp 8b455c1b7a5e 
>   indra/newview/llviewerparcelmgr.cpp 8b455c1b7a5e 
> 
> Diff: http://codereview.secondlife.com/r/520/diff
> 
> 
> Testing
> ---
> 
> See the massive 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: As a music fan, I want audio to fade in gently so my immersion is increased

[Richard Nelson]

> On Nov. 23, 2011, 8:18 a.m., Richard Nelson wrote:
> > Quick comment regarding behavior...we really should not play any audio 
> > during the teleport blackout as it is disconcerting to feel like you are 
> > still in an area (hearing the audio) but not be able to see or interact 
> > with anything.  Everything else sounds great.
> 
> Jonathan Yap wrote:
> The reason I changed the behavior when the teleport progress bar was 
> showing was to keep the state engine logic simple -- it activates when there 
> is change in your current parcel music URL.
> 
> Otherwise you have to deal with various cases as exceptions to how the 
> state engine processes stream transitions.
> 
> If you force-fade the current stream when the teleport progress bar 
> appears and the teleport fails you'll have to fade the stream back in.  I am 
> not sure if there are other edge cases that might appear.
> 
> I suggest trying Oz's PO test viewer (there is a download link in a jira 
> comment).  Personally I felt having the music keep playing and then fade out 
> and in when you arrive was more enjoyable (you can consider it in-flight 
> entertainment).  This also might tie in well with any improvements to the 
> progress bar screen no longer being a blank page as was discussed in one of 
> Oz's user group meetings.
>
> 
> Oz Linden wrote:
> On the other hand, testing in the current Development viewer [Second Life 
> 3.2.4 (245662)], when I started from a parcel with a stream and TP'ed to a 
> new region&parcel without one, I got a sudden cutoff when the TP screen came 
> up but the stream restarted when the screen dropped and then cut off quickly 
> (presumably because new parcel properties had arrived).  That's arguably not 
> as good as a fade after teleport.

One of the in-progress changes to the teleport/log in screen would involve 
playing video+audio on that login screen, which should not be fighting with 
background audio.


- Richard


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


On Nov. 23, 2011, 5:59 a.m., Jonathan Yap wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/520/
> ---
> 
> (Updated Nov. 23, 2011, 5:59 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Audio fading in has been added for when a music stream starts.  Audio fading 
> out has been added for when there is a change in the music stream that is 
> playing.
> 
> Two dead files have been eliminated.
> 
> An existing bug in how music was not being paused correctly has been fixed.
> 
> When you are teleporting you will hear the music stream from the place you 
> are leaving.  This is a change in behavior; previously the music stream was 
> stopped when the teleport progress bar was being displayed.
> 
> This code change affects several areas where music is started or stopped.  
> The new code has evolved significantly, so please look for things that might 
> not "make sense."
> 
> 
> This addresses bug STORM-591.
> http://jira.secondlife.com/browse/STORM-591
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 8b455c1b7a5e 
>   indra/newview/lloverlaybar.h 8b455c1b7a5e 
>   indra/newview/lloverlaybar.cpp 8b455c1b7a5e 
>   indra/newview/llpanelnearbymedia.h 8b455c1b7a5e 
>   indra/newview/llpanelnearbymedia.cpp 8b455c1b7a5e 
>   indra/newview/llvieweraudio.h 8b455c1b7a5e 
>   indra/newview/llvieweraudio.cpp 8b455c1b7a5e 
>   indra/newview/llviewermedia.cpp 8b455c1b7a5e 
>   indra/newview/llviewerparcelmgr.cpp 8b455c1b7a5e 
> 
> Diff: http://codereview.secondlife.com/r/520/diff
> 
> 
> Testing
> ---
> 
> See the massive 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-1713: Mouse pointer flickers when hovering over any active/clickable UI item

[Richard Nelson]

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


Please do not associate the ability to set the mouse cursor with the fact that 
a view is mouse_opaque.  Mouse_opaque, a.k.a. blockMouseEvent is used as a 
shortcut that could easily be replaced by some complicated logic that always 
returns true.  It does not indicate the desire to update the mouse cursor.  The 
rule we use for mutating side effects (i.e. setting tooltip, changing mouse 
cursor, etc.) is that by default the leafmost view wins.  In this case, the 
proper fix is to store the current mouse cursor in your LLWindow* 
implementation and then only set it once a frame in LLWindow*::gatherInput()

- Richard Nelson


On Dec. 5, 2011, 9:20 a.m., Ansariel Hiller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/521/
> ---
> 
> (Updated Dec. 5, 2011, 9:20 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Description
> ---
> 
> The fix for the flickering mouse cursor consists of 2 parts:
> 
> The first part changes LLView::childrenHandleHover() so that the cursor is 
> only set if the control itself is blocking the mouse event and not at every 
> hierarchy level in the control hierarchy. If the cursor would be set at each 
> level, it would cause a flicker in case the different controls set a 
> different cursor. This change actually resembles the algorithm used prior the 
> start of the flickering.
> 
> The second part changes LLToolTip::handleHover() and specifically handles 
> flickering of the cursor while hovering over the "i"-button of a hovertip. 
> The general call to LLPanel::handleHover() was changed to be only called if 
> the tooltip itself does not set the cursor itself. Logging the calls to 
> LLWindowWin32::setCursor() revealed that if the "i"-button on a hovertop is 
> hovered with the cursor said method is called twice with different cursors 
> alternatively. Checking the call stack further revealed that one call is 
> coming from LLToolTip::handleHover() and the other one from 
> LLButton::handleHover(). Latter gets invoked if LLPanel::handleHover() is 
> called. Since nothing is really done here except setting the cursor to 
> UI_CURSOR_ARROW only ti get then set to UI_CURSOR_HAND if 
> LLPanel::handleHover() returns, it doesn't make sense to do invoke that 
> method unless the cursor is not changed in the tooltip itself. So 
> LLPanel::handleHover() is only invoked if the tooltip does not set the cursor 
> itself, so that child controls should take care.
> 
> 
> This addresses bug STORM-1713.
> http://jira.secondlife.com/browse/STORM-1713
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a4a5d827c7f5 
>   indra/llui/lltooltip.cpp a4a5d827c7f5 
>   indra/llui/llview.cpp a4a5d827c7f5 
> 
> Diff: http://codereview.secondlife.com/r/521/diff/diff
> 
> 
> Testing
> ---
> 
> Testing was done by myself on Firestorm rev. 24073 
> (http://hg.phoenixviewer.com/phoenix-firestorm-lgpl/rev/bcc95de39ca9) on 
> Windows 7 and Lance Corrimal on Dolphin Viewer. Apparently the fix works 
> without any side-effects
> 
> 
> Thanks,
> 
> Ansariel Hiller
> 
>

___
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-1713: Mouse pointer flickers when hovering over any active/clickable UI item

[Richard Nelson]

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



indra/llui/lltooltip.cpp
<http://codereview.secondlife.com/r/521/#comment1082>

let's keep the old code and make the cursor something that is set many 
times, but only propagated to the OS once per frame, like in gatherInput() or 
LLViewerWindow::draw();





indra/llui/llview.cpp
<http://codereview.secondlife.com/r/521/#comment1083>

I know this is reverting to older behavior, but the older behavior is wrong 
in tying the ability to set cursors to the mouse_opaque attribute.  


- Richard Nelson


On Dec. 5, 2011, 9:20 a.m., Ansariel Hiller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/521/
> ---
> 
> (Updated Dec. 5, 2011, 9:20 a.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Description
> ---
> 
> The fix for the flickering mouse cursor consists of 2 parts:
> 
> The first part changes LLView::childrenHandleHover() so that the cursor is 
> only set if the control itself is blocking the mouse event and not at every 
> hierarchy level in the control hierarchy. If the cursor would be set at each 
> level, it would cause a flicker in case the different controls set a 
> different cursor. This change actually resembles the algorithm used prior the 
> start of the flickering.
> 
> The second part changes LLToolTip::handleHover() and specifically handles 
> flickering of the cursor while hovering over the "i"-button of a hovertip. 
> The general call to LLPanel::handleHover() was changed to be only called if 
> the tooltip itself does not set the cursor itself. Logging the calls to 
> LLWindowWin32::setCursor() revealed that if the "i"-button on a hovertop is 
> hovered with the cursor said method is called twice with different cursors 
> alternatively. Checking the call stack further revealed that one call is 
> coming from LLToolTip::handleHover() and the other one from 
> LLButton::handleHover(). Latter gets invoked if LLPanel::handleHover() is 
> called. Since nothing is really done here except setting the cursor to 
> UI_CURSOR_ARROW only ti get then set to UI_CURSOR_HAND if 
> LLPanel::handleHover() returns, it doesn't make sense to do invoke that 
> method unless the cursor is not changed in the tooltip itself. So 
> LLPanel::handleHover() is only invoked if the tooltip does not set the cursor 
> itself, so that child controls should take care.
> 
> 
> This addresses bug STORM-1713.
> http://jira.secondlife.com/browse/STORM-1713
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a4a5d827c7f5 
>   indra/llui/lltooltip.cpp a4a5d827c7f5 
>   indra/llui/llview.cpp a4a5d827c7f5 
> 
> Diff: http://codereview.secondlife.com/r/521/diff/diff
> 
> 
> Testing
> ---
> 
> Testing was done by myself on Firestorm rev. 24073 
> (http://hg.phoenixviewer.com/phoenix-firestorm-lgpl/rev/bcc95de39ca9) on 
> Windows 7 and Lance Corrimal on Dolphin Viewer. Apparently the fix works 
> without any side-effects
> 
> 
> Thanks,
> 
> Ansariel Hiller
> 
>

___
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-1713: Mouse pointer flickers when hovering over any active/clickable UI item

[Richard Nelson]

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

Ship it!


I approve, thanks!  I'm guessing we will end up testing this on all 3 platforms 
during the merge process, so hopefully all goes well.

- Richard Nelson


On Dec. 5, 2011, 2:43 p.m., Ansariel Hiller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/521/
> ---
> 
> (Updated Dec. 5, 2011, 2:43 p.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Description
> ---
> 
> The fix for the flickering mouse cursor consists of 2 parts:
> 
> The first part changes LLView::childrenHandleHover() so that the cursor is 
> only set if the control itself is blocking the mouse event and not at every 
> hierarchy level in the control hierarchy. If the cursor would be set at each 
> level, it would cause a flicker in case the different controls set a 
> different cursor. This change actually resembles the algorithm used prior the 
> start of the flickering.
> 
> The second part changes LLToolTip::handleHover() and specifically handles 
> flickering of the cursor while hovering over the "i"-button of a hovertip. 
> The general call to LLPanel::handleHover() was changed to be only called if 
> the tooltip itself does not set the cursor itself. Logging the calls to 
> LLWindowWin32::setCursor() revealed that if the "i"-button on a hovertop is 
> hovered with the cursor said method is called twice with different cursors 
> alternatively. Checking the call stack further revealed that one call is 
> coming from LLToolTip::handleHover() and the other one from 
> LLButton::handleHover(). Latter gets invoked if LLPanel::handleHover() is 
> called. Since nothing is really done here except setting the cursor to 
> UI_CURSOR_ARROW only ti get then set to UI_CURSOR_HAND if 
> LLPanel::handleHover() returns, it doesn't make sense to do invoke that 
> method unless the cursor is not changed in the tooltip itself. So 
> LLPanel::handleHover() is only invoked if the tooltip does not set the cursor 
> itself, so that child controls should take care.
> 
> 
> This addresses bug STORM-1713.
> http://jira.secondlife.com/browse/STORM-1713
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a984f7ffeb4b 
>   indra/llwindow/llwindow.h a984f7ffeb4b 
>   indra/llwindow/llwindow.cpp a984f7ffeb4b 
>   indra/llwindow/llwindowheadless.h a984f7ffeb4b 
>   indra/llwindow/llwindowmacosx.h a984f7ffeb4b 
>   indra/llwindow/llwindowmacosx.cpp a984f7ffeb4b 
>   indra/llwindow/llwindowmesaheadless.h a984f7ffeb4b 
>   indra/llwindow/llwindowsdl.h a984f7ffeb4b 
>   indra/llwindow/llwindowsdl.cpp a984f7ffeb4b 
>   indra/llwindow/llwindowwin32.h a984f7ffeb4b 
>   indra/llwindow/llwindowwin32.cpp a984f7ffeb4b 
> 
> Diff: http://codereview.secondlife.com/r/521/diff/diff
> 
> 
> Testing
> ---
> 
> Testing was done by myself on Firestorm rev. 24073 
> (http://hg.phoenixviewer.com/phoenix-firestorm-lgpl/rev/bcc95de39ca9) on 
> Windows 7 and Lance Corrimal on Dolphin Viewer. Apparently the fix works 
> without any side-effects
> 
> 
> Thanks,
> 
> Ansariel Hiller
> 
>

___
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-1899: Avatar hand poses randomly get stuck in spread position

[Richard Nelson]

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



indra/llcharacter/llhandmotion.cpp
<http://codereview.secondlife.com/r/592/#comment1136>

you probably don't want to write to this memory if you are getting bad 
values.  Best to warn and then do nothing.


- Richard Nelson


On July 19, 2012, 2:11 p.m., Ansariel Hiller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/592/
> ---
> 
> (Updated July 19, 2012, 2:11 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Description
> ---
> 
> A proposed solution for STORM-1899 and the issue, where the handpose of 
> avatars randomly get stuck in the spread hand position.
> 
> In the current implementation, this case might happen under the following 
> circumstances:
> * Avatar uses an animation with hand pose A
> * A request is issued to change hand pose to B
> * Before the viewer has blended pose A over to pose B, the original pose A is 
> requested again
> * In this case, the hand pose will not be properly (re)set and because 
> mNewPose == mCurrentPose, there will be no further blending, leaving the hand 
> in it's current pose
> 
> The patch will properly reset the hand pose in this case and update the 
> visual parameters of the avatar.
> 
> 
> This addresses bug STORM-1899.
> https://jira.secondlife.com/browse/STORM-1899
> 
> 
> Diffs
> -
> 
>   indra/llcharacter/llhandmotion.cpp 4d9106153407 
> 
> Diff: http://codereview.secondlife.com/r/592/diff/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ansariel Hiller
> 
>

___
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-1899: Avatar hand poses randomly get stuck in spread position

[Richard Nelson]

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

Ship it!


Ship It!

- Richard Nelson


On July 25, 2012, 3:12 p.m., Ansariel Hiller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/592/
> ---
> 
> (Updated July 25, 2012, 3:12 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Description
> ---
> 
> A proposed solution for STORM-1899 and the issue, where the handpose of 
> avatars randomly get stuck in the spread hand position.
> 
> In the current implementation, this case might happen under the following 
> circumstances:
> * Avatar uses an animation with hand pose A
> * A request is issued to change hand pose to B
> * Before the viewer has blended pose A over to pose B, the original pose A is 
> requested again
> * In this case, the hand pose will not be properly (re)set and because 
> mNewPose == mCurrentPose, there will be no further blending, leaving the hand 
> in it's current pose
> 
> The patch will properly reset the hand pose in this case and update the 
> visual parameters of the avatar.
> 
> 
> This addresses bug STORM-1899.
> https://jira.secondlife.com/browse/STORM-1899
> 
> 
> Diffs
> -
> 
>   indra/llcharacter/llhandmotion.cpp 4d9106153407 
> 
> Diff: http://codereview.secondlife.com/r/592/diff/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ansariel Hiller
> 
>

___
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] XUI Cleanup Project

[Richard Nelson]

There is already code to round-trip XUI files through the param block  
mechanism such that invalid attributes and default values are dropped and  
formatting is normalized.  That code hasn't been exercised in a few years,  
though, so it probably won't run correctly right away, but it does provide  
a mechanism for keeping XUI files in line with changing param block schema  
going forward.

R.

On Fri, 03 Aug 2012 15:32:41 -0700, Kadah  wrote:

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> This weekend I was working on some XUI files and my OCD (I don't
> actually have OCD) was getting very frustrated over the random
> formatting, lack of consistent indentation, trailing whitespace,
> random/missing newlines and wildly differently sorted arguments.
> Usually I'd either completely retab the file or give up, but I was
> lazy and tired of not doing anything. So needless to say I got nothing
> done on in XUI and spent the few days writing a script to reformat XUI
> files to some level of code standards.
>
> There is no XML code standards on
> http://wiki.secondlife.com/wiki/Coding_standard
> So I used my own, which were as such:
> * Unix EOLs
> * Child elements indented +4 spaces
> * Each argument on onw line and indented +1
> * Arguments sorted by defined list then alpha order
>  (eg. 'name' and composting before random control specific args)
> * Comments must be preserved
>
> Initially I looked for existing tools/libs that can do XML cleanup,
> but I had the same results I found the last time I looked, that none
> of them could cover all the requirements. Surprisingly comment support
> was actually hardest to find support for, unsurprisingly argument
> sorting is extremely uncommon and the ones that did were poor.
> After that I attempted to find an XML lib for parsing, but found that
> pretty much all of them did not support comments or the tabled output
> was unstable.
>
> So I ended up writing the XML parser and reserializer myself.
> Everything appears to output correctly, but I lack a diff tool that
> supports XML to be able to do verification. The text diff tools I'm
> familiar with can only do word/character comparisons and the reordered
> args, and sometimes drastically improved whitespace, make manual
> checking tedious.
>
> So does anyone know of a, preferably, open source XML diff tool?
>
> I'm guessing it could be possible to extract some of the XML/XUI file
> parsing from the viewer and make one, but it would be a pain in the rear.
>
>
> If this works out, I'll release the scripts (MIT licensed), and submit
> the requisite jira/RB to push upstream XUI cleaned up, which would be
> really nice.
>
> Samples:
> Input:
> http://hg.secondlife.com/viewer-release/raw/24c6da14256c/indra/newview/skins/default/xui/en/panel_preferences_general.xml
> Output: http://pastebin.com/9K40N49w
>
> Input:
> http://hg.secondlife.com/viewer-release/raw/24c6da14256c/indra/newview/skins/default/xui/en/menu_login.xml
> Output: http://pastebin.com/i16De7xU
>
> - -Kadah
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v2.0.17 (MingW32)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iQEcBAEBAgAGBQJQHFGJAAoJEIdLfPRu7qE2jtwIAKFh7Xe7GJD97ar9kuaQAx13
> Iaj8ZaRB9HVfjdOiJJiT6aaTodEag0B23i19WH0fvq23kgZOGzXgiUppsw3TeiLr
> eTX0bnct/CqmTDx+afYJY9QExhb0osC8FuFzjxVld6KtQk46Wn6a58ljNvyc459e
> ySBCUgoivg+1oa5XqPP2UmUHE5OK0oqnhALKEfcLpW0AFpbv1yLzrJNEZYE3iIRn
> 3/g0mAhcpMtC6Um8XjR3+l/lTi50iTz2T5APnxWX8LNepwOvT2muffHgwGDGRwCt
> 6/e1SZjP9AI4hfVf4HM7Rn0JIc87Vp3zODf5KtCsw93er0Qmhi99OBZfn8DriXI=
> =GHbm
> -END PGP SIGNATURE-
> ___
> 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
___
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] XUI Cleanup Project

[Richard Nelson]

This code is triggered via LLUICtrlFactory::createFromFile() whenever a  
valid output_node is passed in.  There isn't currently any code that  
triggers that behavior, and there is no real documentation, other than  
what you can glean from the small number of comments in the source.

R.

On Fri, 03 Aug 2012 16:47:35 -0700, Kadah  wrote:

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> On 8/3/2012 3:36 PM, Richard Nelson wrote:
>> There is already code to round-trip XUI files through the param
>> block mechanism such that invalid attributes and default values are
>> dropped and formatting is normalized.  That code hasn't been
>> exercised in a few years, though, so it probably won't run
>> correctly right away, but it does provide a mechanism for keeping
>> XUI files in line with changing param block schema going forward.
>>
>> R.
>
> This is in the viewer somewhere? Is there any documentation on it?
>
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v2.0.17 (MingW32)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iQEcBAEBAgAGBQJQHGMXAAoJEIdLfPRu7qE29jIH/0sILRiF32s4FXKZDjgU6wNw
> 8gRi9gM2+b422ae3io6K22lAl/Tp3hoYiDDeaRvu8j+rBPjgbjJcPtSQBndWVsnI
> foFuHxSW3TIHTutXNmvWTVXFULwWXkCJQGVPi4CC1VCQewJsCKQsEnqk8/i52cKK
> nL5az4vnA6LqPHKE/VwX93h9m8OIQfsZKRQGysckFhfYLLWCGuhc+db6tN98WN7s
> gW6y772R01QDCm1C66Dp8GjUzrZK/ZVIxK33lFiqksX7cSqtcWoY4pHzudGlMMMP
> nYFuP2kQXL7svcsgcLiPq6zw0RYEEExnjgyY8VlBI1xteFwdO4J1wn/svjcR32I=
> =hcEF
> -END PGP SIGNATURE-
___
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] "Adding a dialog" outdated

[Richard Nelson]

Yes, LLFloaterReg is the current mechanism for creating and displaying new types of floaters (windows).There is a separate mechanism for "notification" style dialogs, in llnotifications*.h/cpp, appropriately enough.  Unfortunately, I don't think there is good documentation in the wiki for that yet.  The closest thing is probably http://wiki.secondlife.com/wiki/Adding_UI_Hints.  Usually the term "dialog" maps to our concept of "notification" so I'm guessing that might actually be more appropriate in your case.R.On Thu, 18 Oct 2012 07:51:23 -0700, Oz Linden (Scott Lawrence)  wrote:
On 2012-10-18 09:41 , CJ Davies wrote:


  I'm trying to follow the "Adding a dialog" page on the wiki

http://wiki.secondlife.com/wiki/Adding_a_dialog

however I suspect it hasn't been updated to reflect changes to the 
codebase & as such I can't get it to work. In particular,

LLUICtrlFactory::getInstance()->buildFloater(this, "floater_foo.xml");

doesn't compile as there is no buildFloater method in LLUICtrlFactory. 
Can anybody shed some light on how we now go about creating dialogs?




It's possible (likely, even) that there's more than one way, but one
I worked on recently worked this way

The floater is registered in newview/llviewerfloaterreg.cpp:

 LLFloaterReg::add("prefs_autoreplace", "floater_autoreplace.xml", (LLFloaterBuildFunc)&LLFloaterReg::build);




It is then shown by code in newview/llfloaterpreference.cpp:

LLFloaterReg::showInstance("prefs_autoreplace");



The code for that floater is in newview/llfloaterautoreplacesettings.cpp





  ___
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: Put the viewer version into marker files, and report errors only when the version matches

[Richard Nelson]

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

Ship it!


Functionally, it looks good...just one minor comment about structure


indra/newview/llappviewer.cpp
<http://codereview.secondlife.com/r/607/#comment1161>

Structurally, it seems like this would be cleaner if opening the file, 
writing out the marke version, and closing it all happened in one function.  
Otherwise, you have this constraint that the file needs to be opened before 
being passed into recordMarkerVersion and closed afterwards.

Just picking a nit, sorry.


- Richard Nelson


On Nov. 2, 2012, 1:55 p.m., Oz Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/607/
> ---
> 
> (Updated Nov. 2, 2012, 1:55 p.m.)
> 
> 
> Review request for Viewer and Callum Prentice.
> 
> 
> Description
> ---
> 
> In all the marker files used to detect how the viewer run terminates, record 
> the version.  When checking the results, report errors only if the current 
> version matches the version in the file.  This prevents errors in one version 
> from being reported against the subsequent version.
> 
> 
> This addresses bug storm-1850.
> http://jira.secondlife.com/browse/storm-1850
> 
> 
> Diffs
> -
> 
>   indra/newview/llappviewer.h 3d35a13561fc 
>   indra/newview/llappviewer.cpp 3d35a13561fc 
> 
> Diff: http://codereview.secondlife.com/r/607/diff/
> 
> 
> Testing
> ---
> 
> Several simulated crashes both of the modified and unmodified viewers, and 
> some in which the marker file was modified manually to simulate different 
> viewers. Launched the new viewer after different crashes (and normal exits) 
> and confirmed (using logging temporarily added for that purpose) that the 
> reported last exec event was correct - and is always reported as Normal if 
> the previous version and the running version were not the same.
> 
> 
> Thanks,
> 
> Oz Linden
> 
>

___
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: Put the viewer version into marker files, and report errors only when the version matches

[Richard Nelson]

> On Nov. 9, 2012, 2:05 p.m., Richard Nelson wrote:
> > indra/newview/llappviewer.cpp, line 3376
> > <http://codereview.secondlife.com/r/607/diff/1/?file=8087#file8087line3376>
> >
> > Structurally, it seems like this would be cleaner if opening the file, 
> > writing out the marke version, and closing it all happened in one function. 
> >  Otherwise, you have this constraint that the file needs to be opened 
> > before being passed into recordMarkerVersion and closed afterwards.
> > 
> > Just picking a nit, sorry.
> 
> Oz Linden wrote:
> I didn't do that because the usage of the two marker files is so 
> different: the exec marker file is locked, but the logout marker file is not; 
> and there is at least one code path in which the logout marker file is 
> (apparently deliberately) not closed at all.
>

Hmmm, ok.  Fair enough.


- Richard


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


On Nov. 2, 2012, 1:55 p.m., Oz Linden wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/607/
> ---
> 
> (Updated Nov. 2, 2012, 1:55 p.m.)
> 
> 
> Review request for Viewer and Callum Prentice.
> 
> 
> Description
> ---
> 
> In all the marker files used to detect how the viewer run terminates, record 
> the version.  When checking the results, report errors only if the current 
> version matches the version in the file.  This prevents errors in one version 
> from being reported against the subsequent version.
> 
> 
> This addresses bug storm-1850.
> http://jira.secondlife.com/browse/storm-1850
> 
> 
> Diffs
> -
> 
>   indra/newview/llappviewer.h 3d35a13561fc 
>   indra/newview/llappviewer.cpp 3d35a13561fc 
> 
> Diff: http://codereview.secondlife.com/r/607/diff/
> 
> 
> Testing
> ---
> 
> Several simulated crashes both of the modified and unmodified viewers, and 
> some in which the marker file was modified manually to simulate different 
> viewers. Launched the new viewer after different crashes (and normal exits) 
> and confirmed (using logging temporarily added for that purpose) that the 
> reported last exec event was correct - and is always reported as Normal if 
> the previous version and the running version were not the same.
> 
> 
> Thanks,
> 
> Oz Linden
> 
>

___
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] XML schema

[Richard Nelson]

That wiki page relies on being manually kept up to date, which is not something we've been doing.  While there is currently no official schema for XUI files, that will change in the future.  The parameter blocks as defined in code are theoretically able to generate their own schemata at runtime.  There is still a bit of work remaining to finish this and since it is relatively low priority, I can't promise a date when we'll have it done.  But rest assured, we recognize the need.R.On Thu, 15 Nov 2012 05:56:14 -0800, Nicky Perian  wrote:I am attempting to move some v1 based xml files to work with viewer 3. As is usual when starting something new my reach is short of what I need to grasp.I will narrow this down, in v1 this attribute   is present and processed in imprudence but, fails in v3 based kokua.It is not listed in https://wiki.secondlife.com/wiki/Skinning_HowTo/Common_XUI_attributes.How is the XML schema delivered to the viewer?Is there a source of additional attributes that can be referenced or added to v3 viewers? ___
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] Question about BUG-41029 and 64 bit usage

[Richard Nelson]

FWIW, those particular units types were introduced as part of the LLTrace  
metrics update and found several cases where the incorrect units were  
being recorded, resulting in skewed/invalid metrics.  The point is not  
that it is hard to multiply by a constant to do unit conversion...it is  
that programmers sometimes forget to, so it is hard to know how much you  
can trust metrics that have not been thoroughly vetted in the code.

On Fri, 16 Dec 2016 00:44:51 -0800, Henri Beauchamp  wrote:

> On Thu, 15 Dec 2016 11:13:35 -0800, Callum Prentice (Callum) wrote:
>
>> https://jira.secondlife.com/browse/BUG-41029
>>
>> .../...
>>
>> There is a lot usage of 32 bit types (U32Bytes, U32Megabytes etc.)  
>> defined
>> indirectly here:
>>
>> https://bitbucket.org/lindenlab/viewer64/src/9270caf3d4324f9c1c33aa158f80e0fe84036a48/indra/llcommon/llunittype.h?at=default&fileviewer=file-view-default#llunittype.h-824
>>
>> that are used to count memory sizes/usage/difference etc.  I think we  
>> can
>> convert them to their U64 equivalents for all viewers.
>>
>> Nat points out, rewriting this code using size_t as a return type would
>> make more sense but that seems like it would involve more invasive code
>> changes including changes in fundamental LL headers.
>>
>> What does the collective wisdom say?
>
> My *personal* wisdom says that this U32Bytes/U32Megabytes templatized
> thingy is just the expression of the lazyness of its coder (frankly,
> is it *that* difficult to use a constant multiplier to convert from
> bytes to megabytes ?) and just adds more complexity to the generated
> code without any benefit whatsoever.
>
> As a result, I did not backport this non-sense to the Cool VL Viewer
> and kept everything in normal variables (U32 for megabytes, U64 for
> bytes, etc), using constants to convert from one unit to the other
> when needed; my viewer is therefore unafffected by that bug...
>
> My advice would therefore be to revert the corresponding commit.
>
> Henri.
> ___
> 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


-- 
Festina Lente
___
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] How is the XUI part of the interface designed?

[Richard Nelson]

Unfortunately there is no GUI editor for UI layout.  We currently  
hand-edit XML.  A GUI editor is something I've wanted to do for a while,  
and the framework is mostly in place.  But it is still a pretty large task  
to undertake.

R.

On Wed, 06 Oct 2010 17:02:26 -0700, Robert Martin   
wrote:

> On Wed, Oct 6, 2010 at 7:50 PM, Rob Nelson  
>  wrote:
>> There's a GUI editor, but last I checked, only LL employees and a  
>> select few
>> really know how to use it;  The manual is still in some LL wiki  
>> somewhere.
>>
>> Rob
>
> oh really could Some Linden please put both on the public wiki  
> somewhere??
>


-- 
Using Opera's revolutionary email client: http://www.opera.com/mail/
___
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] opensource-dev Digest, Vol 9, Issue 27

[Richard Nelson]

Just create a combo_box like this:

  
 
 
 
 
 
   

and put it in the proper spot in floater_tools.xml and everything should  
just work.

FWIW, I recommend doing development of XUI in your OS-equivalent  
Application Data directory:

on Windows 7, for example, copy floater_tools.xml to  
c:\users\your_account\application  
data\roaming\secondlife\skins\default\xui\en\ and edit in place.

Enjoy.

R.


On Wed, 06 Oct 2010 20:25:30 -0700, Kent Quirk (Q Linden)  
 wrote:

> That sort of thing is more involved, I'm afraid; you have to change the  
> control. I don't know offhand if the XUI commit binding mechanism can be  
> made to work with only XUI change -- I kind of doubt it. If not, you'll  
> have to  change the c++ event handlers from 4 individual event handlers  
> to one that does all 4 things on change of the combo_box.
>
> It's probably worth some spelunking through XUI to see if there's an  
> example of binding the combo box; if not, I bet one of the devs around  
> here would help you write the C++ changes to make it work.
>
> Q
>
> On Oct 6, 2010, at 11:04 PM, SuezanneC Baskerville wrote:
>
>> Thanks, Kent, I'm glad I asked.
>>
>> The keystroke, by the way,  appears to be Control Shift T, not Control  
>> Alt T,  on my XP system running the Development Viewer.
>>
>> I'd like to replace the Focus, Move, Edit, Create, and Land buttons  
>> with a list, I suppose a combo_box, to save horizontal space.  How does  
>> one do that?
>>
>>
>> --
>>
>> Message: 7
>> Date: Wed, 6 Oct 2010 22:30:51 -0400
>> From: "Kent Quirk (Q Linden)" 
>> Subject: Re: [opensource-dev] How is the XUI part of the interface
>>designed?
>>
>> There's no GUI editor. However, there is a nice little assistance tool.  
>> From the login screen ONLY, you can invoke the GUI Preview tool by  
>> hitting alt-ctrl-T (or cmd-ctrl-T on a Mac). It will bring up a dialog  
>> that you can use to display any of our dialog boxes, in two languages  
>> at once if you want. If you configure it, you can also have it open the  
>> dialog file in your text editor, and the "show rectangles" checkbox  
>> makes it easy to see which parts of the dialog have which names.
>>
>> If you edit a dialog, you can show and hide it, and it will reload. So  
>> you should be able to experiment with a layout rather easily; it's not  
>> a live graphical editor, but it makes it pretty easy to edit. Just  
>> realize the dialogs are not "live", and so sometimes things won't look  
>> right, or may have overlapping fields because only one of them is  
>> visible at any given time.
>>
>>Q
>>
>> ___
>> 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
>


-- 
Using Opera's revolutionary email client: http://www.opera.com/mail/
___
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