VWR-17050 No nearby people when over approxiamately 1000 meters


Submitter:	Twisted Laws	Reviewers
Branch:		Groups:	viewer
Bugs:	VWR-17050	People:
Change Number:	None	Repository:	viewer-development

Description:

This modifies the getAvatars function in llworld to also include any avatars that are found within the range from the LLCharactes list as well (the list of avatars that is in the viewer object list).  This should make sure that anyone that you visually see within range shows up in the list.  Note that changing it in this function also affects LLFloaterAvatarPicker::populateNearMe, LLLocalSpeakerMgr::updateSpeakerList, as well as the LLPanelPeople::updateNearbyList that was originally mentioned in the Jira.  The region avatars lists only contain valid position data when the avatars are below 1024m.  The comment that mentions about retrieving uuids is based on the function, not the current uses.  No current calls in the code are done with the avatar_ids argument NULL.  Duplicates in the returned list need to be suppressed.

Testing Done:

Simple testing in sandboxes of this patch at 20m and 2000m heights with and without avatars nearby.  Tested with varying the NearMeRange to insure it does not show avatars beyond the range.  Testers need to understand that RenderFarClip has an impact on the avatars that are actually in the viewer object list, so setting NearMeRange to a great distance at high altitude won't necessarily add avatars to the list.  Basically if you can see the avatar and its within NearMeRange, the avatar should be in the nearby avatar list in the people panel.

Expand All

Summary changed from No nearby people when over approxiamately 1000 meters to VWR-17050 No nearby people when over approxiamately 1000 meters

Description:

fixed the title

It seems to me that the new code can completely replace the old one, i.e. the search based on the map is not adding anything different or is it?

Twisted Laws Jan. 31, 2011, 7:12 p.m. (Jan. 31, 2011, 7:12 p.m.)

There are several reasons that replacing the code may provide a different result.  The list of avatars returned by the function is based on NearMeRange which could be a greater value than RenderFarClip.  In that case, replacing the code would only return the list of avatars in RenderFarClip range and would ignore a greater NearMeRange.  In the case of a person owning an island, they may have NearMeList set to 1200 so they could see the presence of any avatar in their region at any altitude and yet have RenderFarClip set to 128.  I realize that at this point its not really "NearMe".  Also I've noticed in the sandboxes that quite often a person shows up in the region avatar list before showing up in the viewer object list and in my personal implementation of a radar floater I mark the avatar names that are not visible.  In the released versions of the Nearby list, the Zoom-in option is not available for avatars that are not visible so designers must have considered this situation.

I guess my other reason would be changing it in llworld wouldn't be the right place to have the function getAvatars if it ignored map data and only used character data.  Maybe creating a new getAvatars in LLCharacters would be the right way to do it then.

indra/newview/llworld.cpp (Diff revision 1)

I fail to see how the "1000m" condition is tested in this code. Same for the "only do this when we are retrieving uuid's".

Twisted Laws Jan. 31, 2011, 6:55 p.m. (Jan. 31, 2011, 6:55 p.m.)

It can only provide the function if it has been storing uuid's (avatar_ids not null) or it would provide duplicates.  The reason its for over 1000m is the viewer object list position is ignored if it had already added the uuid from the region data.  I.e, the region avatar position is incorrect if the user is over 1024m and will not be added to the list as its not in NearMeRange.

indra/newview/llworld.cpp (Diff revision 1)

This area where I search for the existance of an uuid in the list is something that works but it would seem that theres some better method.  Open to suggestions.

indra/newview/llworld.cpp (Diff revision 1)

I would strongly prefer to see this rearranged as properly nested if/then/else rather than using continue.

Similarly, I'd like to have the break replaced by just adding another exit condition to the 'for' loop.

Diff
- added Diff r2

Description:

I redid the patch, hopefully making the comments more acceptable and took out the use of continue.   I moved retrieval of the character positions before the world positions as I found cases where the information would be incorrect right near 1024 testing with a couple people below 1024, and a couple people above.  Note this is a replacement patch.

Kitty Barnett Feb. 5, 2011, 6:36 a.m. (Feb. 5, 2011, 6:36 a.m.)

Is there any reason to "fix" this in getAvatars() rather than just "fixing" it when processing the CoarseLocationUpdate legacy region message (and when processing the still unused cap)?

The minimap for instance won't call getAvatars() but will rather access the position directly so it would still be broken for nearby avatars when >1000m.

Additionally, if the plan is to still enable the http cap at some point in the future then fixing the viewer's handling of that now would ideally mean that the problem is fixed for everyone as soon as it's enabled rather than having to wait another quarter for the next viewer release that contains the fix.

Twisted Laws March 1, 2011, 7:20 p.m. (March 1, 2011, 7:20 p.m.)

Note that the final version of this patch that was tagged "ship it" was not what was in the merge in viewer-development, instead the first version was included.

You have a pending review.

Other reviews

Your comment

Review Board 1.6.11

This change has been marked as submitted.