Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006531opensim[REGION] Script Functionspublic2013-02-04 23:262013-02-08 15:36
Reporterorenh 
Assigned Toorenh 
PrioritynormalSeverityfeatureReproducibilityN/A
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Versionmaster (dev code)Fixed in Version 
Summary0006531: [PATCH] Allow setting the Phantom flag of a single prim in a linkset
DescriptionThis patch extends the LSL functions llSetStatus() and llGetStatus() to allow setting and getting the Phantom flag of a single prim. The new flag is:

  OS_STATUS_PHANTOM_PRIM = 0x8000;

This is a companion to the existing flag, which changes the Phantom flag of the entire linkset:

  STATUS_PHANTOM = 16;
Steps To ReproduceSee "Additional Information" for a description of the problem that this function solves. Here's how to reproduce the problem:

1. Create two boxes, A and B.
2. Make box B phantom.
3. Link the boxes.
4. Take the linkset into your inventory, and rez it back. In the newly-rezzed copy box B won't be phantom.

The new llSetStatus() flag allows creating a script that will maintain box B's phantom status even when it's rezzed. Add the following script to box B:


integer OS_STATUS_PHANTOM_PRIM = 0x8000;

default
{
    state_entry()
    {
        llSetStatus(OS_STATUS_PHANTOM_PRIM, TRUE);
    }
    
    changed(integer change)
    {
        if (change & CHANGED_REGION_START)
            llResetScript();
    }
    
    on_rez(integer param)
    {
        llResetScript();
    }
    
    collision_start(integer nd)
    {
        llResetScript();
    }
}


Then, unlink box B; change it to phantom; and link it back. Now you can take the object into your inventory and rez it back and box B will remain phantom.
Additional InformationIt is often desirable to have a single phantom prim in a linkset. For example, this is often used to create stairs. However, the standard LSL functions only allow changing the phantom flag for an entire linkset, not a single prim. To fix this, highly complicated scripts have been designed: see http://wiki.secondlife.com/wiki/Phantom_Child. [^] These scripts are complicated and dangerous. This patch adds a minor change to llSetStatus() that allows modifying a single prim's phantom status.

There is already a precedent for this behavior in llSetStatus(): it has two flags, STATUS_BLOCK_GRAB and STATUS_BLOCK_GRAB_OBJECT, for changing the "STATUS_BLOCK_GRAG" flag for a prim and a linkset, respectively.
TagsNo tags attached.
Git Revision or version number6c82e4fbe916c860bcce7b09dbbdad269faec6d1
Run Mode Grid (Multiple Regions per Sim)
Physics EngineODE
Environment.NET / Windows64
Mono VersionNone
Viewer
Attached Filespatch file icon 0001-Extended-llSetStatus-and-llGetStatus-to-allow-settin.patch [^] (4,103 bytes) 2013-02-04 23:26 [Show Content]

- Relationships

-  Notes
(0023511)
melanie (administrator)
2013-02-05 11:24

Sorry, hard -1.

There are many interrelations between phantom, vd and physical and this patch doesn't account for any of them. For the sake of allowing a scripting trick, and not even in the way SL scripts are used to doing it, major other functionality is disturbed.
This patch would allow setting individual prims to phantom without regard to the object status and a whole. That status would, however, not persist in any form and be overwritten and lost during object transitions, like going physical or enabling VD. The behaviors are unpredictable and would often be unintended.
This patch has the potential to break other functionality and is certain to confuse scripters not familiar with OpenSim internals.
(0023512)
orenh (administrator)
2013-02-05 12:20

There are many Phantom Child scripts, and they already cause single prims to become phantom, so whichever problems might exist already exist. Therefore, adding this patch will not cause any new problems. It will ELIMINATE problems because it works much more safely than the current scripts.
(0023513)
melanie (administrator)
2013-02-05 12:22

It's incompatible, bloaty and will in practice not do what it says on the box. Better to let the existing SL hacks continue to work and remain compatible than to introduce a new flag that will make people believe that it would cause deterministic behavior when in fact it's not so.
(0023514)
orenh (administrator)
2013-02-05 12:24

I assure you that it does work, and better than the current scripts.
(0023515)
melanie (administrator)
2013-02-05 12:26

It's the wrong way. We should not introduce another hack when there is a canonical way (prim physics shape).
(0023516)
orenh (administrator)
2013-02-05 12:29

That way doesn't work. Perhaps it will in the long run, but "in the long run we are all dead".
(0023517)
melanie (administrator)
2013-02-05 13:09

And adding it now will leave us with a hack we can then not get rid of because it will be in use in existing content.
(0023519)
orenh (administrator)
2013-02-05 23:05

Changing the properties of individual prims is not a hack: it is a well-established practice which already exists in llSetStatus(). I am adding a missing feature, one which the market has shown is BADLY desired. Rejecting a good, working solution because something else might come along someday is not the OpenSim way.
(0023520)
melanie (administrator)
2013-02-05 23:10

The OpenSim way is also not to bloat code and to introduce hacks we're going to be stuck with forever just because no one stepped up to develop doing it the right way. There is a canonical solution - it needs to be implemented, not a hack.
(0023521)
orenh (administrator)
2013-02-05 23:12
edited on: 2013-02-05 23:12

One person's "code bloat" is another person's feature. Do you agree?

(0023522)
melanie (administrator)
2013-02-05 23:15

No. You call it a feature and prima facie it is. However, it would cause scripts to be written to use it instead of the canonical way and that means it would not be possible to remove this once the canonical way is implemented.
With letting this into the code base there would be two hacks (instead of just one), both doing the same thing, for no one's gain in the long run.
The existing hacks work, so can be used. There is no need for yet one more hack and the energy that went into this discussion should be invested into making the canonical way work.
(0023523)
orenh (administrator)
2013-02-05 23:18

The existing hack doesn't work well enough. Have you seen these scripts? They work by resetting the prim to a plain box, and then reapplying all of its previous properties (except for phantom). It's incredibly complicated because it needs to know about all the possible properties of prims. That makes it brittle when new features are added to OpenSim (or Second Life). Furthermore, if the script is terminated while it's running then the prim can remain in its "box" state, losing all of its previous properties. I have seen this happen. This patch will eliminate both problems: brittleness and data loss.
(0023524)
melanie (administrator)
2013-02-05 23:23

And introduce yet another "legacy hack" we can't get rid of again. Please invest your time into implementing the proper solution, prim physics shape.
(0023525)
melanie (administrator)
2013-02-06 00:20

Commit e5beb480eaf2 implements this to the point where scripts can use it. Note that this is the canonical way as mentioned above and does NOT need a "refresh" script in the prim!
(0023527)
orenh (administrator)
2013-02-06 03:10

Thanks, Melanie! You really closed the book on that one...

I hate to be a nitpicker, but I see two problems with that commit. First, in MySQLSimulationData.SerializeTerrain(), the height is fixed at 20 instead of using the given heightmap. Second, the same field is stored as "Restitution" in the database and "Bounce" in XML. It would be better to use the same name in both places (as well as in the code), for consistency.

I don't mean to burden you; I just want to make sure the issues I found are real. If so then I can patch them myself.
(0023528)
melanie (administrator)
2013-02-06 10:27

The code is as it is. I'm not going to get into a whole lot of merge hassle just to satisfy someone's sense of "right". We have loads of fields that have different names in the database than in code.
(0023534)
justincc (administrator)
2013-02-06 18:33

Melanie,

* Justifying adding a new inconsistently named field to core just because existing fields are badly named is NOT sufficient. Code in core should start out without a known inconsistency that we are going to be stuck with practically forever. Is there a good reason for the database column not to be named Bounce?

* Why did you not add an SQLite implementation? This is a database module that is supported by core and should have the same functionality as MySQL.

* Your commit appears to introduce an additional mistake where the StoreRegionWindLightSettings() call in core is disabled

@@ -741,7 +747,7 @@ namespace OpenSim.Data.MySQL
                         {
                             //No result, so store our default windlight profile and return it
                             nWP.regionID = regionUUID;
- StoreRegionWindlightSettings(nWP);
+// StoreRegionWindlightSettings(nWP);
                             return nWP;
                         }
                         else
(0023535)
melanie (administrator)
2013-02-06 18:37

Restitution is the proper (Linden) term. Bounce is what my developer called it. We can rename it but I didn't see it as terribly important.

I'm unable to test SQLite and Avination doesn't use SQLite. In the past our contributions have been welcome without SQLite implementations.

The removed WL call is actually a bug fix, see chat history. It's intended.
(0023536)
nebadon (administrator)
2013-02-07 08:57

I have tried to make this work, and nothing i do makes the child prims phantom, can someone please explain better how to test this?
(0023540)
justincc (administrator)
2013-02-08 15:36

Melanie, thanks for doing the rename from Bounce to Restitution in the code.

The SQLite implementations are almost always pretty simple, as is testing as it doesn't even require a database to be set up. As the OpenSimulator project has long-standing support for SQLite out of the box, this means that until such a time as SQLite is dropped (an idea which has been kicked about and rejected before), the expectation is that a release should always contain an equivalent SQLite implementation except where the functionality could be regarded as experimental.

By not implementing SQLite, you're usually pushing the cost of implementation on to me as the team member who does the fixup work in producing a release. I can't be bothered to make a big hullabaloo about this as the implementations are pretty simple (though still more costly for me as someone who did not write the original functionality) and this has been going on for a while. But this doesn't make me particularly happy and I would not accept an external patch that implemented storage in MySQL but not SQLite.

In this case, it so happens that Oren contributed the SQLite (and MSSQL, which isn't fully supported) implementations in http://opensimulator.org/mantis/view.php?id=6534 [^] so I (or perhaps someone else) will apply this soon.

- Issue History
Date Modified Username Field Change
2013-02-04 23:26 orenh New Issue
2013-02-04 23:26 orenh Status new => assigned
2013-02-04 23:26 orenh Assigned To => justincc
2013-02-04 23:26 orenh File Added: 0001-Extended-llSetStatus-and-llGetStatus-to-allow-settin.patch
2013-02-04 23:26 orenh Status assigned => patch included
2013-02-05 11:24 melanie Note Added: 0023511
2013-02-05 12:20 orenh Note Added: 0023512
2013-02-05 12:22 melanie Note Added: 0023513
2013-02-05 12:24 orenh Note Added: 0023514
2013-02-05 12:26 melanie Note Added: 0023515
2013-02-05 12:29 orenh Note Added: 0023516
2013-02-05 13:09 melanie Note Added: 0023517
2013-02-05 23:05 orenh Note Added: 0023519
2013-02-05 23:10 melanie Note Added: 0023520
2013-02-05 23:12 orenh Note Added: 0023521
2013-02-05 23:12 orenh Note Edited: 0023521 View Revisions
2013-02-05 23:15 melanie Note Added: 0023522
2013-02-05 23:18 orenh Note Added: 0023523
2013-02-05 23:23 melanie Note Added: 0023524
2013-02-06 00:20 melanie Note Added: 0023525
2013-02-06 00:20 melanie Status patch included => resolved
2013-02-06 00:20 melanie Resolution open => fixed
2013-02-06 03:10 orenh Assigned To justincc => orenh
2013-02-06 03:10 orenh Note Added: 0023527
2013-02-06 03:10 orenh Status resolved => feedback
2013-02-06 03:10 orenh Resolution fixed => reopened
2013-02-06 10:27 melanie Note Added: 0023528
2013-02-06 10:27 melanie Status feedback => resolved
2013-02-06 10:27 melanie Resolution reopened => fixed
2013-02-06 10:28 melanie Status resolved => closed
2013-02-06 18:33 justincc Note Added: 0023534
2013-02-06 18:37 melanie Note Added: 0023535
2013-02-07 08:57 nebadon Note Added: 0023536
2013-02-08 15:36 justincc Note Added: 0023540


Copyright © 2000 - 2012 MantisBT Group
Powered by Mantis Bugtracker