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

[Boroondas Gupte]

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


(The first 4 comments aren't about your change, but about pre-existing code. Feel free to ignore them. Please do take note of the last 4 comments of this review which do concern your change.)


indra/llmath/llvolume.h
<http://codereview.secondlife.com/r/317/#comment932>

    Why are the primitive shapes in decimal notation, but the masks for them in hexadecimal notation? That's confusing.



indra/llprimitive/llprimitive.cpp
<http://codereview.secondlife.com/r/317/#comment931>

    Could be re-written as a switch-case structure. (Not that that would be any shorter, as we'd need break;s everywhere.)



indra/newview/llfloatertools.cpp
<http://codereview.secondlife.com/r/317/#comment933>

    An empty line wouldn't hurt here.



indra/newview/lltoolplacer.cpp
<http://codereview.secondlife.com/r/317/#comment934>

    Why are we mentioning all these cases when we just let them fall through to the default case?
    
    This could be handled like https://codereview.secondlife.com/r/231/diff/1-2/#chunk1.14 , i.e. make the list of valid cases exhaustive, move their common action (incl. the break;) before the default: label and output a warning if we reach the default case.



indra/newview/lltoolplacer.cpp
<http://codereview.secondlife.com/r/317/#comment935>

    Remove the empty line.



indra/newview/lltoolplacer.cpp
<http://codereview.secondlife.com/r/317/#comment936>

    Where are these numbers coming from? Are they used elsewhere in the code?



indra/newview/llviewerobjectlist.h
<http://codereview.secondlife.com/r/317/#comment937>

    Please re-introduce the empty line before
    // Inlines



indra/newview/llviewerobjectlist.cpp
<http://codereview.secondlife.com/r/317/#comment938>

    Is a global variable really the way to go here? Also, please add a short comment explaining the semantics of this variable.


- Boroondas


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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110720/eb0c8acf/attachment-0001.htm