Syle Devin Posted February 1, 2013 Posted February 1, 2013 I have a script that changes color from an HUD. It starts listening on click and will detect what face you touched so it knows where to change the color. The problem is that I cannot get the face integer to do anything, it will detect what face you touched but still only change the color of face 0 and not the one it said you touched. integer face; integer listen_handle; float gap = 30.0; default { state_entry() { } changed(integer change) { if (change & CHANGED_COLOR) { llSetTimerEvent(gap); //llSay(PUBLIC_CHANNEL, "Inactivity Timer Reset"); } } touch_start(integer total_number) { key owner = llGetOwner(); key touchingAvatar = llDetectedKey(0); integer face = llDetectedTouchFace(0); llSay(PUBLIC_CHANNEL, "You touched face number " + (string) face); if (owner == touchingAvatar) listen_handle = llListen(-5872695687565, "", NULL_KEY, "" ); //llSay(PUBLIC_CHANNEL, "Started Listening"); llSetTimerEvent(gap); } timer() { // llSay(PUBLIC_CHANNEL, "Due to innactivity the script has stopped listening."); llListenRemove(listen_handle); llSetTimerEvent(0.0); return; } listen( integer channel, string name, key id, string message ) { if (llToLower(message) == "finished") { llListenRemove(listen_handle); llSetTimerEvent(0.0); return; } if (llToLower(message) == "green1") { llSetColor(<0.22353, 0.70980, 0.29020>, face); return; } if (llToLower(message) == "green") { llSetColor(<0.00000, 0.58039, 0.11765>,face); return; } if (llToLower(message) == "green-1") { llSetColor(<0.00000, 0.36863, 0.12549>,face); return; } } } My script works but it doesn't change the correct face color. Is there anyone that would be able to help me figure out how to have the integer read? I've searched and tested to the best of my abilities and still not able to figure it out.Thanks for your time and if you are able to help :)
Dora Gustafson Posted February 1, 2013 Posted February 1, 2013 This line specifies a local variable different from rhe global with the same name integer face = llDetectedTouchFace(0); try this instead: face = llDetectedTouchFace(0); :smileysurprised::smileyvery-happy: 1
Syle Devin Posted February 1, 2013 Author Posted February 1, 2013 Ah so I wasn't actually telling the whole script what the integer face equaled? I tried what you suggested and couple of other things to no avail. When I switched the integer face; with the face = llDetectedTouchFace(0); I got a syntax error right before the =. I can think of a different way that the script can be done but it would require a lot more code. I hope the script can still be made to work the way I have it.
Dora Gustafson Posted February 1, 2013 Posted February 1, 2013 That change should certainly not introduce a syntax error. A typo must have sneaked in. Read what you wrote carefully, I can't tell without seeing the script :smileysurprised::smileyvery-happy: Edit: it couldn't be that you changed the global variable to face = llDetectedTouchFace(0); ??? Do not touch the way the global variable was from the beginning, just remove integer in the touch event, changeinteger face = llDetectedTouchFace(0);toface = llDetectedTouchFace(0); :smileysurprised::smileyvery-happy: 1
Innula Zenovka Posted February 1, 2013 Posted February 1, 2013 Dora's code certainly works. All you need to do is remove the word "integer" in the touch_start event (line 25). That's where the problem is.
Syle Devin Posted February 1, 2013 Author Posted February 1, 2013 I was doing it wrong then I did try to change the global variable but that was already correct. I again removed what you said and it worked! Thanks much! Do you mind me asking why I might not need integer infront of face when it is after the touch start? In the test script in the lsl wiki they have integer face = llDetectedTouchFace(0); but then again they don't have a global variable in that script. Is that what makes the difference?
Innula Zenovka Posted February 1, 2013 Posted February 1, 2013 If you declare a global variable -- integer face; before the default state -- then it's available thoughout the script. However, declaring a local variable of the same name -- which you did in the touch_start event -- cancels that out, in effect overwriting it, since you can only have one variable with a particular name, so you end up with a local variable that's only available in the touch_start event.
Ela Talaj Posted February 1, 2013 Posted February 1, 2013 Don't write code like this. It is a very bad style. listen( integer channel, string name, key id, string message ) { if (llToLower(message) == "finished") { llListenRemove(listen_handle); llSetTimerEvent(0.0); return; } if (llToLower(message) == "green1") { llSetColor(<0.22353, 0.70980, 0.29020>, face); return; } if (llToLower(message) == "green") { llSetColor(<0.00000, 0.58039, 0.11765>,face); return; } if (llToLower(message) == "green-1") { llSetColor(<0.00000, 0.36863, 0.12549>,face); return; } } Should be: listen( integer channel, string name, key id, string message ) { if (llToLower(message) == "finished") { llListenRemove(listen_handle); llSetTimerEvent(0.0); } else if (llToLower(message) == "green1") { llSetColor(<0.22353, 0.70980, 0.29020>, face); } else if (llToLower(message) == "green") { llSetColor(<0.00000, 0.58039, 0.11765>,face); } else if (llToLower(message) == "green-1") { llSetColor(<0.00000, 0.36863, 0.12549>,face); } }
Innula Zenovka Posted February 1, 2013 Posted February 1, 2013 What's wrong with using return rather than a series of else if's? I would normally write it the way you do, and not use returns, but that's just because it's how I'm used to seeing it done. What's the disadvantage to doing it the other way?
Syle Devin Posted February 2, 2013 Author Posted February 2, 2013 Does removing the return change how the script runs? Knowing HTML more than I do lsl it sounds like one of those things that is ok but is looked badly upon because it would be unnecessary code. If it really is unnecessary for the effect I want. If i don't need it there then I'll remove it, less code to worry about, but if it changes the script then I will leave it.
Innula Zenovka Posted February 2, 2013 Posted February 2, 2013 In this context, return exits the event. That is, as soon as the script finds a condition that evaluates as TRUE, it will change the colour and then stop and wait for another event to be triggered. Ela's way -- which is how I was taught to do it, too -- of using if .. else if ... else if... amounts to the same thing, at least in your code it does. That is, if you have a series of if.. else if... else if it will, on finding the first TRUE condition, stop evaluating the rest of the list. If there was something else in the listen event after this list of "if .. else if.." it would go on and do that, but since there isn't anything else to do, to my mind it amounts to the same as using return. The difference, I suppose, is that if there's anything you want to happen later on in the listen event, after you've changed the colour, you shouldn't be using return. And I suppose it could well be argued that it's a bad habit to get into because, if later on you need to add something else later on in the listen event, you would then have to remember to go back and take out all the returns. If would certainly be wrong to have a series of if... if.. if because the difference between if ... if ... if and if.. else if .. else if .. is that in the case of if.. if.. if.. every if condition will be evaluated. You don't want that to happen in this script because you know in advance that only one condition can be true, so you don't want to waste time and resouces evaluating anything that comes after the first true condition, since you know in advance everying after the first true condition will be false.
LepreKhaun Posted February 2, 2013 Posted February 2, 2013 There's also the chance of using "return value", which would compile w/o any problem but, since it is within an event and not a function, would throw a runtime error.
Ela Talaj Posted February 3, 2013 Posted February 3, 2013 There is no difference in execution. Moreover, in most C/C++ compilers (though not all) you'd get exactly the same assembly code. It's as I said originally a matter of programming style. Most software companies would have programming style rules handbooks where they codify even a number of spaces curly brackets should be positioned at in respect to the previous lines The idea is mostly to prevent typos and missing things. In our specific case you are not going to forget "else if" and if you do a compiler will remind you, but it is easy to forget "return", which may lead to a whole program being executed somewhat differently than you think and in complex systems with many modules and thousands of line of code in each bugs like this are hell to find. (Of course in the real world no one going to write a lot of "else if" but would use a switch statement.)
161488303349 Posted February 7, 2013 Posted February 7, 2013 just on coding style then with these kinda things can use a lookup to avoid whole bunches of if else if it also makes it easy to add/remove colors (or whatever) without having to mod the code something like: list colors = [ "green1", <0.22, 0.71, 0.29>, "green", <0.00, 0.58, 0.12>, "green-1", <0.00, 0.37, 0.13>, "finished"];integer face;listen ( integer channel, string name, key id, string message ){ integer pick = llListFindList(colors, [llToLower(message)]); if (pick == llGetListLength(colors) - 1) // then finish { llListenRemove(listen_handle); llSetTimerEvent(0.0); } else if (pick >= 0) // then is valid message llSetColor(llList2Vector(colors, pick + 1), face); } ps. I not test this example. just done it off the top my head. but I think is ok
Recommended Posts
Please take a moment to consider if this thread is worth bumping.
Please sign in to comment
You will be able to leave a comment after signing in
Sign In Now