Mantis Bug Tracker
Main | My View | View Issues | Change Log | Roadmap | Summary | My Account
  

View Issue Details [ Jump to Notes ] [ Issue History ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006063opensim[REGION] Script Functionspublic2012-06-26 09:512012-07-06 23:38
ReporterTalun 
Assigned Tojustincc 
PrioritynormalSeverityfeatureReproducibilityhave not tried
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Versionmaster (dev code) 
Target VersionFixed in Version 
Summary0006063: [PATCH] an implementation of osNPCTouch
DescriptionFollowing on from an OSGrid forum thread and a chat with Shad MOrdre, a patch for consideration...

To allow NPCs to interact with objects (in the same region as the NPC) by touch as if they were using a gui client.

What I have got:-

void osNPCTouch(LSL_KEY npcKey, LSL_Key objectKey, LSL_INTEGER linkNum)

Only LINK_THIS and LINK_ROOT are valid for this function. Any other of the LINK_* constants will be ignored and no touch takes place.

1. If linkNum is LINK_THIS then the prim with the key objectKey will be touched.

2. If linkNum is LINK_ROOT or 0 then the root prim of the link set will be touched, even if the root prim key is not objectKey

3. For any other value of linkNum a search will be made through the linkset for a prim with that link number. If found that prim will be touched. If no prim is found for that link number thefinction fails silently and no touch takes place.

The touch is fired as if it came from an old client that does not support face touch detection or (probably) one of the text clients like Metabolt. Since there is no mouse the llDetectedTouch* functions will return the defaults (See the LSL Wiki for full details)

llDetectedTouchBinormal TOUCH_INVALID_VECTOR
llDetectedTouchFace TOUCH_INVALID_FACE
llDetectedTouchNormal TOUCH_INVALID_VECTOR
llDetectedTouchPos TOUCH_INVALID_VECTOR
llDetectedTouchST TOUCH_INVALID_TEXCOORD
llDetectedTouchUV TOUCH_INVALID_TEXCOORD

If the prim is not found or would not allow a normal client to touch it then this function fails silently.


TagsNo tags attached.
Git Revision or version numberHead
Run ModeStandalone (1 Region)
Physics EngineODE
Environment.NET / Windows64
Mono VersionNone
Viewer
Attached Filespatch file icon 0001-osNpcTouch-function.patch [^] (8,746 bytes) 2012-06-26 09:51 [Show Content]
txt file icon timing.txt [^] (1,000 bytes) 2012-06-27 12:49 [Show Content]
patch file icon 0001-Mantis-6063-Add-osNpcTouch.patch [^] (8,885 bytes) 2012-07-01 14:24 [Show Content]
patch file icon 0001-Mantis-6063-osNpcTouch.patch [^] (8,676 bytes) 2012-07-03 10:12 [Show Content]

- Relationships
parent of 0006066resolvedMichelle Argus Patch adding NPC to llDetectedType() 

-  Notes
(0021693)
melanie (administrator)
2012-06-26 23:50

 I believe the search for name could be somewhat costly and is of limited usefulness. It should probably be removed.
(0021695)
Talun (manager)
2012-06-27 00:52

 This was the only reliable way I could find to obtain the key of child prims in no-modify objects. Without this complex comms would be required in the NPC controlling script and the object containing the child prim to be touched so that keys could be passed. This would cause problems touching "bought in" items.

Finding keys for root prims can be done with llSensor with, I think, similar overhead to the method I have used. That also uses a linear search.

I will set up some timing tests and report back the results.
(0021696)
Michelle Argus (reporter)
2012-06-27 10:51

 I tested the patch and the new function works great.

However, as I already mentioned in the OSG forum, I do see major issues with some lsl systems around were NPCs could be used for abuse.

Example: We have a region were anyone can own land and landwoners can use NPCs on their land. A neighbour has a Votestation, eg. from Oliveira. Any Landowner could now use NPC to fake Avatarvotes, which can lead to getting banned by the Votesystem.

Some systems like Rentboxes and Vendors can be tricked out and even break compleatly due to false names and uuid which dont belong to existing real avatars. In my case 1 Project with over 4 years of development and more than 300k scriptlines in lsl and php are going down the drain.

My suggestion would be to introduce NPCs to llDetectedType which also is used in sensor/touch/collision events OR intruduce touch events to OSSL and exclude NPCs from all lsl events.
(0021698)
Talun (manager)
2012-06-27 12:48

 Michelle, the problems with NPCs you note above probably deserve a Mantis of their own. Additionally it is not possible to eject an NPC, I have not yet tried banning but I suspect that may have problems too.

--------

The speed tests....

Since people will probably use unlinked objects and sensor to obtain IDs since they change whenever an object is rezzed so cannot be hardcoded I have compared the speed of a named sensor and osNPCTouch with a name that would not be found.

I filled a newly created region with 15,000 prims.

I modified the calls to doObjectSensor in SensorRepeat and GetSceneObjectPart(name)in Scene.cs to repeat the call 1,000 times and report the total time in that loop.

Sensor will always iterate through all the objects and GetSceneObjectPart(name) will always stop on the first match, as mentioned above I used a name that would not be found to force a full scan so that the following results are the normal times for sensor but worst case for the search from osNPCTouch.

Note, times are for 1000 searches!
Sensor Search by name took 8798 millis
Sensor Search by name took 8221 millis
Sensor Search by name took 7831 millis
Sensor Search by name took 8814 millis
Sensor Search by name took 8065 millis

osNPCTouch Search by name took 5991 millis
osNPCTouch Search by name took 6006 millis
osNPCTouch Search by name took 5975 millis
osNPCTouch Search by name took 6022 millis
osNPCTouch Search by name took 5959 millis

The small code changes to get the times are attached as timing.txt
(0021700)
melanie (administrator)
2012-06-27 14:12

 The times for your optimized search are a bit lower, but i do hold that usefulness is limited. Child prims in bought objects seldom have sensible names so that a generic way to address them needs to be found.
I believe that search by name for root prims only would be even faster than for child prims and the root prim uuid plus a child prim index would be a workable solution. osNPCTouch(key npc, key object, integer link) should do the trick.
(0021701)
Talun (manager)
2012-06-27 17:08

 The average for sensor above is 8346 millis and the straight linear search 5990 almost 30% faster and that is worst case.

My test objects/script would show no difference in speed searching by root part name, they are all single prims, I have no idea what the normal percentage of linked prims and average linkset size would be.

I had anticipated that creators might save the key from name searches for re-use, hence the return value.

You suggest osNPCTouch(key npc, key object, integer link), I had not thought of using the link number but the signature is just for a straight key lookup without the option to search or did you mean osNPCTouch(key npc, string keyOrName, integer link) so that name search of root prims was still possible if keyOrName did not parse to a UUID?
(0021702)
Michelle Argus (reporter)
2012-06-27 17:55

 Added a Patch to include NPCs to llDetectedType: http://opensimulator.org/mantis/view.php?id=6066 [^]

 To prevent that OS version are available were no security checks can be done, the patch for including NPCs in llDetectedType() should be implemented prior to osNPCTouch.
(0021705)
justincc (administrator)
2012-06-28 01:06

 My comments.

* I don't like touch by name either. Even if objects are explicitly renamed there is no way to distinguish between multiple copies with the same name, or different objects with the same name. I think we should only allow object keys.

* I believe adding the link number is unnecessarily complicated and also of limited usefulness - an object designer would just have to change their object slightly for any link numbers fixed in scripts to become invalid. It seems to me better to get child prims of interest via functions such as llGetLinkName() which could match via iteration a child prim of interest (e.g. a button called "nuke") and then invoke touch with llGetLinkKey() or similar.

* I guess the scenario I'm thinking of is where an NPC first detects an object (e.g. via a sensor in an attachment and is then able to find other prims using existing functions). What other use cases are there?

* I'm not fond of having NPCAvatar.Touch(). In theory NPCAvatar is meant to go away as there is no client component in the scene. I know there's theoretically more flexibility at the moment as it can invoke the object events, but again in theory, methods such as ProcessObjectGrab should eventually move out to modules. Going through the client interfaces often unnecessarily introduces another layer of having to look up objects we already know about via UUID - cpu busywork (OpenSim is rife with this). I would rather see these methods invoked directly for now, which would be more in common with other NPCModule methods.

* It shouldn't be necessary to do part == null or IsDeleted checks on something just retrieved from GetSceneObjectPart. That method should never return null and doing an IsDeleted check there will do virtually nothing to shut down any race conditions (the standard response here is to let the code get on with it - any actions done should theoretically have no impact with a deleted object, and updates generated for already deleted objects will get caught before they go out to viewers).

Please feel free to debate these points - I think this is a complicated topic that is worth some thought. I'm also writing this at the end of the day so please forgive/correct any inaccuracies/omissions.
(0021706)
melanie (administrator)
2012-06-28 02:02

 * I'm -1 on touch by name. Although it's faster, it harbors the danger of touching a far away object not intended to be touched and may lead to "touch hijacking", e.g. intentionally creating objects of equal names to an object the NPC is known to interact with. Bad Mojo.

* The link number is of great usefulness since sold objects don't get changed very often. Vendors, for instance, will always use the same link number for the prims because the internal scripts already depend on them. Link number is the only way to touch a child prim on a non-cooperating object. The llGetLinkKey, et. al. functions are only available inside the object and therefore cannot be used by an NPC emulating a real client.

* Other use cases are interacting with known objects (detected caspervend vendor, touching "forward" button, link 2).... where the link number of the forward button for that particular object is simply known.

* Although currently client based methods may move to modules in the future I personally believe that a (simulated) client allows greatest flexibility. In that sense I believe this is the correct approach. The point of whether or not NPC should even be able to do these things is debatable. There is a huge potential for griefing and/or crashing scripts, but also there is a whole new set of options for regression testing things that depend on avatar actions. Since OSSL functions have threat levels and allow functions, this can easily be minimized.

* Agree on the null checks, drop them
(0021708)
Michelle Argus (reporter)
2012-06-28 08:00

 * As the name search can result in many unwanted and/or random items to be pased on, I would use a Addon scripts dropped in the items to be touched which reliably passes the wanted information to the npc script. Addon scripts are common pratice for most vendors and also can be used to find the correct linknumber if needed.

Using the existing sensor returns all items in a defined range which then can be sorted by whatever criteria the npc script needs.

* I like the idea of using the link number. I do however agree with justin that these can change as people can modify their vendors by eg. adding additional displays or removing backward buttons to reduce primusage
(0021709)
Talun (manager)
2012-06-28 12:58

 So....
No name search and LINK_ROOT/LINK_THIS or a link number similar to llGetLinkName and the null check removal.

I will remove the return of the key too.

Work in progress...
(0021710)
justincc (administrator)
2012-06-28 21:43

 Further comments.

* Fair points about link numbers. I still think it's a very clumsy way of doing this but I can't see any LSL (or even OSSL) functions for retrieving details about other child prims. I can envisage future OSSL functions that can retrieve this data, so that NPC's, for instance, can look for child prims that meet certain critera (e.g. names that ends with "button"). However, this is something for another time and place - I think link number will be sufficient in this function.

* Search by SOP ID is no slower than SOG ID since SOP ID search uses an index - it does not iterate through prims. Therefore, I think the id should still accept part IDs since this is potentially more convenient, rather than possibly having to fetch a full ID after detecting some part ID. It would also be more compatible with multi-level link sets (if we ever get there). If the ID is for a part, then any non-root link parameter could return an error.

* I disagree about clients since I think they add an extra layer of complexity (memory use and indirection) for no gain compared with invoking module interfaces or future interfaces directly. However, I accept this is a debatable point - I don't have an issue with having this code in NPCAvatar right now.

Thanks for considering our feedback and being willing to adapt your patch, Talun.
(0021712)
melanie (administrator)
2012-06-28 21:58

 So, Talun, please leave in link numbers, allow child prim ids and remove name search and key return.

If a child prim ID is given, LINK_THIS should touch the child prim. Real link numbers should address child prims of the object as if a root id was given. Additionally, LINK_ROOT makes sense then, meaning to touch the root from a given child id. This could be useful if touching something as a result of a chat message received from a non-root prim.
(0021717)
Michelle Argus (reporter)
2012-06-29 17:57

 Just a thought after a test:

I tried to see how many touches a NPC can manage to process in 1 second using a simple while loop. My server was under quite some load when testing and I came to an avarage of 1200 touches per second.

 I normaly am not fond of adding limitation, but maybe one should think if a default limit per timeinterval should be implemented to prevent extensive spamming?

But, just thinking out load...
(0021720)
justincc (administrator)
2012-06-29 23:23

 I feel the security discussion is best made in an opensim-dev mailing list thread rather than on this mantis.
(0021723)
Talun (manager)
2012-07-01 14:26

 Sorry for the delay, a new patch 0001-Mantis-6063-Add-osNpcTouch.patch has been added. I will update the description above to reflect the changes made and that text will form the basis of a wiki entry for the function
(0021728)
melanie (administrator)
2012-07-01 19:22

 * A link number that is not found, or an invalid LINK_* constant, should cause no touch to be generated.

* As there is no more name lookup and no more return value, the description of these two features should be removed.
(0021730)
Talun (manager)
2012-07-01 19:51

 I have updated the description.

What about the case of the function being called to touch an unlinked prim using LINK_ROOT instead of zero or trying to touch a link set using link number zero, should these fail too?

At the moment I treat these equally and send the root prim of a link set or the only prim if unlinked.
(0021731)
melanie (administrator)
2012-07-01 20:16

 That appears to be correct behavior. A single prim is treated as the root prim for the purposes of link messaging, so that should be the same here to be consistent across the API.
(0021732)
melanie (administrator)
2012-07-01 20:19

 Below are the two documentation changes I would suggest. Those express the desired behavior.
Changes are in upper case.


Only LINK_THIS and LINK_ROOT are valid for this function. Any other of the LINK_* constants will BE IGNORED AND NO TOUCH TAKES PLACE.

3. For any other value of linkNum a search will be made through the linkset for a prim with that link number. If found that prim will be touched. If no prim is found for that link number THE FUNCTION FAILS SILENTLY AND NO TOUCH TAKES PLACE.
(0021741)
Talun (manager)
2012-07-03 10:16

 A noew patch making the requested changes has been added
0001-Mantis-6063-osNpcTouch.patch
(0021759)
justincc (administrator)
2012-07-05 22:15

 Link to original discussion thread which inspired this mantis for reference http://forums.osgrid.org/viewtopic.php?f=6&t=4189 [^]
(0021762)
justincc (administrator)
2012-07-06 21:42

 Since I believe the latest patch matches the things that we discussed, I've applied as git master 1b1f841. If there are further amendments these can be done in POST. Thanks very much, Talun!

As discussed in IRC, for more sophisticated touch requirements we can add a future overloaded osNpcTouch() function that takes parameters/options. However, I think we should limit this to just one additional function.

In your comments Talun you mention that there is documentation for this function? Is this on the opensimulator wiki?
(0021765)
justincc (administrator)
2012-07-06 23:33

 reopening so Talun can make some textual adjustments
(0021766)
Talun (manager)
2012-07-06 23:36

 I have updated the wiki with a simple example http://opensimulator.org/wiki/OSSLNPC [^]

And added the documentation from the description above to http://opensimulator.org/wiki/OSSL_Implemented [^]

- Issue History
Date Modified Username Field Change
2012-06-26 09:51 Talun New Issue
2012-06-26 09:51 Talun File Added: 0001-osNpcTouch-function.patch
2012-06-26 09:52 Talun Status new => patch included
2012-06-26 10:00 Talun Description Updated View Revisions
2012-06-26 23:45 justincc Assigned To => justincc
2012-06-26 23:45 justincc Status patch included => assigned
2012-06-26 23:50 melanie Note Added: 0021693
2012-06-26 23:53 justincc Status assigned => patch included
2012-06-27 00:52 Talun Note Added: 0021695
2012-06-27 10:51 Michelle Argus Note Added: 0021696
2012-06-27 12:48 Talun Note Added: 0021698
2012-06-27 12:49 Talun File Added: timing.txt
2012-06-27 14:12 melanie Note Added: 0021700
2012-06-27 17:08 Talun Note Added: 0021701
2012-06-27 17:55 Michelle Argus Note Added: 0021702
2012-06-27 17:56 Michelle Argus Relationship added parent of 0006066
2012-06-28 01:06 justincc Note Added: 0021705
2012-06-28 02:02 melanie Note Added: 0021706
2012-06-28 08:00 Michelle Argus Note Added: 0021708
2012-06-28 12:58 Talun Note Added: 0021709
2012-06-28 21:43 justincc Note Added: 0021710
2012-06-28 21:58 melanie Note Added: 0021712
2012-06-29 17:57 Michelle Argus Note Added: 0021717
2012-06-29 23:23 justincc Note Added: 0021720
2012-07-01 14:24 Talun File Added: 0001-Mantis-6063-Add-osNpcTouch.patch
2012-07-01 14:26 Talun Note Added: 0021723
2012-07-01 14:28 Talun Description Updated View Revisions
2012-07-01 19:22 melanie Note Added: 0021728
2012-07-01 19:43 Talun Description Updated View Revisions
2012-07-01 19:51 Talun Note Added: 0021730
2012-07-01 20:16 melanie Note Added: 0021731
2012-07-01 20:19 melanie Note Added: 0021732
2012-07-03 10:12 Talun File Added: 0001-Mantis-6063-osNpcTouch.patch
2012-07-03 10:16 Talun Note Added: 0021741
2012-07-03 10:16 Talun Description Updated View Revisions
2012-07-05 22:15 justincc Note Added: 0021759
2012-07-06 21:42 justincc Note Added: 0021762
2012-07-06 21:42 justincc Status patch included => resolved
2012-07-06 21:42 justincc Resolution open => fixed
2012-07-06 23:33 justincc Note Added: 0021765
2012-07-06 23:33 justincc Status resolved => feedback
2012-07-06 23:33 justincc Resolution fixed => reopened
2012-07-06 23:36 Talun Note Added: 0021766
2012-07-06 23:36 Talun Status feedback => assigned
2012-07-06 23:38 Talun Status assigned => resolved
2012-07-06 23:38 Talun Resolution reopened => fixed

Copyright © 2000 - 2012 MantisBT Group
Powered by Mantis Bugtracker