Major Issue with Papyrus

*markdf* · January 4, 2017

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

Andy14 · January 4, 2017

I never use this notation None == var.

Thanks for the good tip.

Now I will probably rework some scripts of mods.

*markdf* · January 4, 2017

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...

davisev5225 · January 4, 2017

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.

Andy14 · January 4, 2017

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.

*markdf* · January 4, 2017

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.

davisev5225 · January 4, 2017

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.

*markdf* · January 4, 2017

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.

magodelaoscuridad · August 26, 2018

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

Fotogen · August 26, 2018

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.

magodelaoscuridad · August 28, 2018

What about a Logic AND:

if Caster != None && akSource == Caster

...

endIf

how make that?

cminnow · September 5, 2018

Did anyone ever figure out why Combat fatigue puts you in 3rd person? Is there a way to disable that?

Ruffled Pigeon · September 5, 2018

On 8/28/2018 at 1:05 PM, magodelaoscuridad said:

What about a Logic AND:

if Caster != None && akSource == Caster

...

...

endIf

how make that?

if Caster && akSource == Caster

Sign In

Major Issue with Papyrus

Recommended Posts

Archived

Recently Browsing 0 members