What's wrong with this profit splitting script?

This is supposed to pay a share to a friend but it's seems to be very unreliable. Sometimes it's works sometimes it doesn't (as my transaction history revealed). Can anyone spot a problem? Thanks in advance.

 // simple single prim multi person vendor script. pays you then you pay the other person the amount set
key payee = "fdb2e05b-7e12-4cb4-a63e-e1baf4357a74";

integer price = 120;
integer share = 60;
string name;
list inventory;


default
{

    on_rez(integer param)
        {
            llResetScript();
        }
    
    state_entry()
        {
            llSetPayPrice(price, [price, PAY_HIDE, PAY_HIDE, PAY_HIDE]);
            llRequestPermissions(llGetOwner(), PERMISSION_DEBIT);
        }
    
    run_time_permissions(integer perm)
        {
            if(perm & PERMISSION_DEBIT)
            {
                state online;
            }
        }
    }
    
    state online
        {
        on_rez(integer param)
            {
                llResetScript();
            }
    
    money(key id, integer amount)
        {
            integer i;
            integer num = llGetInventoryNumber(INVENTORY_ALL);
            
            for (i = 0; i < num; ++i) {
                name = llGetInventoryName(INVENTORY_ALL, i);
                if(llGetInventoryPermMask(name, MASK_OWNER) & PERM_COPY) {
                    inventory += name;
                } else {
                    llOwnerSay("Cannot give asset \""+name+"\", owner lacks copy permission");
                }
            }
            
            //we don't want to give them this script
            i = llListFindList(inventory, [llGetScriptName()]);
            if(~i) {//if this script isn't found then we shouldn't try and remove it
                inventory = llDeleteSubList(inventory, i, i);
            }
            
            if(amount > price)
            {
                llGiveInventoryList(id, llGetObjectName(), inventory);
                llGiveMoney(payee,share);
                
                llGiveMoney(id, amount - price);
            }
            if(amount < price)
            {
                llGiveMoney(id, amount);
            }
            else
            {
                llGiveInventoryList(id, llGetObjectName(), inventory);
                llGiveMoney(payee, share);
            }
            
            inventory = [];
        }

}

I don't like state changes, personally, so I don't know if the state change you have is mucking up your debit permission.

Other than that, nothing jumps out at me. One thing you could try is adding some debugging code to it to IM you on failures or successes and see if it's breaking in certain situations.

Obviously you can't detect if givemoney fails, but you can at least see that it called the function.

Hmm would this work without these state changes?

The code isn't very optimal...so I'm reposting with some changes that may help:

// simple single prim multi person vendor script. pays you then you pay the other person the amount set
key payee = "fdb2e05b-7e12-4cb4-a63e-e1baf4357a74";

integer price = 120;
integer share = 60;
string name;
string SCRIPT;
string FOLDER;
list inventory;

//Making you a build inventory function to run at state_entry and changed in the "online" state.  That way, you don't run it every purchase.
getinv()
{
inventory = [];
FOLDER = llGetObjectName();
//Setting your folder name once instead of at every purchase
integer i;
integer num = llGetInventoryNumber(INVENTORY_ALL);

for (i = 0; i < num; ++i) {
name = llGetInventoryName(INVENTORY_ALL, i);
if(name != SCRIPT)  //Checking against script name BEFORE adding to list, instead of deleting after.
{
if(llGetInventoryPermMask(name, MASK_OWNER) & PERM_COPY) {
inventory = (inventory = []) + inventory + [name];  //efficient list management
} else {
llOwnerSay("Cannot give asset \""+name+"\", owner lacks copy permission");
}
}

default
{

on_rez(integer param)
{
llResetScript();
}

state_entry()
{
SCRIPT = llGetScriptName();  //Get the script name
llSetPayPrice(PAY_HIDE, [price, PAY_HIDE, PAY_HIDE, PAY_HIDE]);
//Why leave the option to pay more/less if you just want them to pay the stated price?
//Changed the initial price to a PAY_HIDE
llRequestPermissions(llGetOwner(), PERMISSION_DEBIT);
}

run_time_permissions(integer perm)
{
if(perm & PERMISSION_DEBIT) state online;
}
}

state online
{
state_entry()
{
getinv();
}

changed(integer change)
{
if(change & CHANGED_INVENTORY) getinv();
}

on_rez(integer param)
{
llResetScript();
}

money(key id, integer amount)
{
//Why bother with over/under pay amounts--you're restricting them to one button option anyway.  Just double-check it and if the amount is incorrect at all, send it back.
if(amount != price) 
{
llGiveMoney(id, amount);
llWhisper(0, "The price of " + FOLDER + " is " + (string)price + ".  Please pay that amount.");
}
else
{
llGiveInventoryList(id, FOLDER, inventory);
llGiveMoney(payee,share);
}
}

There are other things that could be done to the script--but that should get you going at least a bit better.

Why are you putting an on_rez event into a state (other than the default state)?? The default state is always the first state entered, and the script reset is being done in the on_rez event in the default state. Putting an on_rez event into any other state is useless, since the script will already be rezzed by the time it gets to the online state. Also, you only need it once in the default state.

If I create a script that has two states - default and running

I do a switch to running at some stage.

I take the object into inventory and pull it back out, it will continue to operate in the running state - the on_rez in the default state will not be called.

If you have on_rez ( on_changed , etc ) processing and you switch states then you should put those sections in every state that could be running at the time it is taken and pulled back out of inventory, as that state will continue running when it is rezzed

Thanks for all your replies! Thanks Kenn I'll try that script.

I'm not much of a scripter, this script I actually found in the forum and then just edited it so the Object name would be the folder name.

You'd merely get rid of the state online command in the on_rez section, and move the state online stuff into the default section. Oh, and eliminte the on_rez section that does the reset script. Probably not helping you any, and you don't need to reset the script for the vendor I think.

Then just check the perms in the money section before you try to pay out.

But Kenn's code is much cleaner.

I do want to mention, however, that the primary reason to use a state-change in this vendor is to ensure that the owner grants the PERMISSION_DEBIT permission. There are definitely ways to make sure this permission has been granted without changing states--but a state change on granted-permission is the easiest.

State changes aren't necessarily bad, they just need to be used appropriately.

The llResetScript(); through the on_rez() event is just a simple way of ensuring that, when rezzed, the script always asks the current owner for PERMISSION_DEBIT. It's not a bad thing to do, though there are again other ways around it (however, an llResetScript() does have the advantage of clearing absolutely everything from the script quickly and easily).

Anyway...all in all it's a very simple, very basic, vendor script. There are quite a few other things that could be done to it that would add to usability and efficiency. Still, it'll accomplish what you want it to accomplish.