Re: [opensource-dev] Fix for "Attachments displayed in mouselook" bug

2010-10-09 Thread Ponzu 
Putting in now.

Funny thing is I *like* seeing my hair and glasses when I am in Mouselook.
it makes me feel like I am really looking out of my avatars eyes.

Maybe it needs to be an option 8-)


On Fri, Oct 8, 2010 at 10:31 PM, Philippe (Merov) Bossut <
me...@lindenlab.com> wrote:

> return (type != 0) && mRenderTypeEnabled[type];
___
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] Fix for "Attachments displayed in mouselook" bug

2010-10-09 Thread Argent 
I don't normally gripe about stuff like this, but somehow this one
triggered my twitches from 20 years of supporting PhD programmers.
Brilliant guys, but sometimes it's SO hard to figure out what they're
trying to do.

This is a case where the trinary "?:" operator is much more readable
and understandable.

(type == 0) ? FALSE : mRenderTypeEnabled[type];

But even better:

if(type == 0)
  return FALSE; // explain why here .. eg "in this context we are
always rendering attached prims on the head when blah blah..."
else
  return mRenderTypeEnabled[type];
___
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] Fix for "Attachments displayed in mouselook" bug

2010-10-09 Thread Zabb65 
I tend to agree with Argent on this, it should have a comment if there
is a hack in place to make it all function.

I ran it through hg blame and it claims that revision 11977 was when a
big block in pipeline.cpp changed

11977: BOOL LLPipeline::hasRenderType(const U32 type) const
11977: {
11977:  return mRenderTypeEnabled[type];
11977: }

previously this function was in a header and looked like

BOOL hasRenderType(const U32 type) const{ 
return (type &&
(mRenderTypeMask & (1 I don't normally gripe about stuff like this, but somehow this one
> triggered my twitches from 20 years of supporting PhD programmers.
> Brilliant guys, but sometimes it's SO hard to figure out what they're
> trying to do.
>
> This is a case where the trinary "?:" operator is much more readable
> and understandable.
>
> (type == 0) ? FALSE : mRenderTypeEnabled[type];
>
> But even better:
>
> if(type == 0)
>  return FALSE; // explain why here .. eg "in this context we are
> always rendering attached prims on the head when blah blah..."
> else
>  return mRenderTypeEnabled[type];
> ___
> 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] Fix for "Attachments displayed in mouselook" bug

2010-10-09 Thread Boroondas Gupte 
 On 10/09/2010 08:59 PM, Zabb65 wrote:
> I ran it through hg blame and it claims that revision 11977 was when a
> big block in pipeline.cpp changed
> [...]
> http://hg.secondlife.com/viewer-development/changeset/c09f9bcd9d20
> however does not show this, and I suspect that it is heavily truncated
> on bitbucket, even though the full patch can be pulled from the hg
> repository itself.
I can't see it in the raw changeset, either. Probably |hg blame| got
confused by the huge merges and them being backed out and un-backed out
('backed in'?) again. The revision where this really changed seems to be
83606bbdc084
.
Found via hg bisect, like this:

hg bisect -g 0

hg bisect -b 467c72e14d6e

hg bisect -c'grep "(mRenderTypeMask & (1< I tend to agree with Argent on this, it should have a comment if there
> is a hack in place to make it all function.
> [...]
> Comments for hacks like this are a good thing. Leaving them only in
> commit messages leaves them to be lost and forgotten and then broken
> by whoever browses the code.
Yeah, this is definitely something that needs in-code documentation.

> On Sat, Oct 9, 2010 at 12:19, Argent  wrote:
>> I don't normally gripe about stuff like this, but somehow this one
>> triggered my twitches from 20 years of supporting PhD programmers.
>> Brilliant guys, but sometimes it's SO hard to figure out what they're
>> trying to do.
>>
>> This is a case where the trinary "?:" operator is much more readable
>> and understandable.
>>
>> (type == 0) ? FALSE : mRenderTypeEnabled[type];
>>
>> But even better:
>>
>> if(type == 0)
>>  return FALSE; // explain why here .. eg "in this context we are
>> always rendering attached prims on the head when blah blah..."
>> else
>>  return mRenderTypeEnabled[type];
>>
Maybe it's because I'm in academia myself, but I don't think these are
significantly more readable than Merov's line. The real issue is IMHO
the lack of comments. Of course, the original implementation

was rather confusing. Implicit casting to bool is commonly used for null
pointer checks, but (without wanting to blame anyone) can probably
safely be declared bad style for non-pointer variables, comments being
present or not.

Maybe a stupid question, but why do we need special casing here at all?
Can't we just set |mRenderTypeEnabled[0]| to |FALSE| at an appropriate
place? (Of course with a comment there explaining why we do that.)

Cheers,
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] Fix for "Attachments displayed in mouselook" bug

2010-10-09 Thread Ponzu 
Would it be better to just store FALSE in mRenderTypeEnabled[0]?

On Sat, Oct 9, 2010 at 12:19 PM, Argent  wrote:

> I don't normally gripe about stuff like this, but somehow this one
> triggered my twitches from 20 years of supporting PhD programmers.
> Brilliant guys, but sometimes it's SO hard to figure out what they're
> trying to do.
>
> This is a case where the trinary "?:" operator is much more readable
> and understandable.
>
> (type == 0) ? FALSE : mRenderTypeEnabled[type];
>
> But even better:
>
> if(type == 0)
>  return FALSE; // explain why here .. eg "in this context we are
> always rendering attached prims on the head when blah blah..."
> else
>  return mRenderTypeEnabled[type];
> ___
> 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] Fix for "Attachments displayed in mouselook" bug

2010-10-09 Thread Dave Booth 
  On 10/9/2010 10:07, Ponzu wrote:
> Photo attached, from behind my eyes 8-)
>
>
>
>
> ___
> 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
looks like you wear the same glasses I do :)
___
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] Fix for "Attachments displayed in mouselook" bug

2010-10-09 Thread Tateru Nino 
 Well, that depends. That element might be used for something else, 
elsewhere.


If the _intended_ behaviour of the function (and of the array/container) 
is documented, then it's relatively easy to make decisions about how 
best to implement that, clearly, concisely and efficiently - or at least 
to argue it back and forth :)


All too many times, we've gone too far down the wrong rabbit-hole 
because we've tried to treat the code as the documentation when the code 
doesn't necessarily express what it is actually intended to do.



On 10/10/2010 1:57 PM, Ponzu wrote:

Would it be better to just store FALSE in mRenderTypeEnabled[0]?

On Sat, Oct 9, 2010 at 12:19 PM, Argent > wrote:


I don't normally gripe about stuff like this, but somehow this one
triggered my twitches from 20 years of supporting PhD programmers.
Brilliant guys, but sometimes it's SO hard to figure out what they're
trying to do.

This is a case where the trinary "?:" operator is much more readable
and understandable.

(type == 0) ? FALSE : mRenderTypeEnabled[type];

But even better:

if(type == 0)
 return FALSE; // explain why here .. eg "in this context we are
always rendering attached prims on the head when blah blah..."
else
 return mRenderTypeEnabled[type];
___
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


--
Tateru Nino
http://dwellonit.taterunino.net/

___
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