[LE] Need help speeding up my script

[candlepin]

I have a script that searches through the player's inventory and swaps out one set of items for another. It works properly but unfortunately is unacceptably slow. Here is the portion of my script that does the swapping:

 

 

 

Function SwapItemsInventory(FormList ListA, FormList ListB)
	If PlayerRef.GetItemCount(ListA)
		Int i = ListA.GetSize() 
		while i
			i -= 1
			Form ItemA = ListA.GetAt(i) As Form
			int Count = PlayerRef.GetItemCount(ItemA)
			If Count
				Form ItemB = ListB.GetAt(i) As Form
				PlayerRef.RemoveItem(ItemA, Count, True)
				PlayerRef.AddItem(ItemB, Count, True)			
			Endif
		EndWhile
	EndIf
EndFunction 

 

 

 

When the player doesn't have many items, this works fine. But if you have a lot of items (say 500+) in your inventory it is unbearably slow. I have a lot of items I'd like to swap (a little over 200) and since I have this swapping triggered when a crafting menu is opened I need it to happen as quickly as possible. In my current testing where I convert all 200+ items it takes approximately 45 seconds, which is obviously too long. Nobody will use my mod if they have to wait that long every time they open a crafting menu. Hell, I wouldn't even use it. I have a few thoughts on how I could make this faster and would really appreciate input (including other suggestions) on how to do this faster. My initial thoughts:

 

1) Use a brute force approach. Define all 200+ items from ListA and the corresponding 200+ items from ListB. Then use a more targeted search and replace for each item set. Maybe something like this:

 

 

 

If PlayerRef.HasItem(ItemA1)
	int Count = PlayerRef.GetItemCount(ItemA1)
	PlayerRef.RemoveItem(ItemA1, Count, True)
	PlayerRef.AddItem(ItemB1, Count, True)			
EndIf
If PlayerRef.HasItem(ItemA2)
    int Count = PlayerRef.GetItemCount(ItemA2)
    PlayerRef.RemoveItem(ItemA2, Count, True)
    PlayerRef.AddItem(ItemB2, Count, True)   
EndIf

etc

 

 

 

But, I'm not sure that this would actually be any faster than my original approach.

 

2) Threading? I don't quite understand the coding, but basically it allows multiple parts of the code to be performed at once. I've read that this is especially helpful in increasing the speed of scripts that use a lot of repetitive latent functions, which my script certainly does.

 

3) Poor man's threading? Could I achieve similar results to threading by just splitting up my lists and having multiple effects that call scripts that each convert smaller chunks of my original list?

For example have Script1 attached to Effect1. Script 1 encodes the conversion of the first 25 items in my original list. Script 2, attached to Effect 2, encodes the conversion of the next 25 items in my original list. Since these are separate scripts, they should be run at the same time and thus achieve the goal faster.

 

Any input would be greatly appreciated!

 

[FrankFamily]

If the thing to avoid is iterating throught he whole formlist, here's an idea:

 

RemoveItem passing a formlist of the items you want to remove if the player has them, count being 999 and sending them to a container you place somewhere. This should move all your items that exist in the formlist to a container (I think, haven't tested, wiki suggest this is the case, "If you pass in a form list, it will remove aiCount of each item in the form list from the container. If there isn't aiCount of a particular item in the container, it will remove all of them."). Then with OnItemAdded you catch the items as they enter this container, remove them and add the new one to the player. Assuming it works, perhaps it's faster, though maybe the events are too much and it's actually worse.

 

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

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

[candlepin]

So that could be something like this?

 

script applied to player:

PlayerRef.RemoveItem(ListA, 999, true, SwapContainer)

script on SwapContainer:

Event OnItemAdded(Form akBaseItem, int aiItemCount, ObjectReference akItemReference, ObjectReference akSourceContainer)
     Int FormNumber ListA.Find(akBaseItem)
     ItemB = ListB.GetAt(FormNumber)
     PlayerRef.AddItem(ItemB, aiItemCount, true)
     SwapContainer.RemoveItem(akBaseItem, aiItemCount, true)
EndEvent     

[FrankFamily]

Yeah, that's what I was thinking.

[ReDragon2013]

candlepin wrote: "3) Poor man's threading?"

 

In case you don't have a quest with aliases, splitting the formlist to a couple of smaller once should increase the script runtime by using different script names.

There are two very strange ways provided by Bethesda https://www.creationkit.com/index.php?title=Creating_Multithreaded_Skyrim_Mods

but no fear you don't have to use such a construct to make runtime faster.

 

Unfortunately you don't give us the whole script, only a short function. Improvement here is "dust in the wind".

The runtime of arrays should be a bit faster than formlists.

[candlepin]

candlepin wrote: "3)Â Poor man's threading?"

Â

In case you don't have a quest with aliases, splitting the formlist to a couple of smaller once should increase the script runtime by using different script names.

There are two very strange ways provided by Bethesda https://www.creationkit.com/index.php?title=Creating_Multithreaded_Skyrim_Mods

but no fear you don't have to use such a construct to make runtime faster.

Â

Unfortunately you don't give us the whole script, only a short function. Improvement here is "dust in the wind".

The runtime of arrays should be a bit faster than formlists.

I don't have access to the full script right now, but can post it later. This is how I have things set up:

 

Spell, magic effect, and attached script. This script does the actual switching of ingredients. This can either be done manually (casting the spell) or called in response to opening a crafting station (see below). There really isn't much more to the script than the function, although I can post it later.

 

Quest with optional aliases for crafting stations. Activation of these aliases calls a script in the player alias (via state change).

 

Quest with player alias. Script attached. When state change happens (upon activation of a crafting station), this script calls the swap spell on the player depending upon a global variable check (no need to swap items if they are already in the correct form).

[ReDragon2013]

my suggestion as follow, cast a spell to the player (which has a magic script effect) and 10 objectReference script which runs all on the baseobject of the spawned container

 

cpPlayerEffectScript

 

Scriptname cpPlayerEffectScript extends ActiveMagicEffect
; problem: How to remove the temporary placed container?

  FormList PROPERTY ListA auto        ; all 200 items you like to swap


; -- EVENTs --

EVENT OnEffectStart(Actor akTarget, Actor akCaster)
    ; MiscSack02Small_NoRespawn "Sack" [CONT:0010E889]
    objectReference oRef = akTarget.PlaceAtMe( Game.GetForm(0x0010E889) )        ; oRef = SwapContainer

    akTarget.RemoveItem(ListA, 999, TRUE, oRef)

;;;    oRef.Delete()
;;;    oRef = None
ENDEVENT

; https://www.creationkit.com/index.php?title=RemoveItem_-_ObjectReference
;; Function RemoveItem(Form akItemToRemove, int aiCount = 1, bool abSilent = false, ObjectReference akOtherContainer = None) native

 

 

 

 

cpSwapContainerScript

 

Scriptname cpSwapContainerScript extends ObjectReference
;Scriptname cpSwapContainer01Script extends ObjectReference
; ..
;Scriptname cpSwapContainer09Script extends ObjectReference

  FormList PROPERTY ListC auto        ; ListC.GetAt(0) is the incoming player item
  FormList PROPERTY ListB auto        ; ListB.GetAt(0) is the swap item for giving back

 ; for example: You have 200 items in ListA, make new formlists (ListC, ListB)
 ;              to split ListA in 10 scripts so you have 20 items or minor left


; -- EVENTs --

EVENT OnInit()
; https://www.creationkit.com/index.php?title=AddInventoryEventFilter_-_ObjectReference
    self.AddInventoryEventFilter(ListC)
    RegisterForSingleUpdateGameTime(0.0)
ENDEVENT


EVENT OnUpdateGameTime()
    Debug.Trace("cpSwapCS: has been reached..  " +self)
    ; whatever you like here else
ENDEVENT


EVENT OnItemAdded(Form akBaseItem, Int aiItemCount, ObjectReference akItemReference, ObjectReference akSourceContainer)
; akSourceContainer == Game.GetPlayer() as ObjectReference

;IF (akBaseItem as MiscObject) || (akBaseItem as Weapon) || (akBaseItem as Armor)
    myF_Action(akBaseItem, aiItemCount, akSourceContainer)        
;ENDIF
ENDEVENT


; -- FUNCTION --

;--------------------------------------------------------
FUNCTION myF_Action(Form fm, Int n, ObjectReference oRef)
;--------------------------------------------------------
    int i = ListC.Find(fm)
    IF (i >= 0)
        oRef.AddItem(ListB.GetAt(i), n, TRUE)       ; back to the player
        self.RemoveItem(fm, n, TRUE)                ; remove from swapcontainer
    ENDIF
ENDFUNCTION

 

 

[candlepin]

Instead of spawned containers, could I use specific placed containers (maybe called via an alias)? Dedicate one container per formlist pair?

[foamyesque]

The problem with the remove-by-formlist approach is that, if I recall what you've said about this mod correctly, you want to add a varying number of items to the player. FormList adding will add the same count of every item on the form list.

 

There's a few approaches you could take here. For the fastest possible inventory walking, SKSE provides a GetContainerForms() function, which returns a Form[] array of every item, which you can then iterate through to pull the counts of, then do the swap. However, this is still going to be slow on large inventories.

 

I would instead take a slightly different approach and do preprocessing. Specifically, I would use OnItemAdded and OnItemRemoved events on a player alias -- coupled with inventory filters so that you only listen for the items you care about -- to keep track of how many items the player currently has. You'd want to do a check OnInit as well, of course, for the case of someone starting your mod up mid-stream, and sanity checks now and then, to guard against stack dumps breaking the tracking.

 

In essence, every time one of the items you're going to swap is added to, or removed from, the player, you add or remove its swapped version to a chest (seeded, of course, with the results of the initial inventory check/sanity checks). Then, when someone opens the crafting menu, you change the inventory filters to be an empty formlist, so no events are caught, you pull the entire chest inventory you want in, then dump everything currently in their inventory into the other chest. Then you re-enable the filters, and as they craft from their inventory they will also be drawing down the items you moved into the hidden chest. And then when they close the menu, you again turn off the filters, shift the chest inventories around, and turn them on again.

 

This will somewhat slow down any operations to do with those particular items, but that's preferable to one really long hang in one place.

[cdcooley]

Your solution in post number #3 with a persistent swap container in some holding cell should be the fastest and least CPU intensive.

 

The aiItemCount value will be the number of that particular item actually transferred even though you said to transfer up to 999.

 

I wouldn't use a spawned container that's just adding overhead to create and destroy an object.

 

I would use a single "SwapContainer.RemoveAllItems()" call in the script applied to the player instead of removing each item type individually in the container's script. You can do that with a small delay after transferring the items or if this swapping function is going to run periodically on the player you could do the RemoveAllItems call just before you transfer the items. If a batch stays in the container for a while it won't hurt anything. You just don't want the number of things in the container to keep accumulating forever.

 

And depending on how often you want to do that swapping you may be better off handling the swap feature by using an inventory filter and OnItemAdded event directly from a ReferenceAlias script for an alias referencing the player. (In that case you would need to remove items from the chest one at a time).

 

But I wouldn't use the OnItemAdded event on the player strategy unless it's critical that you swap out items as soon as they are added to the player. Just using the event system and swap container method should be fast enough.