[LE] Need help speeding up my script

[foamyesque]

Multithreading is an interesting and powerful thing -- I make fairly major use of it in my mannequin mod -- but there's lots of ways to shoot yourself in the foot with it too. It's *generally* best used when you want to do something relatively slow to a lot of different objects more or less simultaneously. In my case, that was inventory restock and manipulation across a few score NPCs and chests.

 

You're working with a lot of data but it's all in one place, so multithreading may help, but I would first try to get rid of, or shift to processing elsewhere, every single frame-linked function call you can. Every time you make such a call there's the possibility of your script being paused to allow another script to execute on that thread, and they can never execute faster than your frame rate even if they don't lose processing priority. They're poison if you need to really push performance. That's why RemoveItem(formlist) and RemoveAllItems() are tremendously faster than a looped RemoveItem(form) setup for even quite small numbers of items.

Edited April 23, 2018 by foamyesque

[foamyesque]

Okay so I've got the script together and it successfully does the swaps but there's a catch; the alchemy crafting screen doesn't reflect the changes until after at least one potion has been crafted. I'm not sure how to force an update of it; it's not a problem I've had to solve before.

[candlepin]

Interesting. My testing with my first method slowly updated the player's inventory while in the crafting menu without the need to craft anything.

 

I've been looking at this a bit too and here is what I came up with for a function to reset the container inventory:

 

 

 

Function UpdateContainer()
	If (CP_GV_swap.GetValue() == 0)			; items in inventory are in non-ingredient form
		SwapContainer.RemoveAllItems()		; SwapContainer to house items to be added to player up swap.  contains "mirror inventory"
		Int i = CP_VanillaItems.GetSize() 
		While i
			i -= 1
			int Count = PlayerRef.GetItemCount(CP_VanillaItems.GetAt(i) As Form)
			If Count
				Form ItemB = CP_MissingIngredients.GetAt(i) As Form
				SwapContainer.AddItem(ItemB, Count, True)			
			Endif
		EndWhile		
	ElseIf (CP_GV_swap.GetValue() == 1)			; items in inventory are in ingredient form
		SwapContainer.RemoveAllItems()
		Int i = CP_MissingIngredients.GetSize() 
		While i
			i -= 1
			int Count = PlayerRef.GetItemCount(CP_MissingIngredients.GetAt(i) As Form)
			If Count
				Form ItemB = CP_VanillaItems.GetAt(i) As Form
				SwapContainer.AddItem(ItemB, Count, True)			
			Endif
		EndWhile	
	EndIf
EndFunction

 

 

 

I figured it would be quicker and probably cleaner to clear the container contents and then restock it than to compare the container contents to the player's inventory and then adjust. But maybe I'm wrong about that?

 

So this would go in a script attached to the player via ReferenceAlias. I'm thinking that this function would be called upon OnInit(). Maybe also OnPlayerLoadGame()? Not sure when else. Perhaps after exiting the crafting station menu?

 

And here is what I've come up with so far for adjusting the container contents on the fly:

 

 

 

Event OnItemAdded(Form akBaseItem, int aiItemCount, ObjectReference akItemReference, ObjectReference akSourceContainer)
	If (CP_GV_swap.GetValue() == 0)
		Int ListNumber = CP_VanillaItems.Find(akBaseItem)
		Form SwapItem = CP_MissingIngredients.GetAt(ListNumber)
		SwapContainer.AddItem(SwapItem, aiItemCount, True)
  ;   SwapContainer.AddItem(CP_MissingIngredients.GetAt(CP_VanillaItems.Find(akBaseItem)),     
  ;   aiItemCount, True)
  ;  would the above be faster?  it's more confusing to me, but less code
	ElseIf (CP_GV_swap.GetValue() == 1)
		Int ListNumber = CP_MissingIngredients.Find(akBaseItem)
		Form SwapItem = CP_VanillaItems.GetAt(ListNumber)
		SwapContainer.AddItem(SwapItem, aiItemCount, True)
	EndIf
EndEvent

Event OnItemRemoved(Form akBaseItem, int aiItemCount, ObjectReference akItemReference, ObjectReference akSourceContainer)
	If (CP_GV_swap.GetValue() == 0)
		Int ListNumber = CP_VanillaItems.Find(akBaseItem)
		Form SwapItem = CP_MissingIngredients.GetAt(ListNumber)
		SwapContainer.RemoveItem(SwapItem, aiItemCount, True)
	ElseIf (CP_GV_swap.GetValue() == 1)
		Int ListNumber = CP_MissingIngredients.Find(akBaseItem)
		Form SwapItem = CP_VanillaItems.GetAt(ListNumber)
		SwapContainer.RemoveItem(SwapItem, aiItemCount, True)
	EndIf
EndEvent

 

 

 

I'd put this in the same script attached to the player via ReferenceAlias.

 

To speed things up, I figure I'll also need to add InventoryEventFilters:

 

 

 

Function AddFilter()
	If (CP_GV_swap.GetValue() == 0)
		RemoveAllInventoryEventFilters()
		AddInventoryEventFilter(CP_VanillaItems)
	ElseIf (CP_GV_swap.GetValue() == 1)
		RemoveAllInventoryEventFilters()
		AddInventoryEventFilter(CP_MissingIngredients)
	EndIf
EndFunction

 

 

 

This can also be called OnInit() and whenever the global variable is swapped.

 

And finally, here is the actual swap function.

 

 

 

Function InventorySwap()
	If (CP_GV_swap.GetValue() == 0)
		SwapContainer.RemoveAllItems(PlayerRef)
		PlayerRef.RemoveItem(CP_VanillaItems, 999, true, SwapContainer)
	ElseIf (CP_GV_swap.GetValue() == 1)
		SwapContainer.RemoveAllItems(PlayerRef)
		PlayerRef.RemoveItem(CP_MissingIngredients, 999, true, SwapContainer)
	EndIf
EndFunction

 

 

 

That's what I've put together so far. I have to do some more thinking about when to add and remove the InventoryEventFilters, but I think this is a pretty good start.

Edited April 24, 2018 by candlepin

[candlepin]

Hmm... random thought. Instead of having the alternative version in a chest somewhere, why not just add it directly to the player's inventory (or remove it) using the same OnItemAdded/OnItemRemoved approach? That way, both versions of the item are always accessible with no delay.

 

The only issue I see with this approach would be the inventory weight. Maybe make one of them weightless? Or update the player's carry weight? I might need to think about this possibility some more.

[foamyesque]

Hrm, this is weird. I'm getting *two* OnItemRemoved fires when the player eats something, which is breaking the approach. It's possible to adapt it to just read the amount in the player's inventory, but that's a slower process...

Edited April 25, 2018 by foamyesque

[foamyesque]

Well, until I can diagnose why I get two OnItemRemoved events, these are my scripts:

 

On a player alias:

Scriptname CP_IngredientSwap extends ReferenceAlias  

GlobalVariable Property CP_SwapState Auto
FormList Property CP_IngredientList Auto
FormList Property CP_OriginalList Auto
FormList Property CP_ItemStack Auto
FormList Property CP_EmptyList Auto
Perk Property CP_AlchemyPerk Auto
Spell Property CP_TransmuteIngredients Auto
ReferenceAlias Property SwapContainerAlias Auto

ObjectReference PlayerRef = none
ObjectReference SwapContainerRef = none

bool bUpdating = false
bool bSwapping = false
int iSKSEVersion = 0

Event OnInit()

    GotoState("Starting")
    iSKSEVersion = SKSE.GetVersion()
    RegisterForSingleUpdate(5)

EndEvent

Event OnPlayerLoadGame()

    GotoState("Starting")
    iSKSEVersion = SKSE.GetVersion()
    RegisterForSingleUpdate(1)

EndEvent

State Starting

    Event OnUpdate()

        if bUpdating
            return
        endif

        bUpdating = true

        SwapContainerRef = SwapContainerAlias.GetRef()
        self.ForceRefTo(Game.GetPlayer())
        PlayerRef = self.GetRef()
        (PlayerRef as Actor).AddPerk(CP_AlchemyPerk)

        SeedInventory(true)
        SeedInventory(false)

        bUpdating = false
        GotoState("Started")

        OnUpdate()
        SwapInventory(CP_SwapState.GetValue() as bool)
    
    EndEvent

    Function SeedInventory(bool abSwapState)

        FormList LookupList = CP_OriginalList
        FormList SendList = CP_IngredientList

        if abSwapState
            LookupList = CP_IngredientList
            SendList = CP_OriginalList
        endif

        SwapContainerRef.RemoveAllItems()

        if iSKSEVersion > 0
            ; SKSE version

            Form[] LookupArray = LookupList.ToArray()
            Form[] SendArray = SendList.ToArray()

            int i = LookupArray.Length
            while i > 0
                i-=1
                int iCount = PlayerRef.GetItemCount(LookupArray[i])
                if iCount > 0 && i < SendArray.Length
                    SwapContainerRef.AddItem(SendArray[i], iCount)
                endif
            endwhile
        else
            ; non-SKSE version

            int i = LookupList.GetSize()
            int j = SendList.GetSize()
            while i > 0
                i-=1
                int iCount = PlayerRef.GetItemCount(LookupList.GetAt(i))
                if iCount > 0 && i < j
                    SwapContainerRef.AddItem(SendList.GetAt(i), iCount)
                endif
            endwhile
        endif

    EndFunction

EndState

State Started

    Event OnUpdate()
        
        while bUpdating
            Utility.Wait(0.1)
        endwhile

        bUpdating = true
        while CP_ItemStack.GetSize()
            Form testItem = CP_ItemStack.GetAt(0)

            int iCount = PlayerRef.GetItemCount(testItem)
            CP_ItemStack.RemoveAddedForm(testItem)
            Form swapItem = LookupItem(testItem, CP_OriginalList, CP_IngredientList)
            if !swapItem
                swapItem = LookupItem(testItem, CP_IngredientList, CP_OriginalList)
            endif

            if swapItem
                SwapContainerRef.RemoveItem(swapItem, 999999, true)
                SwapContainerRef.AddItem(swapItem, iCount, true)
            endif

        endwhile

        bUpdating = false    
        
    EndEvent

    Event OnSpellCast(Form akSpell)
        if akSpell == CP_TransmuteIngredients
            SwapInventory(!(CP_SwapState.GetValue() as bool))
        endif
    EndEvent

    Function SwapInventory(bool abTargetSwap)

        while bSwapping
            Utility.Wait(0.1)
        endwhile

        bSwapping = true

        FormList SendList = CP_OriginalList
        FormList GetList = CP_IngredientList

        if !abTargetSwap
            SendList = CP_IngredientList
            GetList = CP_OriginalList
        endif

        RemoveAllInventoryEventFilters()
        AddInventoryEventFilter(CP_EmptyList)

        PlayerRef.RemoveItem(SendList, 999999, true, SwapContainerRef)
        SwapContainerRef.RemoveItem(GetList, 999999, true, PlayerRef)

        RemoveAllInventoryEventFilters()
        AddInventoryEventFilter(GetList)
        AddInventoryEventFilter(SendList)

        CP_SwapState.SetValue(abTargetSwap as int)
        bSwapping = false

    EndFunction

EndState

Event OnItemAdded(Form akBaseItem, Int aiItemCount, ObjectReference akItemReference, ObjectReference akSourceContainer)
    UnregisterForUpdate()
    CP_ItemStack.AddForm(akBaseItem)
    RegisterForSingleUpdate(0.1)    
EndEvent

Event OnItemRemoved(Form akBaseItem, Int aiItemCount, ObjectReference akItemReference, ObjectReference akDestContainer)
    UnregisterForUpdate()
    CP_ItemStack.AddForm(akBaseItem)
    RegisterForSingleUpdate(0.1)
EndEvent

Form Function LookupItem(Form akBaseItem, FormList akLookupList, FormList akSendList)

    int iIndex = akLookupList.Find(akBaseItem)
    
    if iIndex < 0
    elseif iIndex < akSendList.GetSize()
        return akSendList.GetAt(iIndex)
    endif
    
    return none
EndFunction

Function SeedInventory(bool abSwapState)
EndFunction

Function SwapInventory(bool abTargetSwap)
EndFunction

A perk fragment script, to override the standard activation and ensure the swaps are completed before the crafting menu opens:

;BEGIN FRAGMENT CODE - Do not edit anything between this and the end comment
;NEXT FRAGMENT INDEX 4
Scriptname foam_PRKF_CP_AlchemyPerk_01001D94 Extends Perk Hidden
bool bActivated = false

;BEGIN FRAGMENT Fragment_0
Function Fragment_0(ObjectReference akTargetRef, Actor akActor)

;BEGIN CODE
if !bActivated
    bActivated = true
    PlayerAlias.SwapInventory(true)
    akTargetRef.Activate(akActor)
    while akTargetRef.IsFurnitureInUse(true) || !Game.IsLookingControlsEnabled()
        Utility.Wait(0.5)
    endwhile
    PlayerAlias.SwapInventory(false)
    bActivated = false
endif
;END CODE

EndFunction
;END FRAGMENT

;END FRAGMENT CODE - Do not edit anything between this and the begin comment

CP_IngredientSwap Property PlayerAlias  Auto  
 

The perk is a perk entry point, of the Activate / Add Activate Choice type. Run Immediately and Override Default are both checked. Conditions are 1. the user must be the player (who is the only one granted the perk in any event, but it's a fail-safe), and 2, the target must have the IsAlchemy keyword.

 

 

The functional mechanic is that, on initialization or game load, the swap container is seeded with the mirrored forms of whatever is in the player's inventory. OnItemAdded and OnItemRemoved events will do an AddForm operation to a FormList that is used to store a stack of all items that have been changed since the last time the swap container was updated, and then unregister any pending OnUpdates and register one of their own. When the OnUpdate processes, it will then pull from the top of the stack, get the count of that form in the player's inventory, and set the swap container to have that many of the mirrored item. The swap is then called either when the player activates an alchemy station or casts a spell.

 

The swap container reference is stored in a second alias on the same quest as the player alias; with minor tweaks you could just as easily directly store it as an objectreference property. I tend to prefer the alias approach, however.

The swap spell has no scripting at all; its cast is detected and processed by the player alias script. An alternative layout could have a small script attached to a magic effect instead, allowing for potentially multiple spells or conditional triggers of the swap, but I didn't think it needed.

 

I debated back and forth on whether the seeding would be best done by iterating through the player's inventory or through the formlists, and settled on the formlists on the grounds they are a constant size and constant data, so the execution time will remain consistent and they're not going to change while the process is running. The swap process is locked so that it will complete before the next iteration begins and avoid data trampling. Likewise, any update calls that happen while the startup update event is running will be thrown out, but since the item stack is still built, the startup update calls the standard update at the end of its processing, allowing for any changes that were made during the seeding to be processed. I decided to listen for both sorts of items, to allow for circumstances where, for example, items are in their food configuration but the ingredient version is added to the player, or the reverse; this could easily occur, for example, if someone is playing with the swap spell.

 

The seeding process has an SKSE branch, because iterating through an array, as SKSE allows you to do, is much, much faster than doing the same thing with a formlist, and the seeding branch is where that's most important. I don't believe any other aspect of the code touches SKSE, so using this doesn't require it; using it will improve performance, but all not having it should cause is an error to the log every time the player loads their game without SKSE. Not a big deal.

 

It is vitally important that the two FormLists storing the original forms and their matching ingredients are exactly matched. While the code will check for out-of-bounds indexes it cannot account for off-by-one or sequencing errors that might break the lookups.

 

 

 

I've also not stress-tested this with the hundreds of forms you say you're after, @candlepin, because quite frankly building that test would be really tedious. You'll need to do that check.

 

EDIT: I also didn't include checks to ensure the properties are filled properly.

Edited April 25, 2018 by foamyesque

[candlepin]

Wow. That is incredibly kind of you, foamyesque. :thumbsup: I will look into implementing your script into my mod this weekend (have a big work presentation on Friday). SKSE functionality isn't a problem; other components of my mod require it.

[foamyesque]

Did you get a chance to try it?

[candlepin]

Unfortunately, I was not able to test this out yet. Due to an unexpected family obligation this weekend, I was unable to work on my mod as much as I was planning to.

 

I did start to implement this though, and ran into a snag. While compiling the player alias script was relatively easy, I ran into issues with the perk fragment script.

 

In particular, I kept getting compilation errors. One of the things the CK didn't seem to like was:

CP_IngredientSwap Property PlayerAlias  Auto  

It also didn't like the function. I haven't done much with script fragments, so I have the feeling I might be doing something wrong. Could you perhaps explain in simple terms how this should be implemented (e.g. where exactly do I add the script and when)? A Skyrim scripting/CK use for dummies version? And thanks again for all your help! It really is appreciated!

[foamyesque]

Fragments are a bit of a pain and unfortunately just drag & dropping code runs into problems. You'll find that despite the explicit property declaration the fragment thinks there are no properties on it, for example.

 

What you want to do is open up the perk, then add a single ";" to the fragment window, then hit compile, then save & close the perk. That will automatically generate the function name and script name. Then open the perk again, go to the fragment window, and now the 'add properties' button should allow you to add the ReferenceAlias property the script needs.

 

From there you should be able to edit the fragment much the way you would other scripts.