Re: [opensource-dev] Review Request: STORM-1001: Viewer needlessly hits the "ObjectMedia" cap with thousands of requests

2011-03-02 Thread Kelly Washington 

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

Ship it!


This change looks good to me. It looks cleaner and more correct.

- Kelly


On Feb. 22, 2011, 11:12 a.m., Kitty Barnett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/162/
> ---
> 
> (Updated Feb. 22, 2011, 11:12 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> S32 LLTextureEntry::setMediaTexGen(U8 media) would appear to be the root 
> cause of the bug:
> 
> The header file suggests that:
> 
> // The Media Tex Gen values are bits in a bit field:
> // +--+
> // | .TTM | M = Media Flags (web page), T = LLTextureEntry::eTexGen, . = 
> unused
> // | 76543210 |
> // +--+
> const S32 TEM_MEDIA_MASK  = 0x01;
> const S32 TEM_TEX_GEN_MASK= 0x06;
> 
> and while LLTextureEntry::setTexGen() and LLTextureEntry::setMediaFlags() 
> each properly mask off the supplied parameter with their respective bit mask, 
> setMediaTexGen() will always return TEM_CHANGE_MEDIA even if only texgen has 
> changed while the media flag hasn't (causing 
> LLVOVolume::processUpdateMessage() to queue a request to the cap when it 
> shouldn't).
> 
> Changing it to:
> 
> S32 LLTextureEntry::setMediaTexGen(U8 media)
> {
>   S32 result = setTexGen(media & TEM_TEX_GEN_MASK);
>   result |= setMediaFlags(media & TEM_MEDIA_MASK);
>   return result;
> }
> 
> appears to resolve the issue completely (the cap isn't hit unless an object 
> nearby has media on it, or changes its URL)
> 
> 
> This addresses bug STORM-1001.
> http://jira.secondlife.com/browse/STORM-1001
> 
> 
> Diffs
> -
> 
>   indra/llprimitive/lltextureentry.cpp UNKNOWN 
> 
> Diff: http://codereview.secondlife.com/r/162/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kitty
> 
>

___
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-1001: Viewer needlessly hits the "ObjectMedia" cap with thousands of requests

2011-03-02 Thread Kelly Washington 


> On March 1, 2011, 10:22 p.m., Merov Linden wrote:
> > indra/llprimitive/lltextureentry.cpp, line 433
> > 
> >
> > Looks like you need to add creation/deletion of mMediaEntry if the 
> > TEM_MEDIA_MASK bit flipped.

That is covered in LLTextureEntry::setMediaFlags.


- Kelly


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


On Feb. 22, 2011, 11:12 a.m., Kitty Barnett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/162/
> ---
> 
> (Updated Feb. 22, 2011, 11:12 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> S32 LLTextureEntry::setMediaTexGen(U8 media) would appear to be the root 
> cause of the bug:
> 
> The header file suggests that:
> 
> // The Media Tex Gen values are bits in a bit field:
> // +--+
> // | .TTM | M = Media Flags (web page), T = LLTextureEntry::eTexGen, . = 
> unused
> // | 76543210 |
> // +--+
> const S32 TEM_MEDIA_MASK  = 0x01;
> const S32 TEM_TEX_GEN_MASK= 0x06;
> 
> and while LLTextureEntry::setTexGen() and LLTextureEntry::setMediaFlags() 
> each properly mask off the supplied parameter with their respective bit mask, 
> setMediaTexGen() will always return TEM_CHANGE_MEDIA even if only texgen has 
> changed while the media flag hasn't (causing 
> LLVOVolume::processUpdateMessage() to queue a request to the cap when it 
> shouldn't).
> 
> Changing it to:
> 
> S32 LLTextureEntry::setMediaTexGen(U8 media)
> {
>   S32 result = setTexGen(media & TEM_TEX_GEN_MASK);
>   result |= setMediaFlags(media & TEM_MEDIA_MASK);
>   return result;
> }
> 
> appears to resolve the issue completely (the cap isn't hit unless an object 
> nearby has media on it, or changes its URL)
> 
> 
> This addresses bug STORM-1001.
> http://jira.secondlife.com/browse/STORM-1001
> 
> 
> Diffs
> -
> 
>   indra/llprimitive/lltextureentry.cpp UNKNOWN 
> 
> Diff: http://codereview.secondlife.com/r/162/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kitty
> 
>

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

[opensource-dev] Review Request: STORM-1044 Improved message template checking.

2011-03-03 Thread Kelly Washington 

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

Review request for Viewer.


Summary
---

Improved message template checking.
* If no changes have been made to the message template the check is local only 
and trivial.
* If changes have been made to the message template then compatibility is 
verified on the first build after the change instead of when they package.
* Automated systems can use --force to force full message template verification


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


Diffs
-

  indra/cmake/TemplateCheck.cmake 3683d7c533f9 
  indra/newview/CMakeLists.txt 3683d7c533f9 
  scripts/messages/message_template.msg.sha1 PRE-CREATION 
  scripts/template_verifier.py 3683d7c533f9 

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


Testing
---

* Built the viewer and verified only the hash is checked.
* Changed the template and verified full verification happens and a new sha1 
saved.


Thanks,

Kelly

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

[opensource-dev] Review Request: VWR-25261 URLs in Top Scripts is always 0.

2011-03-23 Thread Kelly Washington 

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

Review request for Viewer.


Summary
---

Fix the top scripts floater to pull the number of URLs from the correct message 
block.


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


Diffs
-

  indra/newview/llfloatertopobjects.cpp 6de4b45ab61f 

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


Testing
---

* Created an object with a script that requested 9 urls.
* Viewed Region/Estate:Debug:Top Scripts
* Verified the object showed on the list as using 9 urls. (sorted by the urls 
column.)


Thanks,

Kelly

___
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: Allow scripts to be saved/loaded to/from files.

2011-11-18 Thread Kelly Washington 


> On Nov. 18, 2011, 6:06 a.m., Lance Corrimal wrote:
> > Built it into dolphin 3.2 beta, tested it. found a few issues:
> > - main menu option Build/Upload/Script does nothing at all
> > - saving a script that you open from your inventory to disk only works 
> > after you edit the script
> > - saving a script to disk does not add .lsl to the filename by default
> >
> 
> Ima Mechanique wrote:
> Thanks for the feedback.
> - The Build => Upload => Script entry shouldn't be there, I'll correct 
> the code. It's a follow up project, which I haven't completed yet.
> - Saving only works after editing. This is normal behaviour for scripts, 
> I was doubtful of doing it this way, but did so to maintain consistency. If 
> there is a consensus to alter this behaviour I would happily agree to do so.
> - Hmm, .lsl should be added by default, I'll look into this.

Unless opening the script from file automatically saves (uploads and compiles) 
the script then it makes more sense for the save button to be enabled as soon 
as the script is loaded. No need to wait for an edit to enable the save.


- Kelly


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


On Nov. 17, 2011, 3:08 p.m., Ima Mechanique wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/516/
> ---
> 
> (Updated Nov. 17, 2011, 3:08 p.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> ---
> 
> Changes to allow opened script window to save/load to/from files on the users 
> computer.
> 
> 
> This addresses bug https://jira.secondlife.com/browse/storm-1708.
> 
> http://jira.secondlife.com/browse/https://jira.secondlife.com/browse/storm-1708
> 
> 
> Diffs
> -
> 
>   indra/newview/llfilepicker.h a1319d553db9 
>   indra/newview/llfilepicker.cpp a1319d553db9 
>   indra/newview/llfloaternamedesc.h a1319d553db9 
>   indra/newview/llfloaternamedesc.cpp a1319d553db9 
>   indra/newview/llpreviewscript.h a1319d553db9 
>   indra/newview/llpreviewscript.cpp a1319d553db9 
>   indra/newview/llviewerfloaterreg.cpp a1319d553db9 
>   indra/newview/llviewermenufile.cpp a1319d553db9 
>   indra/newview/skins/default/xui/en/menu_viewer.xml a1319d553db9 
>   indra/newview/skins/default/xui/en/panel_script_ed.xml a1319d553db9 
> 
> Diff: http://codereview.secondlife.com/r/516/diff
> 
> 
> Testing
> ---
> 
> Successfully opened and saved scripts from/to local files.
> 
> 
> Thanks,
> 
> Ima
> 
>

___
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: BUG-59: Go-to line function for the internal LSL script editor

2012-09-14 Thread Kelly Washington 

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



indra/newview/llpreviewscript.cpp
<http://codereview.secondlife.com/r/596/#comment1151>

Please put new classes in a new file. (Don't mimic my bad code from years 
ago, please. :) )



indra/newview/llpreviewscript.cpp
<http://codereview.secondlife.com/r/596/#comment1152>

Need null checks on mEditorCore and mEditor



indra/newview/llpreviewscript.cpp
<http://codereview.secondlife.com/r/596/#comment1153>

Need null checks on mEditorCore, mEditor and sInstance



indra/newview/llpreviewscript.cpp
<http://codereview.secondlife.com/r/596/#comment1154>

minor note: This comment is backwards from the code - you describe the 
unwritten else case instead of the implemented if case. It confused me at 
first. Something like '// only deselect, setfocus and close if the cursor 
actually moved' would match the code written, or moving the comment to after 
the if block with something like '// else do nothing '



indra/newview/llpreviewscript.cpp
<http://codereview.secondlife.com/r/596/#comment1155>

Need null pointer checks on mEditorCore and mEditor.


- Kelly Washington


On Sept. 13, 2012, 8:38 a.m., MartinRJ Fayray wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/596/
> ---
> 
> (Updated Sept. 13, 2012, 8:38 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Description
> ---
> 
> Repository is here: https://bitbucket.org/MartinRJ/bug-59
> 
> I more or less cloned the search-function and modified the floater via xml 
> (new file: floater_goto_line.xml) to fit for a 'go to line' floater.
> Also I added a callback to prevalidate the input in the 'go to line' 
> line-editor, so that only numbers can be entered into it.
> 
> 
> This addresses bug BUG-59.
> https://jira.secondlife.com/browse/BUG-59
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 7a056d7afeb2 
>   indra/newview/llpreviewscript.h 7a056d7afeb2 
>   indra/newview/llpreviewscript.cpp 7a056d7afeb2 
>   indra/newview/skins/default/xui/en/floater_goto_line.xml PRE-CREATION 
>   indra/newview/skins/default/xui/en/panel_script_ed.xml 7a056d7afeb2 
> 
> Diff: http://codereview.secondlife.com/r/596/diff/
> 
> 
> Testing
> ---
> 
> Tested on my local PC with a modded "3.3.4-release3" version.
> Open a script with at least 300 lines inside inventory, go to menu 'Edit-> go 
> to line', enter a number less than 300 into the 'Go to' - field. Expected 
> result: the cursor jumps to the entered line number (column 0).
> Repeat the test with a script inside an object's content.
> 
> 
> Thanks,
> 
> MartinRJ Fayray
> 
>

___
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-1911: Go-to line function for the internal LSL script editor

2012-09-19 Thread Kelly Washington 

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

Ship it!


Ship It!

- Kelly Washington


On Sept. 19, 2012, 7:26 a.m., MartinRJ Fayray wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/596/
> ---
> 
> (Updated Sept. 19, 2012, 7:26 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Description
> ---
> 
> Repository is here: https://bitbucket.org/MartinRJ/storm-1911
> 
> I more or less cloned the search-function and modified the floater via xml 
> (new file: floater_goto_line.xml) to fit for a 'go to line' floater.
> Also I added a callback to prevalidate the input in the 'go to line' 
> line-editor, so that only numbers can be entered into it.
> 
> 
> This addresses bug STORM-1911.
> https://jira.secondlife.com/browse/STORM-1911
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt 7a056d7afeb2 
>   indra/newview/CMakeLists.txt 7a056d7afeb2 
>   indra/newview/llfloatergotoline.h PRE-CREATION 
>   indra/newview/llfloatergotoline.cpp PRE-CREATION 
>   indra/newview/llpreviewscript.h 7a056d7afeb2 
>   indra/newview/llpreviewscript.cpp 7a056d7afeb2 
>   indra/newview/skins/default/xui/en/floater_goto_line.xml PRE-CREATION 
>   indra/newview/skins/default/xui/en/panel_script_ed.xml 7a056d7afeb2 
> 
> Diff: http://codereview.secondlife.com/r/596/diff/
> 
> 
> Testing
> ---
> 
> Tested on my local PC with a modded "3.3.4-release3" version.
> Open a script with at least 300 lines inside inventory, go to menu 'Edit-> go 
> to line', enter a number less than 300 into the 'Go to' - field. Expected 
> result: the cursor jumps to the entered line number (column 0).
> Repeat the test with a script inside an object's content.
> 
> 
> Thanks,
> 
> MartinRJ Fayray
> 
>

___
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