SSE Trying to get cumulative value of items in a chest

[WidgitLabs]

As the title says, trying to get cumulative value of all items in a container. The following script works (sometimes), but it isn't consistent. With only one or two items, it works. Beyond that, it starts to fail at a seemingly random point. The 'wait' command was added to see if it was a race condition issue, but it doesn't seem to be. Any thoughts would be appreciated.

Scriptname DonateChestScript extends ObjectReference  

; Aliases


; Imports
import Utility

; Functions and events

Event OnClose(ObjectReference akActionRef)
	Int itemIndex = 0
	Int itemValue = 0
	Int totalValue = 0
	Int totalItems = self.GetNumItems()
	

	If totalItems > 0
		Debug.Notification(totalItems + " item(s) donated!")

		While (itemIndex < totalItems)
			Form checkItem = self.GetNthForm(itemIndex)

			itemValue = getPrice(checkItem, self)
			totalValue += itemValue

			Debug.Notification(checkItem.GetName() + " is worth " + itemValue)
			itemIndex += 1

			Wait(1)
		EndWhile

		Debug.Notification("Total value: " + totalValue)
	Else
		Debug.Notification("No items donated!")
	EndIf
EndEvent


Int Function getPrice(Form checkItem, ObjectReference akActionRef)
	Int itemValue = 0
	Int itemCharge = 0
	Int enchantmentValue = 0
	Enchantment itemEnchantment = None
	ObjectReference checkItemObject = akActionRef.RemoveItem(checkItem, 1)

	If (checkItem.getType() == 41)
		itemEnchantment = (checkItem as Weapon).getEnchantment()

		If (itemEnchantment)
			itemCharge = checkItemObject.GetItemCharge() as Int
			enchantmentValue = (itemEnchantment.getGoldValue() * 8) + (itemCharge * 0.12) as Int
		EndIf
	ElseIf (checkItem.getType() == 26)
		itemEnchantment = (checkItem as Armor).getEnchantment()

		If (itemEnchantment)
			enchantmentValue = itemEnchantment.getGoldValue()
		EndIf
	EndIf

	if (itemEnchantment)
		itemValue = checkItem.getGoldValue() + enchantmentValue
	else
		itemValue = checkItem.getGoldValue()
	endIf
	
	return itemValue
endFunction

Edited June 5, 2018 by WidgetInteractive

[cdcooley]

One obvious problem is that you're looping from 0 to totalItems and you're also removing items from the container at the same time. That will skip at least half of the items in the container because if you remove item zero what was item one shifts down to slot 0. Another is that you're only removing one item from each group of stacked items.

 

So just changing your loop to run the other direction will help.

Event OnClose(ObjectReference akActionRef)
	Int itemIndex = self.GetNumItems()
	Int itemValue = 0
	Int totalValue = 0

	If itemIndex > 0
		Debug.Notification(itemIndex + " item(s) donated!")

		While (itemIndex > 0)
			itemIndex -= 1
			Form checkItem = self.GetNthForm(itemIndex)

			itemValue = getPrice(checkItem, self)
			totalValue += itemValue

			Debug.Notification(checkItem.GetName() + " is worth " + itemValue)
		EndWhile

		Debug.Notification("Total value: " + totalValue)
	Else
		Debug.Notification("No items donated!")
	EndIf
EndEvent

But if you do that you'll also need to add a loop in the getPrice function to make sure you take care of all of the items in a stack.

 

Since you are trying to empty the container completely you can also simply take care of those two problems by changing the nature of your loop.

Event OnClose(ObjectReference akActionRef)
	Int itemValue = 0
	Int totalValue = 0
	Int totalItems = 0


	While (GetNumItems() > 0)
		Form checkItem = GetNthForm(0)

		itemValue = getPrice(checkItem, self)
		totalValue += itemValue
		totalItems += 1

		Debug.Notification(checkItem.GetName() + " is worth " + itemValue)
	EndWhile


	If totalItems > 0
		Debug.Notification(totalItems + " item(s) donated!")
		Debug.Notification("Total value: " + totalValue)
	Else
		Debug.Notification("No items donated!")
	EndIf
EndEvent

But if there's any reasonable number of items (say a stack of arrows, common ingredients, etc.) then it's going to be extremely slow to cycle through them one by one.

 

Tricks for speeding it up include checking the type of the object before finding the price and if it's something that can't be enchanted just getting the value for one item and then multiplying that by GetItemCount() and removing them all as a group. Similarly if you have a potentially enchantable object but there's no enchantment then any remaining object of that type in inventory won't have an enchantment either so you can remove any remaining items of that type as a group. And finally, use "if (checkItem as Weapon)" instead of "if (checkItem.getType() == 41)" (and the same for Armor) because just casting to type Weapon or Armor gives you the same information without taking an entire frame to do it. Those two calls to getType() are expensive calls that aren't needed.

[WidgitLabs]

Well I feel silly... that's a stupid obvious solution. Thanks!

[cdcooley]

Welcome to the club. It took me an unreasonable amount to time to realize what was happening the first time I ran into that same problem.

[WidgitLabs]

Yep... had a major duh moment when I looked at the code again. That's programming 101 and for some reason it wasn't obvious when I wrote that function. Oh well.