Scripting Advice

This is the first time I have actually tried writing a script from pretty much scratch. It does what I want, but I'm sure it isn't the most efficient or streamlined way of doing it. Any advice or suggestions would be appreciated.

The scripts are for some photography studio lights I am working on. The "bulbs" each get the bulb script, and the base of the lamp gets the control script.

Bulb Script

integer CHANNEL = 41;

default {
    state_entry() {
        llListen(CHANNEL, "", NULL_KEY, "");
    }
    
        
    listen(integer channel, string name, key id, string message) 
    {
            if (message == "Red")
                {llSetPrimitiveParams([PRIM_POINT_LIGHT, TRUE, <1.0,0.0,0.0>, 1.0, 3.0, 0.5]);
                {llSetColor(<1.00000, 0.000, 0.00000>, ALL_SIDES);}}
            else if (message == "Blue")
                    {llSetPrimitiveParams([PRIM_POINT_LIGHT, TRUE, <0.0,0.0,1.0>, 1.0, 3.0, 0.5]);
                    {llSetColor(<0.0,0.0,1.0>, ALL_SIDES);}}
            else if (message == "Green")
                    {llSetPrimitiveParams([PRIM_POINT_LIGHT, TRUE, <0.0,1.0,0.0>, 1.0, 3.0, 0.5]);
                    {llSetColor(<0.0,1.0,0.0>, ALL_SIDES);}}
            else if (message == "Yellow")
                    {llSetPrimitiveParams([PRIM_POINT_LIGHT, TRUE, <1.0,1.0,0.0>, 1.0, 3.0, 0.5]);
                    {llSetColor(<1.0,1.0,0.0>, ALL_SIDES);}}
            else if (message == "Purple")
                    {llSetPrimitiveParams([PRIM_POINT_LIGHT, TRUE, <1.0,0.0,1.0>, 1.0, 3.0, 0.5]);
                    {llSetColor(<1.0,0.0,1.0>, ALL_SIDES);}}
            else if (message == "White")
                    {llSetPrimitiveParams([PRIM_POINT_LIGHT, TRUE, <1.0,1.0,1.0>, 1.0, 3.0, 0.5]);
                    {llSetColor(<1.0,1.0,1.0>, ALL_SIDES);}}
            else if (message == "Off")
                    {llSetPrimitiveParams([PRIM_POINT_LIGHT, FALSE, <1.0,1.0,1.0>, 1.0, 3.0, 0.5]);
                    {llSetColor(<1.0,1.0,1.0>, ALL_SIDES);}}
                             
     }
}

Control Script

integer CHANNEL = 45869;
list MENU_MAIN = ["Blue", "Red", "Green", "Yellow", "Purple", "White", "Off"];

default
{
    state_entry() {
        llListen(CHANNEL, "", NULL_KEY, "");
    }
    listen(integer channel, string name, key id, string message)
    { 
    
            if (message == "Blue")
                llShout(41, "Blue");
            else if (message == "Red")
                llShout(41, "Red");
            else if (message == "Green") 
                llShout(41, "Green");
            else if (message == "Yellow")
                llShout(41, "Yellow");
            else if (message == "Purple") 
                llShout(41, "Purple");
            else if (message == "White")
                llShout(41, "White");
            else if (message == "Off") 
                llShout(41, "Off");
                 
     }

    touch_start(integer total_number)
    { 
    if (llSameGroup(llDetectedKey(0))) 
    { 
        llDialog(llDetectedKey(0), "Studio Lights", MENU_MAIN, CHANNEL);
    } 
    else 
    { 
        llWhisper(0,"Owner Access Only"); 
    } 
}
}

Again - I appologize if this is sloppy coding and will definitely appreciate any guidance.

Thanks,
Sparkly

It did appear at first glance that you're listening on one channel and speaking on another; compare:

integer CHANNEL = 41;

with

integer CHANNEL = 45869;

Is that the problem?

I "think" that the channel listed in the control script just sets the channel for the dialog box to speak back to the script. The channel set in the bulb script is the one that the bulbs listen for commands on.

The scripts work fine - the communication seems to work. Like I said before, I'm new to this and just wanted to get any tips on making the scripts more efficient or sim friendly.

Thanks.

Woah.. Easy with the curly braces, Sparkly.

list gColorInfo = ["Red", <1.0, 0.0, 0.0>, "Blue", <0.0, 0.0, 1.0>, more colors...];
.
.
.
    listen(integer channel, string name, key id, string message) 
    {
        integer index = llListFindList (gColorInfo, [message]);
        if (index != -1)
        {
                vector color = (vector)llList2String (gColorInfo, index + 1);
                llSetPrimitiveParams([PRIM_POINT_LIGHT, TRUE, color, 1.0, 3.0, 0.5]); 
                llSetColor(color ALL_SIDES);
        }
    }

For the control script, I'm not sure why you have all the if/shouts.. Just shout whatever the dialog sends you...


edit: or, plan B, have the control script shout the color vector that you want.. That way, you don't have to have the names in both scripts - want a new color? Just add it to the control script..

edit edit: also, just my personal preference, don't use hardcoded numbers down in the code itself.. Define CHANNEL_DIALOG, CHANNEL_COLOR, etc..

As Meade suggests, dumb down your bulb scripts as much as possible, no need to duplicate effort in each light, do it once then use the result.

Again as Meade has already pointed out, use of a list would be more memory efficient and also simplifies the code somewhat.

I would however change your control script to only activate the listen when it is required, i.e. when someone authourised touches it. And shut it off when you have finished with it.

Also use higher channel numbers or even negative ones. Less chance of bumping into someone elses chat.

Using seperate lists for the colour names and the colour vectors would allow you to easily switch to a dialog driven interface.

Newgy-ised Control Script

list MENU_MAIN = ["Blue", "Red", "Green", "Yellow", "Purple", "White", "Off"]; 
list Colours        = ["<0.0,0.0,1.0>","<1.0,0.0,0.0>","<0.0,1.0,0.0>","<1.0,1.0,0.0>","<1.0,0.0,1.0>","<1.0,1.0,1.0>","Off"];
integer CHANNEL = -41; 
integer DialogChannel = 0;
integer Listening = 0;

// -------------------------- 
UpdateListen(key id) 
{ 
    CancelListen(); 
    DialogChannel = 0 - (integer)llFrand(2147483647);
    Listening = llListen(DialogChannel,"",id,""); 
    llSetTimerEvent(20); 
} 
// -------------------------- 
CancelListen() 
{ 
    if(Listening > 0) llListenRemove(Listening); 
    Listening = 0; 
    llSetTimerEvent(0); 
} 

default 
{ 
    state_entry() 
    { 
        CancelListen() ;
    } 
    
    listen(integer channel, string name, key id, string message) 
    { 
        CancelListen() ;
        integer index = llListFindList(MENU_MAIN, [ message ] );
         if(index >= 0)
        {
            string colourString = llList2String(Colours, index);
             llShout(CHANNEL, colourString);
        }
     } 

    touch_start(integer total_number) 
    { 
        key id = llDetectedKey(0);
        if (llSameGroup(id)) 
        { 
            UpdateListen(id);
            llDialog(id, "Studio Lights", MENU_MAIN, DialogChannel); 
        } 
        else 
        { 
            llWhisper(0,"Owner Access Only"); 
        } 
    } 
    
    timer()
    {
        CancelListen();
    }
} 

Newgy-ised Bulb Script

integer CHANNEL = -41; 

default 
{ 
    state_entry() 
    { 
        llListen(CHANNEL, "", NULL_KEY, ""); 
    } 
     
    listen(integer channel, string name, key id, string message) 
    { 
        if("Off" == message)
        {
            llSetPrimitiveParams([PRIM_POINT_LIGHT, FALSE, <1.0,1.0,1.0>, 1.0, 3.0, 0.5]); 
            llSetColor(<1.0,1.0,1.0>, ALL_SIDES);
        }
        else
        {
            vector v = (vector)message;
            llSetPrimitiveParams([PRIM_POINT_LIGHT, TRUE, v, 1.0, 3.0, 0.5]); 
            llSetColor(v, ALL_SIDES);
        } 
    } 
}

Reason for using strings rather than vectors is simply that its more efficient, only one extraction from the list not an extraction and then a conversion from vector to string.

My standard practise is to generate a new channel number on each dialog, it stops you responding to stale requests.

Also be aware that teh llSameGroup test will only work if the item has been deeded or at least given a group and the person touching it is wearing the group tag. An undeeded/ungrouped item will respond to anyone not wearing a tag.

HtH

Also, if the lamps are a single object, you could consider replacing: llShout(41, message);
for: llMessageLinked(LINK_SET, 0, message, NULL_KEY);

And then in the bulb, replacing: listen(integer channel, string name, key id, string message)
for: link_message(integer sender_num, integer num, string message, key id)

(no 'listen' code is then required in the bulb)


Although, as you're shouting, I guess the base is a long way away from the bulb?

Wow

Thanks for all the suggestions and help. I knew there would be a more efficient way to do this.

As for "why" I did certain things - well - it was the only way I knew to do them, from looking at other scripts and trying to figure things out.

The Linked message idea is probably a good one - the "bulbs" are actually part of the same object the control script it in. I was unsure of how to make just the bulbs change without changing the whole object. But I kinda understand how the link-message idea works. Will definitely give that a try.

I have seen other scripts using lists as suggested. I was always kinda confused by them. But, this example makes it a little clearer to me - seeing my script converted to using the lists makes it easier to understand.

Thanks again for all the help. Sometimes I feel very stupid when trying to figure out the whole scripting thing

Thanks,
Sparkly

Don't be silly. Everyone begins as a beginner.

my 2 L$

bulb script

default
{
    link_message(integer sender_id, integer chan, string data, key id)
    {
        if (chan == 99)llSetPrimitiveParams([23, 0, <1.0,1.0,1.0>, 0, 0, 0]);
        
         else llSetPrimitiveParams([23, 1, (vector)data, 1.0, 3.0, 0.5]);
    }
}

control script

list color_data =[
"red"    , "<1.0,0.0,0.0>" ,
"blue"   , "<0.0,0.0,1.0>" ,
"green"  , "<0.0,1.0,0.0>" ,
"yellow" , "<1.0,1.0,0.0>" , 
"purple" , "<1.0,0.0,1.0>" ,
"white"  , "<1.0,1.0,1.0>" ];

default
{
    state_entry()
    {
        llListen(41, "", NULL_KEY, ""); 
    }

    listen(integer channel, string name, key id, string message)
    {
        string input = llToLower(message);
        
        if ( id == llGetOwner() || llSameGroup(id) == TRUE)
        {
            if (input == "off") llMessageLinked(-1, 99,"", NULL_KEY);
            
            else 
            {
                integer search = llListFindList(color_data, [input]);
                
                if (search == -1) llWhisper(0,"invalid color");
                
                else
                {
                    string output = llList2String(color_data, (search + 1) );
                    llMessageLinked(-1, 0 ,output, NULL_KEY);
                }
            }
        }
        
        else llWhisper(0,"you donot have access");
    }
}

llDialog controled version

bulb script is the same as above

control script

integer lc = -878; //Listen channel
key  current_user = NULL_KEY;
integer listencon;

list color_data =[
"red"    , "<1.0,0.0,0.0>" ,
"blue"   , "<0.0,0.0,1.0>" ,
"green"  , "<0.0,1.0,0.0>" ,
"yellow" , "<1.0,1.0,0.0>" ,
"purple" , "<1.0,0.0,1.0>" ,
"white"  , "<1.0,1.0,1.0>" ,
"off"];

offline()
{
    llListenRemove(listencon);
    current_user = NULL_KEY;
}

default
{
    touch_start(integer na)
    {
        key user = llDetectedKey(0);
            
        if (current_user == NULL_KEY)
        {
            if  (user == llGetOwner() || llSameGroup(user) == TRUE)
            {
                current_user = llDetectedKey(0);
                listencon = llListen(lc,"",current_user,"");
                list buttons = llList2ListStrided(color_data, 0, -1, 2);
                llDialog(llDetectedKey(0),"please choose", buttons, lc);
                llSetTimerEvent(30);
            }
            
            else llWhisper(0,"sorry, but you dont have access");
         }
         
         else llWhisper(0, "busy");
    }

    listen(integer channel, string name, key id, string message)
    {
        string input = llToLower(message);
        if (id == current_user)
        {
            if (input == "off") llMessageLinked(-1, 99,"", NULL_KEY);
        
            else
            {
                integer search = llListFindList(color_data, [input]);
            
                if (search == -1) llWhisper(0,"invalid color");
            
                else
                {
                    string output = llList2String(color_data, (search + 1) );
                    llMessageLinked(-1, 0 ,output, NULL_KEY);
                }
            }
        }
        
        offline();
    }
    
    timer(){offline();}
} 

and yea you gotta start somewhere, dont feel silly stupid or anything at least your asking how to do it right

Its only stupid if you don't ask.