Critique My First Script

[EvelynBianchi]

It's been a few days now since I started reading the documentation on the LSL. I thought about halting this journey many times, but I'm happy that I've finally decided to learn this language. I hope to receive much criticism from many of you, so that I can better myself as I continue learning the LSL. :smileyvery-happy:

 

/*************************************************
*  Scripted by EvelynBianchi, also known as Lyn. *
*  Date: 15/gennaio/2016                        * 
*  Description: Simulates a two-way switch.      *
*************************************************/

// Colour Palette
vector BIANCO = <1.0, 1.0, 1.0>;        // WHITE
vector GRIGIO = <0.667, 0.667, 0.667>;  // GRAY
vector BLU = <0.0, 0.45, 0.851>;        // BLUE

float OPAQUE = 1.0;

integer gSwitch = TRUE;  // Switch is on.

default
{
    on_rez(integer param)
    {
        llResetScript();
    }
    
    state_entry()
    {
        llSetColor(BIANCO, ALL_SIDES);
        llSetText("On.", BLU, OPAQUE);
    }
    
    touch_start(integer num)
    {
        gSwitch = !gSwitch;
        
        if (gSwitch) {
            llSetColor(BIANCO, ALL_SIDES);
            llSetText("On.", BLU, OPAQUE);
        }
        else {
            llSetColor(GRIGIO, ALL_SIDES);
            llSetText("Off.", BLU, OPAQUE);
        }
    }
}

 

[steph Arnott]

What do you want me to critical about, that script?

[Rolig Loon]

That's not a bad first try, Evelyn. It's a straightforward toggle switch.  If you're looking for ways to "improve" it, I'd suggest:

1.  Taking almost all of your global variables, except for gSwitch, and putting those values directly into the appropriate commands instead.  They will certainly work where they are, but there's no compelling reason to make them global.  Basically, you should make a variable global if (1) you will be changing its value each time you trigger an event or (2) you will be needing to pass its value from one event or state to another one. There's little to be gained by declaring global constants unless (1) you will be using them in many places -- a dozen ior more, maybe? -- in the script or (2) you want to have the value right at the top of the script, where it is easy to change if you use the script for different purposes. I tend to define a channel number as a global, for example, because I don't want to have to search through the script to find it six months later.

2. Making your gSwitch variable FALSE initially, not TRUE.  As it is, your object's default condition is ON == WHITE == TRUE, which is the reverse of what most people would expect. The first time you click on the object, your statement gSwitch != gSwitch will turn the object OFF==GRAY==FALSE.

Both of those are stylistic suggestions, not things that will affect the logic of your work in any substantial way.  That's why I said that they are things you could do to "improve" it, not things to "fix" it.

Congratulations on your first script.  Welcome to the club.

[Ron Khondji]

The date is wrong.

[EvelynBianchi]

I wrote the script yesterday evening.

[steph Arnott]

They are refering to marzo which is march.

[EvelynBianchi]

Thank you for the suggestions, Rolig! I will definitely take them into account. :smileyhappy:

[EvelynBianchi]

Wow! I cannot believe I typed that. :smileyfrustrated: Grazie mille, Steph. I'm editing the mistake now.

[Qie Niangao]

---

Rolig Loon wrote:

1.  Taking almost all of your global variables, except for gSwitch, and putting those values directly into the appropriate commands instead.  They will certainly work where they are, but there's no compelling reason to make them global.  Basically, you should make a variable global if (1) you will be changing its value each time you trigger an event or (2) you will be needing to pass its value from one event or state to another one. There's little to be gained by declaring global constants unless (1) you will be using them in many places -- a dozen ior more, maybe? -- in the script or (2) you want to have the value right at the top of the script, where it is easy to change if you use the script for different purposes. I tend to define a channel number as a global, for example, because I don't want to have to search through the script to find it six months later.

---

This is more subtle than a new scripter may appreciate at first. Folks who've programmed in languages with "real" pre-processor or compile-time constants will have adopted a rule that few if any literal values should ever appear in the code itself, but rather be defined "up front" or in #include files. The problem is, LSL doesn't have real constants, and using variables as if they were constants does incur (small) memory and (tiny) performance penalties. Thing is, with memory so very limited as it is in LSL scripting, even tiny differences may matter as scripts grow.

I don't especially recommend this, but I often list commented-out "constant" definitions at the top of a script and then also flag everywhere the associated value is used with a comment mentioning the same "constant", so I can find and change all those places when a different value should be used. (Admittedly that's being a little obsessive about memory conservation, which is why I don't necessarily recommend it, but it illustrates the point.)

[ETA: The script looks fine to me, as-is. I wouldn't have even noticed the global variable "constants", all my above rambling notwithstanding.]

[Rolig Loon]

That's a good clarification, Qie. Scripters with prior experience in languages that are not as tight on memory as LSL can afford to be lazy about watching how much of it their code takes.  In fact, most LSL scripts don't run up against the memory limit either, but it's wise to get in the habit of optimizing memory use even with short scripts like this one.  By the time you're writing scripts that run to several hundred lines of code, the habit will be second nature.  Then you can start worrying about how to chop those mega-scripts into meaningful smaller chunks .....

[Dora Gustafson]

I can't believe this is your first script, so well polished
There must be quite a few more first scripts in your waste bin

:smileysurprised::smileyvery-happy:

[steph Arnott]

There is more than enough memory alllowed now, you have 64K which in SL is huge. You can also have satalite scripts. I for one have never even got close to a stack heap crash or a memory limit. Most SL scripts do not even get close to 10K.

[Rolig Loon]

---

steph Arnott wrote:

There is more than enough memory alllowed now, you have 64K which in SL is huge. You can also have satalite scripts. I for one have never even got close to a stack heap crash or a memory limit. Most SL scripts do not even get close to 10K.

---

Your time will come, Steph. :smileytongue:  I hit that 64K limit often enough that it's usually somewhere on my mind, so I need to plan ways divide large scripts into smaller ones that communicate with link messages when that happens.  It's also possible to hit the limit if a script needs to store a lot of data, since LSL is not designed for that sort of thing. It's less of a problem now that we can script KVP to handle some of those data storage issues, but that's only a solution if we are writing scripts to be used in an Experience.

[steph Arnott]

thats becouse that script was writen nearly 5 years ago, i have the same one in my archive.

[steph Arnott]

I set a memory cut out limit that passes to a sub script.

[EvelynBianchi]

I'm not sure if your insulting me. I wrote this script myself using the online documentation. I've written programs like this in other languages before. I was planning on posting a different script I was working on, but it wasn't finished, and I really just wanted to be involved in this community.

[steph Arnott]

Not insulting you at all, if you say you wrote it upto you, just have exactly the same script in my archive when i started in SL.

Spelling

[Wulfie Reanimator]

You're already doing a good job at formatting your code properly!

I'm noticing that you use all-caps for "constant" variables, which aren't meant to be changed. If you're using the Firestorm viewer, you could enable the pre-processor (Gears icon above the code area > Enable LSL preprocessor) in order to not spend memory assigning global variables that only ever have one value. Basically, you'd do this instead:

 

#define BIANCO <1.0, 1.0, 1.0>#define GRIGIO <0.667, 0.667, 0.667>#define BLU <0.0, 0.0, 0.851>#define OPAQUE 1.0integer gSwitch = TRUE; // Switch is on.default[...]

This way, whenever you type BIANCO into your code, the pre-processor replaces it with the text <1.0, 1.0, 1.0>.
You can read more about it here if you're interested, it has even more helpful uses, but that would be most fitting for you:
http://wiki.phoenixviewer.com/fs_preprocessor

 P.S. This is completely optional and nothing you need to concern yourself with as a beginner.

[EvelynBianchi]

I didn't know I could define constants as I would in C. Thank you, Wulfie.