Jump to content

Major Issue with Papyrus


markdf

Recommended Posts

I'm not sure if anyone else has reported this, but I just ran into while trying to figure out why Combat Fatigue doesn't work.

 

The == and != operators are not commutative in Papyrus.

 

If you're a programmer, you understand the significance of this. In many situations iit's not an issue, but there are cases where it can cause problems.

 

Here's what happens:

int var1 = 7
bool var2 = true

If var1 == var2       ;; Converts var2 to an int before comparison.
   DoWhatever()
ElseIf var2 == var1   ;; Converts var1 to a bool before comparison.
   DoSomethingElse()
EndIf

There's a very specific and extremely common situation where this becomes a MAJOR problem.

Actor player = Game.GetPlayer()

If None != player ;; Converts player to a NoneVar, ALWAYS RETURNS FALSE.
    UnreachableFunction()
ElseIf player != None   ;; Converts None to an Actor, returns true.
    Reachable()
EndIf

A lot of programmers use the (None == var) form, because in languages like Java and C it helps to catch certain kinds of errors. But in Papyrus it doesn't work.

 

Basically there are lots of scripts out there whose authors are being good and responsible and double-checking that their properties and variables aren't pointing to None. But they were raised on C or Java or something similar, so they did their comparisons wrong.

 

 

Long story short: the expression

None == Var

is ALWAYS true.

 

 

Here is an example from CombatFatigue:

Event OnInit()
    Utility.Wait(2.0)
    ActorRef = Self.GetActorRef()
    
    ;;; THIS WILL NEVER RUN. EVER.
    If None != ActorRef && !ActorRef.HasSpell(FatigueSpell) 
        ActorRef.AddSpell(FatigueSpell,false)
    EndIf
endEvent    
Link to comment

 

I never use this notation None == var.
 
Thanks for the good tip. :)
Now I will probably rework some scripts of mods.

 

In a lot of programming courses, the instructor will actually recommend that form. It's great in C and Java because it prevents this mistake:

if (myVar = null) {
    // oops...
} else if (null = myVar) {
    // This wont even compile now!!
}

So a lot of programmers do it...

Link to comment

 

 

I never use this notation None == var.
 
Thanks for the good tip. :)
Now I will probably rework some scripts of mods.

 

In a lot of programming courses, the instructor will actually recommend that form. It's great in C and Java because it prevents this mistake:

if (myVar = null) {
    // oops...
} else if (null = myVar) {
    // This wont even compile now!!
}

So a lot of programmers do it...

 

 

A proper compiler, with even a slight amount of proper coding, will catch the error anyway.  Variables should be declared before they're checked.  Using a single = is a declaration, not an equation.  A proper compiler will error out saying the variable already exists, plus some other likely errors (such as casting errors) since the function no longer makes sense.

 

Then again, Java never really did have a proper compiler, and is quite the mess...

 

 

 

Oh, and any instructor that recommends putting the null first should be fired.  Teaching a lazy method instead of proper practices leads to errors and/or messy code.

Link to comment

 

 

I never use this notation None == var.
 
Thanks for the good tip. :)
Now I will probably rework some scripts of mods.

 

In a lot of programming courses, the instructor will actually recommend that form. It's great in C and Java because it prevents this mistake:

if (myVar = null) {
    // oops...
} else if (null = myVar) {
    // This wont even compile now!!
}

So a lot of programmers do it...

 

 

 

As I said, I do not use this strange syntax. Honestly, I do not know how to get on such an idea.
 
But if there are people who program so, it would explain some entries in the log.
 
So far I have not had the idea to check scripts for this syntax. ;)
Link to comment

A proper compiler, with even a slight amount of proper coding, will catch the error anyway.  Variables should be declared before they're checked.  Using a single = is a declaration, not an equation.  A proper compiler will error out saying the variable already exists, plus some other likely errors (such as casting errors) since the function no longer makes sense.

 

Then again, Java never really did have a proper compiler, and is quite the mess...

 

 

 

Oh, and any instructor that recommends putting the null first should be fired.  Teaching a lazy method instead of proper practices leads to errors and/or messy code.

 

The problem is that putting an assignment in an IF statement is perfectly valid in C and many of it's descendants. And it's soooo easy to enter = instead == when you're going fast. Putting the null first turns that mistake into a compiler error so that it's easy to catch. Programming is all about lazy methods, in my opinion. Catching your bugs during compilation saves you tons of time and effort during unit testing. Catching your bugs during unit testing saves you tons of time and effort in application testing. The lazier, the better.

As I said, I do not use this strange syntax. Honestly, I do not know how to get on such an idea.
 
But if there are people who program so, it would explain some entries in the log.
 
So far I have not had the idea to check scripts for this syntax. ;)

 

So far the only mod I've found that has this problem is Combat Fatigue. FindStr may not be grep, but it still does a respectable job.

 

Link to comment

 

A proper compiler, with even a slight amount of proper coding, will catch the error anyway.  Variables should be declared before they're checked.  Using a single = is a declaration, not an equation.  A proper compiler will error out saying the variable already exists, plus some other likely errors (such as casting errors) since the function no longer makes sense.

 

Then again, Java never really did have a proper compiler, and is quite the mess...

 

 

 

Oh, and any instructor that recommends putting the null first should be fired.  Teaching a lazy method instead of proper practices leads to errors and/or messy code.

 

The problem is that putting an assignment in an IF statement is perfectly valid in C and many of it's descendants. And it's soooo easy to enter = instead == when you're going fast. Putting the null first turns that mistake into a compiler error so that it's easy to catch. Programming is all about lazy methods, in my opinion. Catching your bugs during compilation saves you tons of time and effort during unit testing. Catching your bugs during unit testing saves you tons of time and effort in application testing. The lazier, the better.

 

 

The only valid and acceptable case I can think of to declare a variable within an if() statement is if you're explicitly checking if the variable was declared successfully - the declaration itself should return a bool true/false.  For all other use cases, the proper practice is to declare the variable first, then perform an operation on it, not declare it in the middle of an operation on itself!

 

Also, there is a subtle difference between lazy and efficient.  ;)

Link to comment

The only valid and acceptable case I can think of to declare a variable within an if() statement is if you're explicitly checking if the variable was declared successfully - the declaration itself should return a bool true/false.  For all other use cases, the proper practice is to declare the variable first, then perform an operation on it, not declare it in the middle of an operation on itself!

 

Also, there is a subtle difference between lazy and efficient.   ;)

 

Oh, I absolutely agree that you shouldn't do assignments in an if statement. The problem is that you can do it, and the compiler will let you because it's technically valid. And then you accidentally type = instead of == in your condition, and it compiles fine and the if statement has a nasty tendency to work perfectly so the error is hard to track down. That's why null==var (or constant==var) is useful -- it catches that error.

Link to comment
  • 1 year later...
40 minutes ago, magodelaoscuridad said:

So this is bad?

 

Event OnInit()
    FollowerID = new int[6]
    DefalutFollowerID()
    if self.GetActorRef() != none
    RegisterForAnimationEvent(self.GetActorRef(), "SoundPlay.NPCHorseMount")
    RegisterForAnimationEvent(self.GetActorRef(), "SoundPlay.NPCHorseDismount")
    endif
EndEvent

...

if self.GetActorRef()

....

endIf

...

 

is enough.

 

That (a == none) or (a != none) is a trick from languages that have "pointers", memory address. Asembler, low level C and stuff.

Papyrus has no such thing (memory address), so using that (== none) is just wrong use of language.

 

I mean if you use Papyrus, do it Papyrus style. If you use C, do it C style. Don't mix. Only bad things can happen.

 

Link to comment
  • 2 weeks later...

Archived

This topic is now archived and is closed to further replies.

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue. For more information, see our Privacy Policy & Terms of Use