[LE] Code optimization questions

[PeterMartyr]

None of the codes (Jacob Expired Mine) compensate for that look at the codes logically. Where does Jacob code find a piece of armor that occupies Two Slots. I can't see it. Please cut and paste that one particular line of code for me.

Paste Here _ one line of code the finds & stores Future Partitions to be Skipped?

Remember not Checked but Future Partitions to be skipped?

[jacobpaige]

None of the codes (Jacob Expired Mine) compensate for that look at the codes logically. Where does Jacob code find a piece of armor that occupies Two Slots. I can't see it. Please cut and paste that one particular line of code for me.

Paste Here _ one line of code the finds & stores Future Partitions to be Skipped?

Remember not Checked but Future Partitions to be skipped?

slotsChecked += thisArmor.GetSlotMask() ;add all slots this item covers to our slotsChecked variable

if (Math.LogicalAnd(slotsChecked, thisSlot) != thisSlot) 

These two lines deal with this issue. The first one (which comes second in the script) adds the current and all future slots that the current Armor is occupying to the bitmap. The second one checks if the current slot was ever added by the first line by anding (1 & 1 == 0, 1&0 == 0, 0&0 == 1) every bit of the bitmap with the slot, which will result in the slot if it's been added or 0x00000000 if it hasn't. The original author chose to check for '!= thisSlot' rather than '== 0' though. This is easier to see in binary, but it wastes a ton of space, is harder to read, takes much more time to type, and is much more prone to human error when typing, so the original author used hex instead.

 

If you've never used a bitmap before, you should look into them. In the situations where they're appropriate, they're usually the best solution by far. The bitwise and, or and xor operations are generally super fast with basically no overhead. It's why you'll see them frequently in older games that had much more severe memory concerns. You'll also see them in real-time systems where microseconds matter.

[foamyesque]

If you want to move to a continuously tracked player equipment list, that's possible. The trick will be in making sure the OnEquip/OnUnEquip events don't edit the formlist while you're working on it. Individual AddForm/RemoveAddedForm calls are safe, but the FormList could potentially change in between them or as you iterate through it in the equip portion of this.

 

The simplest way to do this would be to create a boolean flag that's global to the script, set it to false by default, have it be set to true when the TriggerSwap function is called and then false once the swapping process is complete. To sidestep ordering problems, my suggestion would be (yet another :v) FormList to act as a buffer strictly of what items have had OnEquip or OnUnequip events called on them. It doesn't care if the item is currently equipped or not; it just adds it to the formlist, then calls RegisterForSingleUpdate() with a short delay. Then you can delay the OnUpdate event as long as the swapping function is called with no issues; any accumulated changes will simply be stored in the buffer formlist. If the same item is equipped/unequipped multiple times before the buffer is processed, there's no issue because AddForm() will not add duplicate forms.

 

Then, in a loop set to iterate while the buffer FormList's size is above 0, you pull the Form from position 0 and see if it's equipped. If it is, you add it to the FormList storing the full list of player gear. If it is not, you Remove it from the storage FormList. Then you remove it from the buffer FormList, and repeat (this is essentially the same solution I provided to candepin in their thread).

 

I'm not sure of a way to speed up the scan of the NPCs. Ultimately you're stuck with GetWornForm(), which is damn slow, unless somebody on the SKSE team's finally added a way to get that data all in a batch. There's ways to set up preprocessing for the NPCs, but it gets really messy.

Edited May 6, 2018 by foamyesque

[jacobpaige]

I realized I was over-complicating things with the quest and just had the mannequin cast the spell on the player OnActivate instead. Also, given the nature of the slowdown, I'm pretty sure that anyone with a better computer than me (aka: almost everyone) won't have the speed problem I'm having (assuming that they run at a higher framerate). So, I've decided to call it good enough for now and move onto my next problem: figuring out how to make this mod play nice with other mods that want to alter mannequin behavior and figuring out if it's even possible to alter my mod in such a way that it can be added or removed from an ongoing game.

 

Are there any good reference materials on what makes a mod (not) work in an ongoing game?

Edited May 6, 2018 by jacobpaige

[ReDragon2013]

No idea it is useful or not. I took your first approach and adjusted the script code. The script was compiled not yet.

 

jpSwapArmorEffectScript

 

Scriptname jpSwapArmorEffectScript extends ActiveMagicEffect
{rewritten by ReDragon 2018}
; https://forums.nexusmods.com/index.php?/topic/6619756-code-optimization-questions/
; jacobpaige wrote: "This works, but it takes ~2-3 seconds .."

  FormList PROPERTY aList auto Hidden        ; Player items, will be filled within the script
  FormList PROPERTY bList auto Hidden        ; NPC items, will be filled within the script

  Int aMax
  Int bMax


; -- EVENTs --

EVENT OnEffectStart(Actor akTarget, Actor akCaster)
IF myF_IsSKSE()
ELSE
    self.Dispel()
    RETURN    ; - STOP - /0  SKSE not found
ENDIF
;---------------------
    int i = myF_TryPlayer(akTarget, akCaster)

IF (i == 1)
    myF_Swap(akTarget, akCaster)
    RETURN    ; - STOP - /1  player is target
ENDIF
;---------------------
IF (i == 2)
    myF_Swap(akCaster, akTarget)
    RETURN    ; - STOP - /2  player is caster
ENDIF
;--------------------- i = 0, 3 or 4
; player is whether target nor caster or both (target and caster) or NPC has an outfit already
    self.Dispel()        ;  just in case?
ENDEVENT


EVENT OnUpdate()
    ; asynchronously running thread for further speed optimization
ENDEVENT

EVENT OnUpdateGameTime()
    ; asynchronously running thread for further speed optimization
ENDEVENT


; -- FUNCTIONs -- 5

;-------------------------
Bool FUNCTION myF_IsSKSE()
;-------------------------
IF (SKSE.GetVersion() > 0)
    Return TRUE
ENDIF
;---------
    Debug.Trace("jpSwapArmor: SKSE is missing! " +self)        ; see "papyrus.0.log" within folder "..\My Games\Skyrim\Logs\Script"
    Return False
ENDFUNCTION


;---------------------------------------------------------
Int FUNCTION myF_TryPlayer(Actor akTarget, Actor akCaster)
;---------------------------------------------------------
    actor player = Game.GetPlayer()

IF (akTarget == player)
    IF (akTarget == akCaster)
        RETURN 0    ; /0 player is both
    ENDIF
    RETURN 1        ; /1 player is target
ENDIF
;---------
IF (akCaster == player)
    ; https://www.creationkit.com/index.php?title=GetNumParts_-_Outfit
    IF (akTarget.GetActorBase().GetOutfit().GetNumParts() == 0)                        ; SKSE only: GetOutfit(), GetNumParts()
        RETURN 2    ; /2 player is caster and NPC does not have any outfit (is naked?)
    ENDIF
    RETURN 3        ; /3 NPC has already an outfit
ENDIF
    RETURN 4        ; /4 no matching for player
ENDFUNCTION


;/ *** SKSE implementation ********************************
int Property kSlotMask30 = 0x00000001 AutoReadOnly ; HEAD
int Property kSlotMask31 = 0x00000002 AutoReadOnly ; Hair
int Property kSlotMask32 = 0x00000004 AutoReadOnly ; BODY
int Property kSlotMask33 = 0x00000008 AutoReadOnly ; Hands
int Property kSlotMask34 = 0x00000010 AutoReadOnly ; Forearms
int Property kSlotMask35 = 0x00000020 AutoReadOnly ; Amulet
int Property kSlotMask36 = 0x00000040 AutoReadOnly ; Ring
int Property kSlotMask37 = 0x00000080 AutoReadOnly ; Feet
int Property kSlotMask38 = 0x00000100 AutoReadOnly ; Calves
int Property kSlotMask39 = 0x00000200 AutoReadOnly ; SHIELD
int Property kSlotMask40 = 0x00000400 AutoReadOnly ; TAIL
int Property kSlotMask41 = 0x00000800 AutoReadOnly ; LongHair
int Property kSlotMask42 = 0x00001000 AutoReadOnly ; Circlet
int Property kSlotMask43 = 0x00002000 AutoReadOnly ; Ears
int Property kSlotMask44 = 0x00004000 AutoReadOnly ; Unnamed
int Property kSlotMask45 = 0x00008000 AutoReadOnly ; Unnamed
int Property kSlotMask46 = 0x00010000 AutoReadOnly ; Unnamed
int Property kSlotMask47 = 0x00020000 AutoReadOnly ; Unnamed
int Property kSlotMask48 = 0x00040000 AutoReadOnly ; Unnamed
int Property kSlotMask49 = 0x00080000 AutoReadOnly ; Unnamed
int Property kSlotMask50 = 0x00100000 AutoReadOnly ; DecapitateHead  * masked out
int Property kSlotMask51 = 0x00200000 AutoReadOnly ; Decapitate      * masked out
int Property kSlotMask52 = 0x00400000 AutoReadOnly ; Unnamed
int Property kSlotMask53 = 0x00800000 AutoReadOnly ; Unnamed
int Property kSlotMask54 = 0x01000000 AutoReadOnly ; Unnamed
int Property kSlotMask55 = 0x02000000 AutoReadOnly ; Unnamed
int Property kSlotMask56 = 0x04000000 AutoReadOnly ; Unnamed
int Property kSlotMask57 = 0x08000000 AutoReadOnly ; Unnamed
int Property kSlotMask58 = 0x10000000 AutoReadOnly ; Unnamed
int Property kSlotMask59 = 0x20000000 AutoReadOnly ; Unnamed
int Property kSlotMask60 = 0x40000000 AutoReadOnly ; Unnamed
int Property kSlotMask61 = 0x80000000 AutoReadOnly ; FX01            * masked out
************************************************************ /;

;-----------------------------------------------------------------------
Form[] FUNCTION GetWornEquipmentButNotHands(Actor aRef, FormList myList)
;-----------------------------------------------------------------------
; from https://www.creationkit.com/index.php?title=Slot_Masks_-_Armor
    form[] a = new Form[30]           ; a = wornForms
    
    int iSlot = 0x00000001            ; iSlot = thisSlot, init by 1
    int iTest = 0x80300000            ; iTest = slotsChecked, (0x00100000 + 0x00200000 + 0x80000000)
     int i = 0                        ; i = index

WHILE (iSlot < 0x80000000)                            ; (i < kSlotMask61)
    IF (Math.LogicalAnd(iTest, iSlot) == iSlot)
        ; slots we have found equipped already or excluded by mask "iTest"
    ELSE
        form fm = aRef.GetWornForm(iSlot)
        IF (fm as Armor)            ; (fm as Armor) = thisArmor
            a[i] = fm
            i = i + 1
            myList.AddForm(fm)      ; fill the desired formlist
            iTest += fm.GetSlotMask()     ; add all slots this item covers to our iTest variable
        ELSE
            iTest += iSlot          ; no armor was found on this slot
        ENDIF
     ENDIF
    iSlot = iSlot * 2               ; double the number to move on to the next slot
ENDWHILE

    IF (myList == aList)            ; test formIDs
        aMax = i
    ELSE
        bMax = i
    ENDIF

    RETURN a                        ; give back the whole array
ENDFUNCTION


;-------------------------------------------------------------------
FUNCTION SwapEquipment(Actor aRef1, Actor aRef2, Form[] a, Int iMax)
;-------------------------------------------------------------------
    bool bOK = (aRef2 == Game.GetPlayer())                    ; who is the player, if TRUE == Target

; https://www.creationkit.com/index.php?title=RemoveItem_-_ObjectReference

    IF ( bOK )
        aRef1.RemoveItem(bList, 1, TRUE, aRef2)        ; ## take a formlist to move items in a rush to the player
    ELSE
        aRef1.RemoveItem(aList, 1, TRUE, aRef2)        ; ## take a formlist to move items in a rush to the NPC
    ENDIF

int i = 0
WHILE (i < iMax)
;##    aRef1.RemoveItem(a[i], 1, TRUE, aRef2)          ; ##
    IF ( bOK )                                               ; * equip player only
        aRef2.EquipItemEx(a[i], equipSound = false)          ; SKSE only
    ENDIF                                                    ; *
    i = i + 1
ENDWHILE
ENDFUNCTION


;------------------------------------------------
FUNCTION myF_Swap(Actor Player, Actor NPC, Int i)
;------------------------------------------------
    form[] a = GetWornEquipmentButNotHands(Player, aList)
    form[] b = GetWornEquipmentButNotHands(NPC, bList)

    SwapEquipment(Player, NPC, a, aMax)
    SwapEquipment(NPC, Player, b, bMax)            ; + EquipPlayer(b)

; https://www.creationkit.com/index.php?title=Revert_-_FormList
    aList.Revert()
    bList.Revert()
ENDFUNCTION

 

 

 

updated version jpSwapArmorEffectScript

 

Scriptname jpSwapArmorEffectScript extends ActiveMagicEffect
{rewritten by ReDragon 2018}
; https://forums.nexusmods.com/index.php?/topic/6619756-code-optimization-questions/
; jacobpaige wrote: "This works, but it takes ~2-3 seconds .."

; will be filled within the script
  FormList PROPERTY aList auto Hidden        ; Player items
  FormList PROPERTY bList auto Hidden        ; NPC items


; -- EVENTs --

EVENT OnEffectStart(Actor akTarget, Actor akCaster)
IF myF_IsSKSE()
ELSE
    self.Dispel()
    RETURN    ; - STOP - SKSE not found
ENDIF
;---------------------
    int i = myF_CheckPlayer(akTarget, akCaster)

IF (i == 1)
    myF_Swap(akTarget, akCaster)
    RETURN    ; - STOP - player is target
ENDIF
;---------------------
IF (i == 2)
    myF_Swap(akCaster, akTarget)
    RETURN    ; - STOP - player is caster
ENDIF
;--------------------- i = 0, 3 or 4
; player is whether target nor caster or both (target and caster) or NPC has an outfit already
    self.Dispel()        ;  just in case?
ENDEVENT


;EVENT OnUpdate()
    ; asynchronously running thread for further speed optimization
;ENDEVENT

;EVENT OnUpdateGameTime()
    ; asynchronously running thread for further speed optimization
;ENDEVENT


; -- FUNCTIONs -- 5

;-------------------------
Bool FUNCTION myF_IsSKSE()
;-------------------------
IF (SKSE.GetVersion() > 0)
    Return TRUE
ENDIF
;---------
    Debug.Trace("jpSwapArmor: SKSE is missing! " +self)        ; see "papyrus.0.log" within folder "..\My Games\Skyrim\Logs\Script"
    Return False
ENDFUNCTION


;-----------------------------------------------------------
Int FUNCTION myF_CheckPlayer(Actor akTarget, Actor akCaster)
;-----------------------------------------------------------
    actor player = Game.GetPlayer()

IF (akTarget == player)
    IF (akTarget == akCaster)
        RETURN 0    ; /0 player is both
    ENDIF
    RETURN 1        ; /1* player is target
ENDIF
;---------
IF (akCaster == player)
    ; https://www.creationkit.com/index.php?title=GetNumParts_-_Outfit
    IF (akTarget.GetActorBase().GetOutfit().GetNumParts() == 0)            ; SKSE required! GetOutfit(), GetNumParts()
        RETURN 2    ; /2* player is caster and NPC does not have any outfit (is naked?)
    ENDIF
    RETURN 3        ; /3 NPC has already an outfit
ENDIF
    RETURN 4        ; /4 no matching for player
ENDFUNCTION

;-----------------------------------------------------------------------
Form[] FUNCTION GetWornEquipmentButNotHands(Actor aRef, FormList myList)
;-----------------------------------------------------------------------
; from https://www.creationkit.com/index.php?title=Slot_Masks_-_Armor
    form[] a = new Form[30]           ; a = wornForms
    
    int iSlot = 0x00000001            ; iSlot = thisSlot, init by 1
    int iMask = 0x80300000            ; iMask = slotsChecked, (0x00100000 + 0x00200000 + 0x80000000)
     int i = 0                        ; i = index

WHILE (iSlot < 0x80000000)                            ; (i < kSlotMask61)
    IF (Math.LogicalAnd(iMask, iSlot) == iSlot)                       ; SKSE required!
        ; slots we have found equipped already or excluded caused by "iMask"
    ELSE
        form fm = aRef.GetWornForm(iSlot)                             ; SKSE required!
        IF (fm as Armor)              ; (fm as Armor) = thisArmor
            a[i] = fm
            i = i + 1
            myList.AddForm(fm)        ; fill the specific formlist, see script properties
            iMask += fm.GetSlotMask()                                 ; SKSE required! add all slots this item covers to iMask
        ELSE
            iMask += iSlot            ; no armor was found on this slot
        ENDIF
    ENDIF
    iSlot = iSlot * 2                 ; double the number to move on to the next slot
ENDWHILE

    RETURN a                          ; give back the whole array
ENDFUNCTION


;--------------------------------------------------------
FUNCTION SwapEquipment(Actor aRef, Actor aRef2, Form[] a)
;--------------------------------------------------------
    bool bOK = (aRef2 == Game.GetPlayer())                    ; who is the player, if TRUE then player is Target
    int iMax

; https://www.creationkit.com/index.php?title=RemoveItem_-_ObjectReference

    IF ( bOK )
        aRef.RemoveItem(bList as Form, 1, TRUE, aRef2)        ; ## take a formlist to move items in a rush to the player
        iMax = bList.GetSize()
    ELSE
        aRef.RemoveItem(aList as Form, 1, TRUE, aRef2)        ; ## take a formlist to move items in a rush to the NPC
        iMax = aList.GetSize()
    ENDIF

int i = 0
WHILE (i < iMax)
;##    aRef.RemoveItem(a[i], 1, TRUE, aRef2)                    ; ## THIS is obsolete because of formlist usage below!
    IF ( bOK )                                                       ; * equip player only
        aRef2.EquipItemEx(a[i], equipSound = false)                  ; SKSE required!
    ENDIF                                                            ; *
    i = i + 1
ENDWHILE
ENDFUNCTION


;-----------------------------------------
FUNCTION myF_Swap(Actor Player, Actor NPC)
;-----------------------------------------
    form[] a = GetWornEquipmentButNotHands(Player, aList)    ; store equipped player items into array and formlist
    form[] b = GetWornEquipmentButNotHands(NPC, bList)       ; store equipped NPC items ..

    SwapEquipment(Player, NPC, a)
    SwapEquipment(NPC, Player, b)            ; + EquipPlayer(b)

; https://www.creationkit.com/index.php?title=Revert_-_FormList
    aList.Revert()
    bList.Revert()
ENDFUNCTION

 

 

Edited May 6, 2018 by ReDragon2013

[jacobpaige]

No idea it is useful or not. I took your first approach and adjusted the script code. The script was compiled not yet.

 

You're right. I should check that they have SKSE. Not everyone reads the mod requirements before installing after all.

 

The check for player seems a bit excessive. The affect is tied to a single target spell that can't target the caster. Just checking that the target or the caster is the player should be enough.

 

Is there a reason to get rid of EquipPlayer() and move its functionality into SwapEquipment()? It feels like it's just making things more complicated than they need to be.

 

Also, is there a reason that you're working so hard to avoid FormList.ToArray()? Is it that expensive of an operation?

 

You should really consider picking better names. The less time people have to spend on remembering what a particular variable is referring to, the faster and easier it will be for them to understand the code. Additionally, it drastically reduces the number of comments you need to write and (more importantly) maintain. Its also a good idea to follow Bethesda's naming conventions. It makes it easier on the community as a whole if they're trying to understand and edit your code. Though, I think I messed that up myself in a few places.

[Evangela]

I find downcounting generally easier to work with, but it's pretty minor all told. If you find upcounting easier to understand and maintain absolutely use that.

 

FormLists need to be created in the CK but they don't need to have anything added to them at that time. Skyrim supports GetSize(), AddForm(), RemoveAddedForm(), Revert(), GetAt(), Find(), and HasForm() commands for manipulating the data in them. They are often the best way to pass information to and from major game systems, such as alias conditions, or to Add/RemoveItem calls, or things of that nature, where arrays -- because they were retrofitted in after release -- aren't accepted.

Formlists are known to get slower the bigger they are in size, where as arrays are just fast period, except in situations that you've already outlined. I learned this the hard way when I worked with 100+ forms in a list. It took about 10 seconds to go through the whole thing - mileage of course varies depending on the system(I use a laptop).

[jacobpaige]

 

I find downcounting generally easier to work with, but it's pretty minor all told. If you find upcounting easier to understand and maintain absolutely use that.

 

FormLists need to be created in the CK but they don't need to have anything added to them at that time. Skyrim supports GetSize(), AddForm(), RemoveAddedForm(), Revert(), GetAt(), Find(), and HasForm() commands for manipulating the data in them. They are often the best way to pass information to and from major game systems, such as alias conditions, or to Add/RemoveItem calls, or things of that nature, where arrays -- because they were retrofitted in after release -- aren't accepted.

Formlists are known to get slower the bigger they are in size, where as arrays are just fast period, except in situations that you've already outlined. I learned this the hard way when I worked with 100+ forms in a list. It took about 10 seconds to go through the whole thing - mileage of course varies depending on the system(I use a laptop).

 

Good to know., but given that the main speed up here is coming from the reduction in external calls, I'm not sure I'd see a speed up even if both actors had every slot filled.

[ReDragon2013]

jacobpaige wrote:

 

"You should really consider picking better names."

That's funny.. did you ever heard about "KISS-Formel" (Keep It Short and Simple). My advice in programming, use short names for function variables and longer names (expressions) for script properties and variables.

 

"The less time people have to spend on remembering what a particular variable is referring to, the faster and easier it will be for them to understand the code."

Most of my comments and also included URLs are made to reflect to the script code snippet you provided us.

 

"Additionally, it drastically reduces the number of comments you need to write and (more importantly) maintain. Its also a good idea to follow Bethesda naming convention."

Not for sure, name convention is important, but it does not make the compiled script better or faster.

 

You took this for example:

Form[] targetEquipped = GetWornEquipmentButNotHands(akTarget)

I used this instead:

form[] a = GetWornEquipmentButNotHands(Player, aList)    ; store equipped player items into array and formlist  

You asked also:

"Is there a reason to get rid of EquipPlayer() and move its functionality into SwapEquipment()?"

Do you really understand the code inside the function:

;--------------------------------------------------------
FUNCTION SwapEquipment(Actor aRef, Actor aRef2, Form[] a)
;--------------------------------------------------------
    bool bOK = (aRef2 == Game.GetPlayer())                    ; who is the player, if TRUE then player is Target
    int iMax

    IF ( bOK )
        aRef.RemoveItem(bList as Form, 1, TRUE, aRef2)        ; ## take a formlist to move items in a rush to the player
        iMax = bList.GetSize()
    ELSE
        aRef.RemoveItem(aList as Form, 1, TRUE, aRef2)        ; ## take a formlist to move items in a rush to the NPC
        iMax = aList.GetSize()
    ENDIF

int i = 0
WHILE (i < iMax)
;##    aRef.RemoveItem(a[i], 1, TRUE, aRef2)                    ; ## THIS is obsolete because of formlist usage!
    IF ( bOK )                                                       ; * equip player only
        aRef2.EquipItemEx(a[i], equipSound = false)                  ; SKSE required!
    ENDIF                                                            ; *
    i = i + 1
ENDWHILE
ENDFUNCTION

I removed every single call of RemoveItem() by using a formlist here!! That means, the following loop is empty and ready for EquipPlayer() action.

 

"Also, is there a reason that you're working so hard to avoid FormList.ToArray()? Is it that expensive of an operation?"

No idea.. I never used SKSE functions. But you get filled the formlist for free by implementing into GetWornEquipmentButNotHands().

  form fm = aRef.GetWornForm(iSlot)                            ; SKSE required!
  IF (fm as Armor)
      a[i] = fm
      i = i + 1
      myList.AddForm(fm)        ; fill the specific formlist, myList as function variable is a pointer to hidden script properties "aList" or "bList"
      iMask += fm.GetSlotMask()                                 ; SKSE required!
  ELSE
      iMask += iSlot            ; no armor was found on this slot
  ENDIF
    ENDIF

Cheers..

 

[Evangela]

It's that kind of clashing of styles, why I don't want to work with other programmers XD.

 

While I wont go all into details, I'll tell you that your style forces you to explain excessively through comments.

Edited May 6, 2018 by Rasikko