Check over script Please

[tagigan]

G'Day,

If possible could someone check over this script and tell me where I have gone wrong.

It's for a Billboard with changing messages at defined intervals. a sequence of 5, displaying for a pre-determined length of time.

eg: seconds, Hours or changing daily.

 

Secondly. CS wouldn't recognise "getEnabled" . It has worked in previous mods, But the timer part wasn't involved.

Just to confirm: I have no scripting knowledge. I just bastardize (no it's not rude) bits and pieces from other places and try to see if they work.

 

Thanks, Tagigan

=======================================================================

scn aemBillboardSCRIPT

 

float Timer

short FirstSecond

short State

 

Begin GameMode

 

set state to 1

 

if ( State == 1 )

if Timer > 120 || FirstSecond == 0

set Timer to 0

set FirstSecond to 1

if (ad01ref.getDisabled == 1)

(ado1aref.getDisabled == 1)

ad01ref.enable

ad01aref.enable

else (ad01ref.getEnabled == 1)

(ado1aref.getEnabled == 1)

ad01ref.disable

ad01aref.disable

Set State to 2

endif

endif

endif

 

if ( State == 2 )

if Timer > 120 || FirstSecond == 0

set Timer to 0

set FirstSecond to 1

if (ad02ref.getDisabled == 1)

(ado2aref.getDisabled == 1)

ad02ref.enable

ad02aref.enable

else (ad02ref.getEnabled == 1)

(ado2aref.getEnabled == 1)

ad02ref.disable

ad02aref.disable

Set State to 3

endif

endif

endif

 

if ( State == 3 )

if Timer > 120 || FirstSecond == 0

set Timer to 0

set FirstSecond to 1

if (ad03ref.getDisabled == 1)

(ado3aref.getDisabled == 1)

ad03ref.enable

ad03aref.enable

else (ad03ref.getEnabled == 1)

(ado3aref.getEnabled == 1)

ad03ref.disable

ad03aref.disable

Set State to 4

endif

endif

endif

 

if ( State == 4 )

if Timer > 120 || FirstSecond == 0

set Timer to 0

set FirstSecond to 1

if (ad04ref.getDisabled == 1)

(ado4bref.getDisabled == 1)

ad04ref.enable

ad04bref.enable

else (ad04ref.getEnabled == 1)

(ado4bref.getEnabled == 1)

ad04ref.disable

ad04bref.disable

Set State to 5

endif

endif

endif

 

if ( State == 5 )

if Timer > 120 || FirstSecond == 0

set Timer to 0

set FirstSecond to 1

if (ad05ref.getDisabled == 1)

(ado5aref.getDisabled == 1)

ad05ref.enable

ad05aref.enable

else (ad05ref.getEnabled == 1)

(ado5aref.getEnabled == 1)

ad05ref.disable

ad05aref.disable

Set State to 1

endif

endif

endif

set Timer to Timer + GetSecondsPassed

End

[nosisab]

Please reformat the code and place it under "code" "/code" (substitute the quotes by the brackets) . There is many repeated lines that should not exist too.

 

I don't know if GetEnabled exist, I'm sure GetDisabled does and does the trick (GetEnabled becomes redundant). What exist is the "Enable" function.

 

Without the reformatting one can just speculate about the code.

 

To be on the sure side, place the second if conditions in each block inside parenthesis.

 

I believe than following the above, yourself will find what is wrong.

 

PS: care about the indentation to ease the comprehension but do not rely on it. Make sure it's correct too.

[Vagrant0]

Get rid of the parenthesis. they should only be used in math functions. This is not C++, and although it can still work right in some cases, adds unnecessary characters to the script to be parsed and can occasionally screw up things which would otherwise work.

 

if (ad02ref.getDisabled == 1)
(ado2aref.getDisabled == 1)

is not only bad code syntax, but is redundant. It should be

if ad02ref.getDisabled == 1

and be left to just that.

Edited March 4, 2010 by Vagrant0

[nosisab]

Get rid of the parenthesis. they should only be used in math functions. This is not C++, and although it can still work right in some cases, adds unnecessary characters to the script to be parsed and can occasionally screw up things which would otherwise work.

 

if (ad02ref.getDisabled == 1)
(ado2aref.getDisabled == 1)

is not only bad code syntax, but is redundant. It should be

if ad02ref.getDisabled == 1

and be left to just that.

Sorry but I think this advice is dangerous. The simple examples above works without problems but the complex or/and/not combinations may lead to issues with the order the 'compiler' takes them. The parenthesis usage grants the final result (that is what matters for the If) is that we want.

 

The parsing occurs only in compiling time and the certainty about what is being computed is enough to make the parenthesis usage good code writing. More yet it helps turning the conditions easier to understand and granting the order they are computed is the same we wish.

[Vagrant0]

Sorry but I think this advice is dangerous. The simple examples above works without problems but the complex or/and/not combinations may lead to issues with the order the 'compiler' takes them. The parenthesis usage grants the final result (that is what matters for the If) is that we want.

 

I beg to differ. When using mixed and/or/not cases, you're almost always best off putting those as separate conditions due to how the script is processed.

 

if getstage MQ05 == 100 && player.getinworldspace Tamriel == 1 || player.getinworldspace Bravilworld != 1

Would work fine without parenthesis, but would have a problem due to how the last condition player.getinworldspace Bravilworld != 1 would return true while in any interior, thus completing the condition. In order to fix the problem you wouldn't need to use parenthesis, but instead would just need to adjust the order conditions are checked. A fixed code could look like either one of the following:

if getstage MQ05 == 100 && player.isininterior != 1 && player.getinworldspace Tamriel == 1 || player.getinworldspace Bravilworld != 1

OR

if getstage MQ05 == 100 && player.getinworldspace Tamriel == 1 || player.getinworldspace Bravilworld != 1
if player.isininterior != 1

In the first case, all 4 conditions would be checked at the same time before preceeding. In the second case, the first 3 conditions would be checked first, and only then would the interior check take place.

 

Even something like

if getstage MQ05 == 100 && (player.getinworldspace Bravilworld == 1 && player.getcrimegold >= 100) ||  (player.getinworldspace Anvilworld == 1 && player.getcrimegold >= 50)
<stuff>

would process better as

if getstage MQ05 == 100
if player.getinworldspace Bravilworld == 1 && player.getcrimegold >= 100
Set variablename to 1
elseif player.getinworldspace Anvilworld == 1 && player.getcrimegold >= 50
Set variablename to 1
endif
if variablename == 1
<stuff>
endif

The reason being that everything within primary if conditions is checked every time that block runs. Meaning that if this were the primary level of a script condition, the script would constantly be checking all 5 conditions in the first case even if none of them are present, regardless of how they're grouped. Meanwhile in the second case, the queststage would be checked first, if that state was found, it would check the first group of secondary conditions individually (2, 2) then check the last secondary condition separately. This is why using long condition strings consisting of multiple functions is not a good idea for primary or secondary condition groups, and usually becomes unnecessary once all those primary and secondary conditions are factored. The script might be longer, but ends up having a smaller processing impact. When using some of the more demanding functions within a condition check, you should ALWAYS have them being controlled by a more generic condition even if redundant.

 

Even with math functions, it's almost always a good idea to assign these to a script variable instead of trying to perform that math as part of a condition check.

 

Regardless, in their usage here, only one condition is being used so parenthesis are unnecessary and only add to liability. As for how infrequent people might actually need to use parenthesis in conditions (most don't even make use of the OR statement due to how easily it can be misused) it serves better to use them when needed rather than as part of standard practice.

Edited March 4, 2010 by Vagrant0