Stability of RemoveAllItems?

[Eillis]

I am trying to temporarily exchange all items of Player and any NPC (including, or rather: especially companions).

Soon after, I perform the exchange again to give all the items back to the original owner.

 

I tried a few ways to do this, but so far only RemoveAllItems worked for me.

(Of course, I had to make use of a persistent container to perform the exchange.)

 

I tried using manual iteration in different ways but nothing worked for me.

(Possibly, I was doing something wrong.)

 

Anyway, it all looks good so far, but I am wondering about quest items or scripted items from various mods.

The wiki page mentions some issues with quest items.

Actually, I don't care about quest items much. I totally wouldn't mind leaving them where they are.

 

Is there a chance that there could be bugs or stability issues caused by using RemoveAllItems (for transfer) on player or NPCs?

Edited September 19, 2022 by Eillis

[RomanR]

Well, CS Wiki mention a problem with enchantments if this command is used on quest items as they will be unequipped but enchantment will be permanetly attached. You will not be able to equip this item again too. Regarding stability with scripted items I think it will depend on what script does. Some scripts are ment to run only on player and sometimes there aren't any checking in them. This could lead to problems. Maybe it's better to disable "playable" flag (only by OBSE) for these type of items.

 

Personaly I use "Quest Item" flag as "don't touch me, please" message for other scripts and if I need it to remove this item, I'm doing it by using RemoveMe command in script attached to it. Never had problem with stability this way.

[Eillis]

I do actually iterate to unequip all items from both player and the NPC before I call RemoveAllItems.

I'm guessing that this should safeguard me from the enchantment problem.

Also, I understand that the enchantment bug does not occur since OBSE 21. (?)

 

I also thought that using quest item flag is an intuitive way to say "don't touch me". I'm guessing many mods would use that.

 

I do wonder about "Quest items are not removed when this function is called on the player".

It sounds like quest items will get removed from the NPC, given to player and then they won't be transferred back.

I'll have to test this later.

 

I did try to use all those Remove functions in all combinations but I couldn't get them to work.

Maybe there was something I missed though...

 

To clarify, I am running all my code from Activator.

Maybe that is what's complicating things.

[GamerRick]

'RemoveAllItems player' will cause the OnAdd block of scripted items to be called. So do AddItem and AddItemNS. RemoveMe and RemoveMeIR do not cause the OnAdd block to fire and can thus be a problem for quest items that use this common method for determining when the player takes a quest item.

 

If you want any help, you are going to have to show us your code and tell us what you are trying to do with it.

Edited September 19, 2022 by GamerRick

[Eillis]

As I understand:

- AddItem creates new items.

- RemoveMe and RemoveAllItems transfer the existing items (when providing a target container).

 

That means that OnAdd block behavior is very counter-intuitive in this case.

RemoveMe and RemoveAllItems do pretty much the same thing, except one of them will call the block and the other won't.

 

Are you certain that this is how it works though?

Wiki states that "quest items are not removed when this function is called on the player".

So they are in fact removed? Or is OnAdd called despite them not being removed?

 

 

As I mentioned, my code works fine so far. I have no issue with it.

I am only asking about any general pitfalls and gotcha's of transferring items - potential problems that you wouldn't notice at first.

Maybe I'm more worried than it's worth, but I just felt that it may be risky to move quest and scripted items indiscriminately.

 

I mean to say that I'd rather focus on general tips and advice rather than specifically on my code.

 

Having said that, of course I don't mind sharing my code, so here it is.

I am sending all player items to a container, then transferring all NPC items to player. Next, I open a menu. After closing the menu, all items are returned to their owners.

So, in other words, I want player to hold NPC's items while the menu is active.

Begin GameMode

	if phase == 0
	
		; Unequip all player items.
		Let playerEquipment := Player.GetEquippedItems
		Let i := ((Ar_Size playerEquipment) - 1)
		while i >= 0
			Let tempRef := playerEquipment[i]
			if tempRef != 0
				if IsQuestItem tempRef == 0
					Player.UnequipItemSilent tempRef 0
				else
					Ar_Erase playerEquipment i
				endif
			endif
			set i to (i - 1)
		loop
		
		; Unequip all companion items.
		Let companionEquipment := companion.GetEquippedItems
		Let i := ((Ar_Size companionEquipment) - 1)
		while i >= 0
			Let tempRef := companionEquipment[i]
			if tempRef != 0
				if IsQuestItem tempRef == 0
					companion.UnequipItemSilent tempRef 0
				else
					Ar_Erase companionEquipment i
				endif
			endif
			set i to (i - 1)
		loop
		
		; Transfer items.
		Player.RemoveAllItems ECTCompanionTransferContainer 0
		companion.RemoveAllItems Player 0
		
		set phase to 1
	endif
	
	; -----------------
	; Phase 1 is in the menus.
	; Switches to phase 2 when finished.
	; -----------------
	
	if phase == 2
	
                ; Transfer items back.
		Player.RemoveAllItems companion 0
		ECTCompanionTransferContainer.RemoveAllItems Player 0
		
	        ; Re-equip player items.
		Let i := ((Ar_Size playerEquipment) - 1)
		while i >= 0
			Let tempRef := playerEquipment[i]
			if tempRef != 0
				Player.EquipItemSilent tempRef 0
			endif
			set i to (i - 1)
		loop

                ; Re-equip companion items.
		Let i := ((Ar_Size companionEquipment) - 1)
		while i >= 0
			Let tempRef := companionEquipment[i]
			if tempRef != 0
				companion.EquipItemSilent tempRef 0
			endif
			set i to (i - 1)
		loop
		
		set phase to -1
	
	endif
	
End

Edited September 20, 2022 by Eillis

[RomanR]

I would add some note - I learned that it's better to check if all items are unequipped in next frame before I will manipulate them further. It helped me to avoid some problems with magical items.

 

Some items are made to be unequipable, I remember dreamworld amulet which acts this way (quest item too).

 

And lastly I never tested if a quest item will be removed from NPC this way. But for your case it means that even if you transfer it to player, you can't transfer it back right?

[GamerRick]

Something I spotted right away is that you are deleting items from the same array you are looping over without accounting for that.: Ar_Erase companionEquipment i

 

Then I spotted: if IsQuestItem tempRef == 0

It should be (see this link): if tempRef.IsQuestItem == 0

Most OBSE commands expect a Ref-ID on the left and a BaseID on the right. It's sometimes important to know if you are checking the reference item or the base item.

 

In your case, what does the GetEquippedItems command return? The reference to that specific item? Or the BaseID? If it's the former you would need to set another reference to baseid of the reference with something like:

ref tempBase
set tempBase to tempRef.GetBaseObject

I have found that the script will crash sometimes if I try to use a reference where it expects a baseid. However, the GetBaseObject command will crash the script if it's already a baseid. So, you need to know what is in that array.

ar_Dump playerEquipment

Edited September 20, 2022 by GamerRick

[RomanR]

@GameRick:

 

Using GetEquippedItems this way is completely fine. This function returns list of equipped items as Base Objects rather than inventory references (as you already figured). Maybe you're puzzled because of naming variable as tempRef. The advantage of this command is that it's already checking the inventory for you and as compiled it will be always faster than any hand-made scripted loop. The limits are obvious - if you want check more than Base Object can provide, you must stroll inventory by yourself.

Edited September 21, 2022 by RomanR

[Eillis]

Yes. It's just as RomanR says. GetEquippedItems returns object IDs.

 

Removing items during iteration is also fine, since I am iterating backwards.

 

 

 

I would add some note - I learned that it's better to check if all items are unequipped in next frame before I will manipulate them further. It helped me to avoid some problems with magical items.

 

Some items are made to be unequipable, I remember dreamworld amulet which acts this way (quest item too).

 

And lastly I never tested if a quest item will be removed from NPC this way. But for your case it means that even if you transfer it to player, you can't transfer it back right?

 

 

Could you elaborate about the problems you mentioned?

It is actually quite important for me to get all items transferred without skipping frames, if possible.

I'd be very interested to know any possible risks and consequences of doing it all in one frame.

 

 

I did some testing with RemoveAllItems transfer.

Here are my findings:

 

- unplayable items are never transferred,

- quest items are not transferred from player. They are transferred from other containers though.

 

As we expected, NPC quest items get transferred from NPC to the player but they won't be transferred back.

 

I haven't tested scripted item blocks extensively.

The small tests I did on OnAdd, OnDrop, OnEquip, OnUnequip blocks behavior seemed weird.

OnUnequip block was called very often. Other blocks rarely or not at all.

Although, it seems that block calling behavior was not much different when I tried to use RemoveAllItems and RemoveMeIR. Both of them seemed weird.

Perhaps the real issue here is that I am doing all the unequipping and transfer at once.

Anyway, I can't say anything for certain here, as I didn't focus on it yet.

 

Speaking of RemoveMeIR, I tried using it instead of RemoveAllItems but it was causing bugs.

Very often, RemoveMeIR was duplicating stackable items. (It's an issue I saw mentioned in other forum threads too.)

At other times, some items were skipped and not transferred at all.

 

 

Going back to RemoveAllItems, it seems quite reliable as far as item transfer goes - no duplication or other such bugs.

There are the specific quirks that I mentioned, which can possibly be worked around or taken advantage of.

I try to manipulate "playable" and "quest item" flags to control which items to transfer and which not. (Of course, after the transfer I restore flags to their original state.)

If item is an armor or clothing, transfer can be prevented by setting the unplayable flag.

If item is on player, transfer can be prevented by setting the quest item flag.

If neither of these two cases, transfer can't be prevented. Best I can do is let the transfer happen and just avoid NPC quest items getting stuck on player - I temporarily deactivate their quest item flag.

 

Summing all that up, I am only left with a problem that NPC's quest items will temporarily be moved to the player.

Well, that and the chaotic behavior of scripted item blocks. (But I might still get to look more into it.)

 

Extra note: I looked up script of CM Partners. It uses plain RemoveAllItems, like it's no big deal.

They only do it between NPC and a container though.

 

 

As I mentioned, I constantly get issues (item duplication, skipping some items) when trying to move items with RemoveMeIR.

Has anyone been able to use this function to transfer items reliably, with no bugs? Maybe there's something I'm missing...

I tried using "foreach itemRef <- container" and "GetInvRefsForItem". Both caused the same issues.

 

 

If anyone needs it, here are the two functions that I wrote for transferring items.

scn ECTTransferItemsAllFunction

ref transferFrom
ref transferTo

array_var restoreQuestItems
array_var restoreUnplayableItems

array_var arrayRow
ref itemTypeRef

Begin Function{transferFrom, transferTo}

    Let restoreQuestItems := Ar_Construct Array
    Let restoreUnplayableItems := Ar_Construct Array
    
    foreach arrayRow <- transferFrom.GetItems
        Let itemTypeRef := arrayRow["value"]
        if IsPlayable2 itemTypeRef == 0
            ; Set item as playable to allow transfer.
            SetIsPlayable 1 itemTypeRef
            Ar_Append restoreUnplayableItems itemTypeRef
        endif
        if (IsQuestItem itemTypeRef == 1) && (transferFrom.GetIsReference Player == 1)
            ; Set player quest item as non-quest to allow transfer.
            SetQuestItem 0 itemTypeRef
            Ar_Append restoreQuestItems itemTypeRef
        endif
    loop

    transferFrom.RemoveAllItems transferTo 0

    ; Restore quest item states.
    foreach arrayRow <- restoreQuestItems
        Let itemTypeRef := arrayRow["value"]
        SetQuestItem 1 itemTypeRef
    loop

    ; Restore unplayable states.
    foreach arrayRow <- restoreUnplayableItems
        Let itemTypeRef := arrayRow["value"]
        SetIsPlayable 0 itemTypeRef
    loop

End

scn ECTTransferItemsSkipSpecialFunction

ref transferFrom
ref transferTo
short skipScriptedItems
array_var inOutTransferredQuestItems

array_var restoreQuestItems
array_var restoreNonQuestItems
array_var restorePlayableItems

short isSpecialItem
short isTransferrableItem
array_var arrayRow
ref itemTypeRef

; inOutTransferredQuestItems - as input: should contain list of quest items which should be allowed to transfer. (Most likely: quest items transferred to player earlier.)
; inOutTransferredQuestItems - as output: contains list of quest items, whichs transfer was not prevented.
; In practice, just provide a single, shared array here when transferring items back and forth between two containers.

Begin Function{transferFrom, transferTo, skipScriptedItems, inOutTransferredQuestItems}

    Let restoreQuestItems := Ar_Copy inOutTransferredQuestItems
    Let restoreNonQuestItems := Ar_Construct Array
    Let restorePlayableItems := Ar_Construct Array

    Ar_Erase inOutTransferredQuestItems

    ; Remove state of quest items returned from player to allow their transfer.
    if (transferFrom.GetIsReference Player == 1)
        foreach arrayRow <- restoreQuestItems
            Let itemTypeRef := arrayRow["value"]
            SetQuestItem 0 itemTypeRef
        loop
    endif

    ; Identify special items and attempt to prevent their transfer.
    foreach arrayRow <- transferFrom.GetItems
        Let itemTypeRef := arrayRow["value"]
        set isSpecialItem to ((IsPlayable2 itemTypeRef == 0) || (IsQuestItem itemTypeRef == 1) || (HasName itemTypeRef == 0) || ((skipScriptedItems == 1) && (IsScripted itemTypeRef == 1)))
        set isTransferrableItem to ((IsPlayable2 itemTypeRef == 1) && ((IsQuestItem itemTypeRef == 0) || (transferFrom.GetIsReference Player == 0)))
        if  (isSpecialItem == 1) && (isTransferrableItem == 1)
            if ((GetObjectType itemTypeRef == 20) || (GetObjectType itemTypeRef == 22))
                ; Set armor/clothing item as unplayable to prevent transfer.
                SetIsPlayable 0 itemTypeRef
                Ar_Append restorePlayableItems itemTypeRef
            elseif (transferFrom.GetIsReference Player == 1)
                ; Set player non-quest item as quest item to prevent transfer.
                SetQuestItem 1 itemTypeRef
                Ar_Append restoreNonQuestItems itemTypeRef
            elseif (IsQuestItem itemTypeRef == 1)
                ; There is no way to prevent transfer. Store item type in output.
                if eval (Ar_Size inOutTransferredQuestItems) >= 0
                    Ar_Append inOutTransferredQuestItems itemTypeRef
                    PrintToConsole "ECT: Unable to prevent transfer of quest item: [%n]: %n => %n" itemTypeRef transferFrom transferTo
                else
                    PrintToConsole "ECT: Warning! Unreported transferred quest item: [%n]: %n => %n" itemTypeRef transferFrom transferTo
                endif
            else
                PrintToConsole "ECT: Transferred special item: [%n]: %n => %n" itemTypeRef transferFrom transferTo
            endif
        endif
    loop

    ; Transfer items.
    transferFrom.RemoveAllItems transferTo 0

    ; Restore quest items state.
    foreach arrayRow <- restoreQuestItems
        Let itemTypeRef := arrayRow["value"]
        SetQuestItem 1 itemTypeRef
    loop

    ; Restore non-quest items state.
    foreach arrayRow <- restoreNonQuestItems
        Let itemTypeRef := arrayRow["value"]
        SetQuestItem 0 itemTypeRef
    loop

    ; Restore playable items state.
    foreach arrayRow <- restorePlayableItems
        Let itemTypeRef := arrayRow["value"]
        SetIsPlayable 1 itemTypeRef
    loop

End

Edited September 21, 2022 by Eillis

[RomanR]

Maybe I raised your concern for nothing, but in case of magical items i found two interesting things - when you unequip a magical item and want equip it immediately, the equip fails (it's not my discovery though). Also equipping a magical item raises OBSE's "OnEquip" event twice. It's propably due how the OBSE handles magic effects to support its own functions and for me it looks like there is some additional processing going on. This way I learned to wait for next frame.

 

However looking at your scripts I noticed that you continuing accesing items after GetItems as Base Objects, but this command returns inventory references for a change. Maybe this is a source of your problems using commands like RemoveMeIR ?