----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/333/#review814 -----------------------------------------------------------
indra/newview/llvoicevivox.h <http://codereview.secondlife.com/r/333/#comment818> Very good idea to explain the semantics of this field in detail, as the name alone doesn't give it away fully. :-) indra/newview/llvoicevivox.cpp <http://codereview.secondlife.com/r/333/#comment817> These are two sentences. Use a period instead of a comma. indra/newview/llvoicevivox.cpp <http://codereview.secondlife.com/r/333/#comment816> requestParcelInfo() might be a more natural name than using 'do'. indra/newview/llvoicevivox.h <http://codereview.secondlife.com/r/333/#comment821> Not a native speaker myself, but I don't think 'keep s.th. staying in <a state>' is proper English. Maybe either write // Setting mRegionHasVoiceCaps to false makes the voice client stay in stateDisabled. or simply // Setting mRegionHasVoiceCaps to false keeps the voice client in stateDisabled. indra/newview/llvoicevivox.h <http://codereview.secondlife.com/r/333/#comment820> This sentence confused me for some reason. I guess 'allows to leave' would be clearer than 'allows escaping'. indra/newview/llvoicevivox.cpp <http://codereview.secondlife.com/r/333/#comment822> I think this should be "changed region before the region cap response for the previous region arrived" to be clear. indra/newview/llvoicevivox.cpp <http://codereview.secondlife.com/r/333/#comment823> https://wiki.secondlife.com/wiki/Coding_standard#Naming_Convention says local variables should be lower_case, not camelCase so make this parcel_local_ID. indra/newview/llvoicevivox.cpp <http://codereview.secondlife.com/r/333/#comment824> What does the first 'it' refer to? "ID with" what? indra/newview/llvoicevivox.cpp <http://codereview.secondlife.com/r/333/#comment825> Why not directly mCurrentRegion = gAgent.getRegion(); and then use mCurrentRegion in the rest of this method? indra/newview/llvoicevivox.cpp <http://codereview.secondlife.com/r/333/#comment826> Here, too, either just 'keeps' without 'staying' or 'makes [...] stay', I think. - Boroondas 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 > >
_______________________________________________ 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
