[opensource-dev] Review Request: VWR-25923 Unnecessary capability request spam

[Armin Weatherwax]

I agree very much about extensive testing of changes in 
key-functionality like voice. Lots of teleporting is the right thing to 
do for this patch (see the jira for more a more detailed list of things 
that I think need to be adressed).
About Inventory failures and being disconnected from the sim, just to 
repeat here what we already where talking about in irc:  neither 
iteration of the patch does anything inventory or disconnecting from 
the sim. To check the "you never know" factor I tried the build from a) 
my viewer-development branch with patch iteration 2 applied and b) 
Imprudence 1.3.2 with neither of both iterations applied. I got 1 time 
disconnected with a) , 2 times almost and 1 time disconnected with b). 
That all looks very much like a simulator - or network issue close to 
the sim to me. However good to address that, too, since for proper 
testing one needs to be able to make a distinction if a patch is 
causing a failure or the current state of the service.

Armin

jaeger_Reg at hotmail.com wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/333/#review819
> -----------------------------------------------------------
>
>
> When I applied this latest patch to firestorm, which is based off of
> LL 2.5.2, I kept getting timed out when TPing from a mainland area
> and a DD of 256 to other regions.  I tried multiple times TPing from
> the area around help island public to magnum sandbox 4 or to help
> people island.  In every case I timed and was logged off after the TP
> was completed, as the viewer was cleaning up and closing connections
> to the old regions.  I don't have this problem with the first
> iteration of this patch.  I feel this scenario needs to be tested
> with this latest patch on v-dev tip to make sure this doesn't happen
> there as well.
>
> The account I used only has 4 friends, and around 24 items in the
> inventory in addition to the library.  It also only has 2 attachments
> (a hud and the firestorm bridge) and is one of the noob outfits.  I
> also am running on an i7 2600k, 16GB ram, ATI 6970, and 20/5mbit FIOS
> connection.
>
> - tankmaster.finesmith
>
> On June 24, 2011, 4:45 a.m., Jonathan Yap wrote:
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > http://codereview.secondlife.com/r/333/
> > -----------------------------------------------------------
> >
> > (Updated June 24, 2011, 4:45 a.m.)
> >
> >
> > Review request for Viewer.
> >
> >
> > Summary
> > -------
> >
> > This is a patch by ArminWeatherHax.  I am creating the request to
> > help speed this fix along in the system.
> >
> > ----
> >
> > Ways to reproduce: log into a simulator.
> > Reproduces: always
> > Affects: any version supported, probably all 3rd party viewers but
> > Kokua (and Imprudence, soon).
> >
> > What happens:
> > In each idle cycle the voice client requests the
> > "ParcelVoiceInfoRequest" capability, thats each time a HTTP GET.
> > See LLVivoxVoiceClient::stateMachine() after comment // Check for
> > parcel boundary crossing
> >
> > Expected:
> > On parcel/region change request the capability once. It's not the
> > region that rezzes in, but the avatar, so do the request for the
> > capability not earlier than the agents region signals
> > capabilitiesReceived() true. After that you are sure if the region
> > returns an empty url you can give up for that region.
> >
> > Not sure about the impact on lag - requesting and returning an url
> > is not much data transmitted, though its a pretty big number of
> > people doing it over and over per second (no matter if they have
> > voice on or off).
> >
> >
> > ----
> >
> > going once again through llviewerregion I see its fortunately not
> > each time a HTTP GET, just once when the agent connects to the
> > region. Though the patch still saves all the lookup if the cap is
> > there while it can't be possibly.
> >
> >
> > This addresses bug VWR-25923.
> >     http://jira.secondlife.com/browse/VWR-25923
> >
> >
> > Diffs
> > -----
> >
> >   doc/contributions.txt 04e2a3ddca51
> >   indra/newview/llviewerregion.h 04e2a3ddca51
> >   indra/newview/llviewerregion.cpp 04e2a3ddca51
> >   indra/newview/llvoicevivox.h 04e2a3ddca51
> >   indra/newview/llvoicevivox.cpp 04e2a3ddca51
> >
> > Diff: http://codereview.secondlife.com/r/333/diff
> >
> >
> > Testing
> > -------
> >
> >
> > Thanks,
> >
> > Jonathan