Aller au contenu

Photo

Can this line of code be written cleaner?


  • Veuillez vous connecter pour répondre
9 réponses à ce sujet

#1
Orion7486

Orion7486
  • Members
  • 161 messages
Here's the line:

 if((GetLevelByclass(2, oPC) > 0) && ((GetDeity(oPC) != "**") && (GetDeity(oPC) != "^^")))

This will return true if the PC is a cleric, and also if his deity is NOT either ** or ^^.

Thanks for any responses.

#2
M. Rieder

M. Rieder
  • Members
  • 2 530 messages
That script will return true if the deity is not ** AND ^^. You need to use || if you want it to return true if deity is not ** or ^^.

I usually put the different conditions on different lines to keep them more organized and to keep my conditionals from stretching way out across the page. So I would write it like this:


if((GetLevelByclass(2,oPC)>0) &&
((GetDeity(oPC)!= "**") || (GetDeity(oPC)!="^^")))

That will return true if the PC is a cleric, and also if his deity is not either ** or ^^.

Modifié par M. Rieder, 15 avril 2011 - 03:28 .


#3
Orion7486

Orion7486
  • Members
  • 161 messages

M. Rieder wrote...

That script will return true if the deity is not ** AND ^^. You need to use || if you want it to return true if deity is not ** or ^^.

That's what I thought at first, too.  I had || there.  If the PC was a cleric but the deity was **,  that convo line didn't show.  Apparently, the script engine doesn't read it as an english reader would read it.  I put in && and the line showed up when the PC was a cleric and deity was **.
I think with the || in there, the engine reads it not as a combined choice but reads it twice. 
    If PC is cleric and deity isn't **, If PC is cleric and deity isn't ^^.
The script engine doesn't  read the line  "if PC is cleric and deity isn't **", and if that parts true, then stop and declare it true.  It will read the line again with the other or part.  So in effect with the || in there, that line can never be declared true.

#4
MasterChanger

MasterChanger
  • Members
  • 686 messages
If you want to run that portion of script only if the deity is neither "**" nor "^^", then yes, you do need to use the AND. Otherwise, you're saying that it can be "**" or "^^" as long as it's not both. Clearly it's not going to be both, so every cleric would pass that check.

The most basic way to work it out conceptually would probably be to nest the statements:

if (GetLevelByclass(2, oPC) > 0)
{
    if (GetDeity(oPC) != "**")
    {
         if (GetDeity(oPC) != "^^")
         {
             // All other clerics allowed past the velvet rope
          }
     }
}


Every nested "if" can be replaced with an AND in a compound statement. If you wanted an either-or choice, it would look like this:

if (GetLevelByclass(2, oPC) > 0)
{
    if (GetDeity(oPC) != "**")
    {
        // We allow "^^" here, but not those New Age "**" types.
    }
    
    else if (GetDeity(oPC) != "^^")
    {
        // We allow "**", but not those stick-in-the-mud "^^" types.
     }
}


The "else" can be represented by an OR in a compound statement. Technically the "else" is not required above--you can just have an "if" and then another "if" not nested--but it makes things clearer for our example.

Hopes this helps.

#5
Orion7486

Orion7486
  • Members
  • 161 messages
Thanks for the insights, guys.

#6
rjshae

rjshae
  • Members
  • 4 507 messages
You could also save the return value of the GetDeity call in a variable then do string compares against that variable. That way you only call GetDeity once. (Not sure if the NWN2 script engine has an optimizer that takes care of that issue.)

#7
MasterChanger

MasterChanger
  • Members
  • 686 messages

rjshae wrote...

You could also save the return value of the GetDeity call in a variable then do string compares against that variable. That way you only call GetDeity once. (Not sure if the NWN2 script engine has an optimizer that takes care of that issue.)


I was going to mention this but decided not to open up the issue. I usually store the result of a function in a variable if I'm going to check it even twice, but some people use different cut-offs, I suppose. Of course, if the function is a doozy to run, or if the function calls happen far enough apart such that there's any possibility the returned value will change, then it's definitely better to store the returned value in a variable.

I'll also store the value if it takes up too much screen real-estate to nest the functions. Of course you can SendMessageToPC or SetLocalString with a ton of nested functions defining the string, but who can read that? A compiler might be able to, but it's harder for a human being.

In the OP's example case, repeating the calls is not so terrible.

#8
Morbane

Morbane
  • Members
  • 1 883 messages
Just my 2 cents worth: MC's Velvet Rope method looks the cleanest.

#9
Kaldor Silverwand

Kaldor Silverwand
  • Members
  • 1 598 messages
Oddly the forum seems to insist on making the word "class" into all lower case even when included in a function name like GetLevelByclass. Personally, I would code it like this (understanding that class is capitalized in GetLevel Byclass and all caps in class_TYPE_CLERIC):

 if ( (GetLevelByclass(class_TYPE_CLERIC, oPC) > 0) && (!(GetDeity(oPC) == "**" || GetDeity(oPC) == "^^")) )

Regards

Modifié par Kaldor Silverwand, 17 avril 2011 - 01:37 .


#10
Orion7486

Orion7486
  • Members
  • 161 messages
Yeah, I was going to mention that despite your clarification on the cap C, you did the same:)
I went with splitting the cleric line, then nesting the double GetDeity line under the cleric line.