Optimal If Statement Question

[octodecoy]

Is it better to check conditions with :

 

"Most likely to succeed > (and check subsequent conditions in the same way if applicable) > Return ; "

 

Or with

 

"Most likely to fail > (and check subsequent conditions in the same way if applicable) > Return ; "

 

Or does it just depend on the situation, or negligible?

[YouDoNotKnowMyName]

It could make a tiny difference in terms of timing.

The "most likely to succeed" approach would be a little bit faster most of the time. But I don't think that that tiny bit of difference could be noticed when actually using the script in the game.

 

But I am not an expert in scripting for FO4.

[SKKmods]

It is always good practice to try and be elegant so your solutions can scale with the principle is to pass or fail fast with minimum resources (as many delayed native script functions take one display frame to execute).

 

The priority depends of your doing pass or fail validations.

 

This is is a significant chunk which has been performance profiled for fail fast with minimum execution budget for a normal distribution of up to 1000 potential input actors (all setters in all workshops);

 

 

 

Bool bInclude = TRUE

If (ThisWorkshop == None) ;zero frames
   bInclude = FALSE
ElseIf ((ThisWorkshop as WorkshopScript).OwnedByPlayer == FALSE) ;zero frames
   bInclude = FALSE
ElseIf ((ThisActor Is WorkshopObjectActorScript) == TRUE) ;Turret zero frames
   bInclude = FALSE
ElseIf ((ThisActor.Haskeyword(pActorTypeRobot) == TRUE) && (pSKK_CSETIncludeRobots.GetValue() == 0)) ;zero frames
   bInclude = FALSE
ElseIf ((ThisActor.Haskeyword(pActorTypeChild) == TRUE) && (pSKK_CSETIncludeChildren.GetValue() == 0))  ;zero frames
   bInclude = FALSE
ElseIf (((ThisActor Is companionactorscript) == TRUE) || ((ThisActor Is dogmeatactorscript) == TRUE)) && (pSKK_CSETIncludeCompanions.GetValue() == 0) ;zero frames
   bInclude = FALSE
ElseIf (ThisActor.IsInFaction(pCurrentCompanionFaction) == TRUE) ; 1 frame
   bInclude = FALSE
ElseIf (ThisActor.IsInFaction(pDomesticAnimalFaction) == TRUE) ;Dog or Brahmin 1 frame
   bInclude = FALSE
ElseIf (ThisActor.ISDisabled() == TRUE) || (ThisActor.ISDead() == TRUE) ; 2 frames
   bInclude = FALSE
EndIf 

 

 

 

Before optimisation with a random order it could take 5 minutes to execute, after is 90 seconds. That's significant.

[octodecoy]

Thank you for both the details and the example.

[ReDragon2013]

SKK50 related post:

 

  GlobalVariable PROPERTY pSKK_CSETIncludeCompanions auto
  GlobalVariable PROPERTY pSKK_CSETIncludeRobots     auto
  GlobalVariable PROPERTY pSKK_CSETIncludeChildren   auto

  Keyword PROPERTY ActorTypeRobot  auto
  Keyword PROPERTY ActorTypeChild  auto
  Keyword PROPERTY ActorTypeAnimal auto

 ;ActorBase PROPERTY WorkshopBrahmin  auto
 ;ActorBase PROPERTY WorkshopNPC      auto
 ;ActorBase PROPERTY WorkshopNPCGuard auto

  Faction PROPERTY CurrentCompanionFaction auto
  Faction PROPTERY DomesticAnimalFaction   auto

  Bool[] aGV        ; array of mod created GlovalVar as value holder


; -- EVENTs --

EVENT OnCellLoad()
    myF_InitArray()        ; do it every time before a while loop is running /OR/ values could be changed
ENDEVENT


; -- FUNCTIONs --

;-----------------------
FUNCTION myF_InitArray()
;-----------------------
;;;  bool[] aGV = new Bool[3]            ; (local) function variable
     aGV = new Bool[3]                   ; (global) script variable

;    IF (pSKK_CSETIncludeCompanions.GetValue() == 0.0)
;        aGV[0] = TRUE
;    ELSE                ; == 1.0
;        aGV[0] = False
;    ENDIF

    aGV[0] = !(pSKK_CSETIncludeCompanions.GetValue() as Bool)
    aGV[1] = !(pSKK_CSETIncludeRobots.GetValue() as Bool)
    aGV[2] = !(pSKK_CSETIncludeChildren.GetValue() as Bool)
ENDFUNCTION


;-----------------------------------------------------------------------------------
Bool FUNCTION myF_IncludeThisWorkshopActor(ObjectReference ThisWorkShop, Actor aRef)
;-----------------------------------------------------------------------------------
IF ( aRef )                    ; IF (aRef != None)
ELSE
    Return False    ; /1  ThisActor is <None>
ENDIF
;---------
IF (ThisWorkshop as WorkshopScript)
ELSE
    Return False    ; /2  ThisWorkshop is not assigned to script of name "WorkshopScript"
ENDIF
;---------
IF (ThisWorkshop as WorkshopScript).OwnedByPlayer)            ; zero frames until
ELSE
    Return False    ; /3  ThisWorkshop is not player owned
ENDIF
;---------
IF myF_Test(aRef)
    Return TRUE     ; /4*  include this actor
ENDIF
;---------    
    Return False    ; /5  ThisActor is valid, but should not be included!
ENDFUNCTION


;---------------------------------
Bool FUNCTION myF_Test(Actor aRef)  ; helper
;---------------------------------
; https://www.creationkit.com/fallout4/index.php?title=Cast_Reference

; The "is" operator checks to see if the expression to the left is the type requested.
; The check is strict for base types (bool, int, etc) but loose for object types (Form, Alias, etc).

;;;    IF (aRef is WorkshopObjectActorScript)        ; DO NOT USE THIS HERE !!!
    IF (aRef as WorkshopObjectActorScript)
        Return False                                        ; 0 Turret
    ENDIF
;    ----------
    IF aGV[0]
        IF (aRef as CompanionActorScript) || (aRef as DogmeatActorScript)
            Return False                                    ; 0 Companion human or dog
        ENDIF
    ENDIF
;    --------------
    IF aGV[1]
        IF aRef.Haskeyword(ActorTypeRobot)
            Return False                                    ; 0 Robot
        ENDIF
    ENDIF
;    --------------
    IF aGV[2]
        IF aRef.Haskeyword(ActorTypeChild)
            Return False                                    ; 0 Child
        ENDIF
    ENDIF
;    --------------
    IF aRef.HasKeyword(ActorTypeAnimal)
        IF aRef.IsInFaction(DomesticAnimalFaction)                           ; +1
            Return False                                    ; 1 Dog
        ENDIF
    ELSE    ;IF (aRef.GetActorBase() == WorkshopBrahmin)                     ; +1
        IF aRef.IsInFaction(CurrentCompanionFaction)                         ; +1
            Return False                                    ; 1 Brahmin
        ENDIF
    ENDIF
;    --------------
    IF aRef.IsDisabled()                                                    ; +1
        Return False        ; actor disabled
    ENDIF
;    ----------
    IF aRef.IsDead()                                                        ; +1
        Return False        ; actor dead
    ENDIF
;    ----------
    Return TRUE        ; actor is enabled, alive and not of type mechanic, companion or child
ENDFUNCTION

 

 

Edited April 27, 2020 by ReDragon2013

[SKKmods]

No we are into a code complexity and maintenance discussion.

 

There is a trade off in maintaining simple code structure to simplify change/maintenance and extreme performance.

 

The best choice depends if the code is designed the CHANGE or designed to LAST.

 

SKK code is generally designed to CHANGE as I like to tinker and add stuff through the life-cycle as new sh1t comes to light through user adoption and feedback.

[ReDragon2013]

You wrote: "No we are into a code complexity and maintenance discussion."

 

In my opinion it's always a good idea to split large code (from events or functions) into smaller functions to have a better overview for maintenance.

I am far away to say some code is better than others. But as Bethesda wiki tell us: "Fail fast, fail early!", "Certain operations, like casting, are faster

then others" and "If you perform an operation and the result .. isn't going to change, .. .Store the result in a variable and re-use the variable where needed.

Prefer to store the variable in a function instead of your script to reduce memory costs and reduce persistence" (like References).

https://www.creationkit.com/fallout4/index.php?title=Performance_(Papyrus)

 

condition samples

IF (ThisWorkShop != None)      ; its comparing with Null-Ref, need a neg operation and need also additional temp bool 
     ; Ref is valid
ENDIF

IF (ThisWorkShop == None)      ; its comparing with Null-Ref and need additional temp bool
ELSE
     ; Ref is valid
ENDIF

IF ( ThisWorkShop )           ; fastest way while Zero flag will be checked only
    ; Ref is valid
ENDIF

IF (ThisActor.IsDisabled() == TRUE)  ; native function with using additional boolVar comparing
   ; actor disabled
ENDIF

IF ThisActor.IsEnabled()      ; convenience function, similiar to "IF !ThisActor.IsDisabled()"
   ; actor enabled
ELSE
   ; actor disabled
ENDIF

IF ThisActor.IsDisabled()
   ; actor disabled
ENDIF

I am sorry, but the papyrus compiler is not generating the fastest code for same condition logic. It generates code as the script coder has written.

Edited April 27, 2020 by ReDragon2013

[SKKmods]

In my experience I loose money on projects and can't feed my kids when developers craft overly complex code that less clever people are unable to maintain. Starvation experience vs opinion.