[opensource-dev] too bad that list posts can't be "sticky".
Could someone please link to this somewhere prominently on the snowstorm webpages? Just as a reminder of sorts? http://xkcd.com/844/ bye, LC ___ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
Re: [opensource-dev] Review Request: STORM-830 Volume slider isn't properly remembered if set to zero
> On Jan. 6, 2011, 5:37 p.m., Aleric Inglewood wrote: > > This is really not how you want to deal with this bug :/. It's a known > > fact that audio mixers are very bad with low volumes. Setting a volume to 0 > > (or something really small) can put a very high load on the CPU for the > > audio mixer, which causes severe problems. See > > https://jira.secondlife.com/browse/VWR-14914 > > > > So, on the contrary (as VWR-14914 fixed a horrible bug that made FPS drop > > drastically): when the volume is set to 0 (or even close to zero) the audio > > channel has to be muted and not mixed, ever. Assuming that VWR-14914 is in > > Viewer 2, and wasn't broken in the meantime, a volume of 0 would cause the > > channel to be muted, but setting it to 0.01 will not cause it to be > > muted, but result in a high CPU load. > > After discussion on IRC Jonathan explained to me what this is all about. The patch is correct. Nevertheless, I was confused about the fact that this value of 0.01 is never going to be USED. Perhaps a different value and comment can avoid that in the future when other coders look at it. mInternalGain is only ever compared with, and the whole point of this patch is to make sure that the first comparison fails (I now understand). A common value used to make sure that a first comparison fails is a value outside the valid range of that variable. The valid range being [0, 1], I'd suggest using -1 and adding a comment in the lines of: // Make sure that the first call to setMasterGain will cause setInternalGain to be called. mInternalGain = -1.f; - Aleric --- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/72/#review128 --- On Jan. 6, 2011, 2:37 p.m., Jonathan Yap wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/72/ > --- > > (Updated Jan. 6, 2011, 2:37 p.m.) > > > Review request for Viewer. > > > Summary > --- > > There is an edge case in setMasterGain during startup which prevents > setInternalGain from being called if the master volume setting and > mInternalGain both equal 0. > > Setting mInternalGain to a very low but non-zero value fixes this issue. > > > This addresses bug STORM-830. > http://jira.secondlife.com/browse/STORM-830 > > > Diffs > - > > indra/llaudio/llaudioengine.cpp 6d44f0d85a80 > > Diff: http://codereview.secondlife.com/r/72/diff > > > Testing > --- > > In Preferences / Sound & Media tested: > Buttons > Ambient > Sound Effects > Stream Music > Media > Voice Chat > > > Thanks, > > Jonathan > > ___ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
Re: [opensource-dev] Review Request: (STORM-550) LLDir::getNextFileInDir fails for some complex wildcard combinations
> On Jan. 5, 2011, 11:34 a.m., Oz Linden wrote: > > Just one policy question with this... > > > > This implementation uses llwarns for various errors in the glob expression. > > > > Given that I expect that the glob expression will normally (always?) be > > hard coded (I hope no one will accept a pattern as input from the user and > > pass it on unverified), should these use llerrs so that the error cannot be > > missed (it crashes)? > > > > Other than that, this looks good now. > > Seth ProductEngine wrote: > Not sure about the policy on llerrs but as far as I remember we were > trying not to overuse it, using only for fatal errors. > If someone eventually will accept a pattern as the user input this might > cause unexpected crashes, so I thought warnings would be enough to trace the > error while hard coding the patterns, though if llerrs will work better in > this case warnings could be replaced. I'd prefer to see llerrs here; there's no use case I can imagine to allow someone to manually enter globs. - Kent --- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/32/#review112 --- On Jan. 5, 2011, 8:33 a.m., Seth ProductEngine wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/32/ > --- > > (Updated Jan. 5, 2011, 8:33 a.m.) > > > Review request for Viewer. > > > Summary > --- > > Fixed LLDir unit test which failed for some complex wildcard combinations. > Added a class implementing directory entries iteration with pattern matching > which is used in unit tests instead of LLDir::getNextFileInDir. > > This code has been run on Linux only. It should be tested under other > platforms and more test cases should be provided. For example changing > directory contents while iterating through it. > > > This addresses bug STORM-477. > http://jira.secondlife.com/browse/STORM-477 > > > Diffs > - > > indra/cmake/Boost.cmake 27dae7b01a81 > indra/llvfs/CMakeLists.txt 27dae7b01a81 > indra/llvfs/lldiriterator.h PRE-CREATION > indra/llvfs/lldiriterator.cpp PRE-CREATION > indra/llvfs/tests/lldir_test.cpp 27dae7b01a81 > > Diff: http://codereview.secondlife.com/r/32/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-826 (partial): fix line endings in files that use a mix of CRLF and LF
--- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/70/#review131 --- Still CRLFs in indra/newview/skins/default/xui/en/floater_web_content.xml , which isn't windows specific, I think. Otherwise https://bitbucket.org/oz_linden/storm-826/changeset/6e6d1de23cce looks good. I'm wondering whether we should also fix indentation on lines that are touched by this change anyway. (E.g. floater_web_content.xml mixes spaces and tabs.) - Boroondas On Jan. 6, 2011, 10:20 a.m., Oz Linden wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/70/ > --- > > (Updated Jan. 6, 2011, 10:20 a.m.) > > > Review request for Viewer. > > > Summary > --- > > This is a simple change to correct existing line endings - I scanned all of > viewer-development to identify files that had a mixture of CRLF and LF > endings and converted them to just LF. > > The diff apparently won't show the change in line ending characters see > the repo (in the issue) for the real change. > I expanded the scope of this to also convert some files that were all the > same CRLF endings but did not obviously need to be. I left files that were > clearly windows specific alone on the theory that non-windows users probably > won't need to touch them, and the windows tools might care. > > > This addresses bug storm-826. > http://jira.secondlife.com/browse/storm-826 > > > Diffs > - > > indra/cmake/GetPrerequisites_2_8.cmake 6d44f0d85a80 > indra/cmake/LLAddBuildTest.cmake 6d44f0d85a80 > indra/newview/llfloaterwebcontent.h 6d44f0d85a80 > indra/newview/llfloaterwebcontent.cpp 6d44f0d85a80 > indra/newview/llimview.h 6d44f0d85a80 > indra/newview/llimview.cpp 6d44f0d85a80 > indra/newview/lllogchat.cpp 6d44f0d85a80 > indra/newview/tests/llremoteparcelrequest_test.cpp 6d44f0d85a80 > indra/viewer_components/updater/tests/llupdaterservice_test.cpp > 6d44f0d85a80 > > Diff: http://codereview.secondlife.com/r/70/diff > > > Testing > --- > > > 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-829 Viewer 2 does not parse /me in object Instant Messages
--- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/71/#review132 --- I believe Aleric's comment is accurate. Logic testing for a prefix should be removed from the patch, and the flag should simply always be specified in this case. It is notable that the flag does trigger exactly the same test that is present in the patch (i.e. it's not case sensitive, it replicates prefix testing in several other places in the code base, etc). A more general fix might be to refactor all of the places that do prefix testing, but that wouldn't affect this specific issue. Again, the patch should be reduced to one line that simply adds the desired flag. - Joshua On Jan. 5, 2011, 6:14 p.m., Jonathan Yap wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/71/ > --- > > (Updated Jan. 5, 2011, 6:14 p.m.) > > > Review request for Viewer. > > > Summary > --- > > The "/me" in the lsl code below would be displayed rather than being > translated to a name: > llInstantMessage(llGetOwner(),"/me Hello, Avatar!"); > > > This addresses bug STORM-829. > http://jira.secondlife.com/browse/STORM-829 > > > Diffs > - > > indra/newview/llviewermessage.cpp 845cab866155 > > Diff: http://codereview.secondlife.com/r/71/diff > > > Testing > --- > > > Thanks, > > Jonathan > > ___ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
[opensource-dev] Daily Scrum Summary - Thursday, January 6, 2011
Thursday, January 6, 2011 General Notes -- - Reminder to devs. If a ticket doesn't pass Review and is rejected, it will move back to the To Do column in GH, if you do more work or clarify questions, it should be moved back through In Progress and then to In Review. - MMOTD: Oz, because he's automating the hell out of it. Team Status -- Merov Linden -- *PAST* - STORM-828 : Texture Saving Does Not Work: More testing and looking into it. Problem is intermittent. Suspect server side issue (new cap routing) as we see cap errors when things go wrong. Checked with Monty, read docs. More instrumentation show that the problem may occur even without cap issues. Suspecting caching issues now. It's deep and multi faceted... Discussed and assigned to Bao. *FUTURE* - STORM-828 : Texture Saving Does Not Work: Continue to investigate a bit with Bao. - STORM-745 : Produce comprehensive perf baseline: Complete that work. *IMPEDIMENTS* - None Oz Linden -- *PAST* - Merge Monkey - Office Hour - Reviewing third party lib builds to be moved to bitbucket - Helped XMPP Group Chat publish open source for project viewer - Reposted line endings fix based on review feedback *FUTURE* - Merge Monkey - Investigate Mercurial hooks for doing commit/pull checks - Look at VWR issues that have patches attached *IMPEDIMENTS* - none Q Linden -- *PAST* - storm-34 - talk with oz about apr stuff - meetings - ooo *FUTURE* - storm-34 - storm-2 - meetings *IMPEDIMENTS* - meetings Esbee Linden -- *PAST* - Meetings, Email - PO reviews - Prepared and sent out instruction and developer viewer build for product council to review - Scheduled 1:1s w/Gez - Reviewed half of the VWRs assigned to PE - Sent feedback to Jonathan on next steps for STORM-615 - Reviewed STORM-236 and decided on next steps *FUTURE* - Meetings - Review Snowstorm Team product backlog and bug log - Work with Rhett on next steps for Viewer icons - PO reviews for open tickets - Talk to Q about Andrew's feedback on STORM-753 (Design for Viewer Saved Layouts) - Finish going through the list of VWRs assigned to PE (finally) - Plan update for system requirements - Look at VWR-18435 and VWR-24357 for Jonathan - Add instructions for STORM-236 for Wolfpup - Write draft for Viewer 2.5 blog post *IMPEDIMENTS* - STORM-702 needs to be moved back through to the Review process so we can do PO and code reviews. Paul ProductEngine -- - OOO - vacation Seth Productengine -- *PAST* - BUG (STORM-447) User is not able to share multi selected objects using drag&drop - WIP. Investigated. Might require a few days long refactoring. Asked Esbee if we should start working at it. - BUG (STORM-514) LLSideTray::Params::Params() gets non-exsistant artwork. - Fixed. *FUTURE* - Pick some issues from bug queue. *IMPEDIMENTS* - None. Andrew Productengine -- *PAST* - Answered Q's comments in STORM-823 (Tab Key not working properly) and STORM-34 (As a User, I want a list of my favorite locations available on the login screen so i can log in to SL right where I want to be). - Normal task STORM-2 (Customizable viewer layouts). - Excluded some floaters that caused crashes on load from saving to layout. Continued fixing the bug with loading undocked sidetray tabs - yesterday's idea of fix was in tight direction but can't be applied directly (it would break dock functionality of floater). Hope to finish it on Monday. *FUTURE* - Normal task STORM-2. Hope to finally finish first pass of feature on Monday. *IMPEDIMENTS* - STORM-2. Waiting for Q's reply to letter about organization of layout saving. - STORM-2. Waiting for Esbee's answers to comments about layout design in STORM-753. - STORM-34. No reply from Q to comment about current implementation of feature. Vadim Productengine -- *PAST* - Bug STORM-226 (Viewer 2 Floating Text aligns improperly): - 
Requested an acceptance criteria. - Bug STORM-393 (Group SLURL has wrong color in the nearby chat): - Fixed. - Bug STORM-428 (Name of blocked person is not presented in Block list if person was added to list from 'Near me' or 'Friends' tabs of Choose resident): - 
Resolved as not reproducible. - Bug STORM-594 (Crash in LLView::handleVisibilityChange): - One more attempt to reproduce and find the reason. No luck so far. - Changed status of some tickets having both approvals from Reviewing to Approved, so that they can be integrated. *FUTURE* - Bug STORM-594 (Cr
Re: [opensource-dev] Review Request: STORM-829 Viewer 2 does not parse /me in object Instant Messages
> On Jan. 7, 2011, 9:36 a.m., Joshua Linden wrote: > > I believe Aleric's comment is accurate. Logic testing for a prefix should > > be removed from the patch, and the flag should simply always be specified > > in this case. > > > > It is notable that the flag does trigger exactly the same test that is > > present in the patch (i.e. it's not case sensitive, it replicates prefix > > testing in several other places in the code base, etc). A more general fix > > might be to refactor all of the places that do prefix testing, but that > > wouldn't affect this specific issue. Again, the patch should be reduced to > > one line that simply adds the desired flag. Joshua, if the check for "/me " and "/me'" were not present and CHAT_STYLE_IRC was always passed in then all llInstantMessages from objects would be treated as if /me had been sent. I don't think this is what is desired. - Jonathan --- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/71/#review132 --- On Jan. 5, 2011, 6:14 p.m., Jonathan Yap wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/71/ > --- > > (Updated Jan. 5, 2011, 6:14 p.m.) > > > Review request for Viewer. > > > Summary > --- > > The "/me" in the lsl code below would be displayed rather than being > translated to a name: > llInstantMessage(llGetOwner(),"/me Hello, Avatar!"); > > > This addresses bug STORM-829. > http://jira.secondlife.com/browse/STORM-829 > > > Diffs > - > > indra/newview/llviewermessage.cpp 845cab866155 > > Diff: http://codereview.secondlife.com/r/71/diff > > > Testing > --- > > > Thanks, > > Jonathan > > ___ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
Re: [opensource-dev] Review Request: STORM-829 Viewer 2 does not parse /me in object Instant Messages
> On Jan. 7, 2011, 9:36 a.m., Joshua Linden wrote: > > I believe Aleric's comment is accurate. Logic testing for a prefix should > > be removed from the patch, and the flag should simply always be specified > > in this case. > > > > It is notable that the flag does trigger exactly the same test that is > > present in the patch (i.e. it's not case sensitive, it replicates prefix > > testing in several other places in the code base, etc). A more general fix > > might be to refactor all of the places that do prefix testing, but that > > wouldn't affect this specific issue. Again, the patch should be reduced to > > one line that simply adds the desired flag. > > Jonathan Yap wrote: > Joshua, if the check for "/me " and "/me'" were not present and > CHAT_STYLE_IRC was always passed in then all llInstantMessages from objects > would be treated as if /me had been sent. I don't think this is what is > desired. @Joshua : Um yes - all of that seemed logical. But the existing code is far from logical, or clean. I just had a discussion with Jonathan on IRC and now understand that the meaning of the CHAT_STYLE_IRC is actually "we found this message to start with "/me", please BLINDLY replace the first 3 characters with the name of the object/avatar". The name of the flag is horribly wrong imho. Also, then I suggested to change the code in the what I had assumed it was: set the flag whenever replacement is necessary and just leave it to the replacement code to check for it. That would therefore require an additional change: make the code that now only tests if the flag is set ALSO test if the message indeed starts with "/me " or "/me'" (and having gotten rid of the code duplication, it then could easily be extended in the future). Unfortunately, not only the testing for "/me " is scattered all over the place, also the code that does the actual replacement of the first 3 characters exists in many places! ... So, without making this a much larger "project", I suppose adding one more duplication of code that checks for "/me " isn't the worst of things, and at least it is an immediate fix for the problem at hand :/. I have no objections anymore if the patch would be used as-is. - Aleric --- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/71/#review132 --- On Jan. 5, 2011, 6:14 p.m., Jonathan Yap wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/71/ > --- > > (Updated Jan. 5, 2011, 6:14 p.m.) > > > Review request for Viewer. > > > Summary > --- > > The "/me" in the lsl code below would be displayed rather than being > translated to a name: > llInstantMessage(llGetOwner(),"/me Hello, Avatar!"); > > > This addresses bug STORM-829. > http://jira.secondlife.com/browse/STORM-829 > > > Diffs > - > > indra/newview/llviewermessage.cpp 845cab866155 > > Diff: http://codereview.secondlife.com/r/71/diff > > > Testing > --- > > > Thanks, > > Jonathan > > ___ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
Re: [opensource-dev] Review Request: STORM-829 Viewer 2 does not parse /me in object Instant Messages
Joshua Linden wrote: > I believe Aleric's comment is accurate. Logic testing for a prefix > should be removed from the patch, and the flag should simply always > be specified in this case. > It is notable that the flag does trigger exactly the same test that > is present in the patch (i.e. it's not case sensitive, it replicates > prefix testing in several other places in the code base, etc). A more > general fix might be to refactor all of the places that do prefix > testing, but that wouldn't affect this specific issue. Again, the > patch should be reduced to one line that simply adds the desired > flag. I'm... wondering why not test *ONCE* when the input is accepted, do the appropriate replacement and set the flag so it gets sent as an action instead of a statement, and be done with it? The IM input, chatbar inputs, and local history tab inputs all need pretty much the same validation, don't they? ___ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
[opensource-dev] Daily Scrum Summary - Friday, January 7, 2011
Friday, January 7, 2011 General Notes -- - Reminder to devs. If a ticket doesn't pass Review and is rejected, it will move back to the To Do column in GH, if you do more work or clarify questions, it should be moved back through In Progress and then to In Review. - MMOTD: Oz - PE is on holiday today - Orthodox Christmas Team Status -- Merov Linden -- *PAST* - STORM-828 : Texture Saving Does Not Work : investigated more with Bao. Issue moved to Bao's backlog. - STORM-150 : Evaluate kakadu new capabilities : Wrote doc, moved to review state, reviewed - STORM-104 : Update KDU to more recent version (v6.4) so we take advantage of decompression perf gain : All sub-tasks done so moved to review state. *FUTURE* - Clean-up all dangling sprint 9 issues assigned to me - STORM-745 : Produce comprehensive perf baseline: Complete that work. *IMPEDIMENTS* - None Oz Linden -- *PAST* - Merge Monkey (tagged 2.5.0-start) - Set up some third party lib bitbucket repos - Code reviews - Looking at old VWR patches - Help triage a TPV issue with SLURLs *FUTURE* - Merge Monkey (handoff to Merov at EOD) - More looking at old VWR patches - Tracking down viewer dependencies, start updating wiki *IMPEDIMENTS* - none Q Linden -- - OOO Esbee Linden -- *PAST* - Meetings, Email - PO reviews - Discussed STORM-34, STORM-2, & STORM-753 w/Q - Reviewed STORM-34,STORM-2, and STORM-753 - Contacted Rhett about STORM-34 - Looped Q into the discussion about Viewer icons and the naming of Developer Viewers - Discussions about Viewer 2.5 beta 1 *FUTURE* - Meetings - Write draft for Viewer 2.5 blog post - Review Snowstorm Team product backlog and bug log - Work with Rhett on next steps for Viewer icons - PO reviews for open tickets - Finish going through the list of VWRs assigned to PE (finally) - Plan update for system requirements - Look at VWR-18435 and VWR-24357 for Jonathan - Add instructions for STORM-236 for Wolfpup *IMPEDIMENTS* - STORM-702 needs to be moved back through to the Review process so we can do PO and code reviews. Paul ProductEngine -- - OOO - holiday Seth Productengine -- *PAST* - BUG (STORM-396) Mouse cursor changes to "+" if try to drop a landmark from object inventory or notecard onto Favorites Bar - Could not reproduce. - BUG (STORM-385) Pressing ENTER key in the Favorites overflow list scrolls list - Fixed. *FUTURE* - BUG (STORM-383) Context menu cannot be open for Landmark that are located in the My inventory->Trash folder *IMPEDIMENTS* Waiting for Esbee's reply on the following issues: - TASK (STORM-797) Parcel SLURL rendering - TASK (STORM-28) As a User, I want the ability to send my calling card to others - BUG (STORM-447) User is not able to share multi selected objects using drag&drop Andrew Productengine -- - OOO - holiday Vadim Productengine -- - OOO - holiday Andrey Productengine -- - OOO - holiday Jonathan Yap -- *PAST* - STORM-830 (Volume slider isn't properly remembered if set to zero) - Fixed, but looking for comments on Review Board. - Long IRC discussion with Aleric this morning on this one. - Wrote VWR-24416 (Allow user to supply/define her own objects in the build floater) - STORM-435 (There is no space between name of object and value of object in Chat step while creating new gesture) - Fixed. - Commented on Review Board entry for STORM-829 (Viewer 2 does not parse /me in object Instant Messages) *FUTURE* - I have other commitments for the next two weeks and will not be around as much. *IMPEDIMENTS* - none - ___ 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
