Script that counts landmarks given not working

[Liana Halostar]

I have the following script that counts how many LMs have been given. The problem is that it always tells me it has given 0 LMs even when I know it has given some out. (And yes, I reset the script.) Any ideas? Or maybe a different script that works?

integer gifts = 0; // sets initial value to zero
 
default
{
    on_rez(integer start_param)
    {
        llResetScript(); // this resets whenever the prim is brought from inventory
    }
 
    state_entry()
    {
        llSetText( "Touch for a Landmark", <1.0, 1.0, 1.0>, 1.0);
        //sets the hover text "message", color (here white), and intensity (can be zero to 1.0)
    }
 
    touch_start(integer total_number)
    {
        if ( llDetectedKey(0) != llGetOwner() )// if the toucher is NOT the owner (!=)
        {
            gifts = gifts++; // count up one from previous value
            llGiveInventory(llDetectedKey(0), llGetInventoryName(INVENTORY_LANDMARK, 0));
        }
        else if ( llDetectedKey(0) == llGetOwner() ) // if the toucher IS the owner (==)
        {
            llOwnerSay((string)gifts + " people have gotten landmarks from me!");
        }
    }
}

[Rolig Loon]

Try replacing

gifts = gifts++;

with

++gifts;

[Liana Halostar]

Thanks Rolig. That did the trick

[Ela Talaj]

Ohh... this is a lovely question! Worth some elaboration. The problem is of course with gifts = gifts++ as Rolig pointed out, but why? In fact, it is one of the very frequent interview questions and the number of entry-level programmers not getting a job because failing to evaluate i = i++ is probably a few too many.

The answer has to do with something called sequence point. It is well defined in here:

http://en.wikipedia.org/wiki/Sequence_point

The article might be a lil too complex for LSL wannabees but would mighty help in preparing for that interview for whoever has it scheduled

[Liana Halostar]

Thanks Ela

That's over my head (I have limited ability to modify scripts but not write them) but hopefully others will benefit.

For reference, I found the script on the SL Wiki: http://wiki.secondlife.com/wiki/User:Toady_Nakamura/Landmark_Giver_and_Counter.

You'd think that anybody publishing a script would have tested it first.

[Sieben Ochs]

Given the way gifts is being called. There is no need for gifts = gifts++; over just using gifts++ or ++gifts.

Only changed it minorly. As the duplication of is or is not owner bugged me. And a simple one time call to an else works.

 

integer gifts = 0; // sets initial value to zero default{    on_rez(integer start_param)    {        llResetScript(); // this resets whenever the prim is brought from inventory    }     state_entry()    {        llSetText( "Touch for a Landmark", <1.0, 1.0, 1.0>, 1.0);        //sets the hover text "message", color (here white), and intensity (can be zero to 1.0)    }     touch_start(integer total_number)    {        if ( llDetectedKey(0) == llGetOwner() ) {// if the toucher IS the owner (==)            llOwnerSay((string)gifts + " people have gotten landmarks from me!");        } else { // if the toucher is NOT the owner (!=)            // count up one from previous value            ++gifts;            llGiveInventory(llDetectedKey(0), llGetInventoryName(INVENTORY_LANDMARK, 0));            llOwnerSay((string)gifts + " people have gotten landmarks from me!");        }    }}

 

[Liana Halostar]

Do these changes have a practical effect? Like reducing script memory? Or is is just a cleaner, more efficient way to write it?

[Rolig Loon]

The new way of writing it does reduce script memory slightly, but not enough to be a compelling reason to do it.  The better reason is clarity. If you can get a message across in fewer words, why not do it?  Since one test is just the reverse of the other (++ vs. !=), you don't have to repeat yourself.  In fact, you can do even better by writing

touch_start(integer total_number)    {        if ( llDetectedKey(0) != llGetOwner() ) {// if the toucher is NOT the owner            // count up one from previous value            ++gifts;            llGiveInventory(llDetectedKey(0), llGetInventoryName(INVENTORY_LANDMARK, 0));        }        // Whether the toucher is the owner or not ....        llOwnerSay((string)gifts + " people have gotten landmarks from me!");    }

 After all, you are going to send the llOwnerSay message no matter what.  Just put that action at the end as a cleanup once you have decided whather to give a landmark or not.

[Liana Halostar]

Thanks Relig. Anything that improves the script is always welcome.

Though I'm not sure why the OwnerSay message is delivered along with the LM since there is typically nobody there to hear it. Poor thing must be lonely.

[Rolig Loon]

Good point.  I didn't question your reasons for doing that, just noted that you could do it in a more compact way.  This is really the heart of scripting ... the logic, figuring out exactly what you want the script to do and developing a road map for getting there.  The rest of the stuff in this thread is challenging in its own way, but it's all mechanics, once you have the logic figured out.