Mannequin Duplication bug - KILL IT WITH FIRE

Post » Wed Jun 20, 2012 6:10 pm

Ok i worked on this script all night and now i officially ****ing HATE mannequins

Background info:

for those that don't know, the "dupe" bug is a glitch caused by an event paradox while the inventory of the mannequin is still open. Essentially, any time you add an item, the event block OnItemAdded() will be queued in a cache and hold position until AFTER the menu closes, in which case it will fire off the length of code contained inside the function. That's all fine, except.... if you remove the item from the mannequin before closing the menu. The event block OnItemRemoved() will fire in its entirety WHILE the menu is open, hence the paradox. So what ends up happening is, if you add an armor, then remove it before closing the menu, the OnItemAdded() will fired from its queued AND cached status regardless if the item has already been removed.

As you know, the OnItemAdded on the mannequin script will add said item to the slot system (regardless if you use the vanilla script or an array system). The next time the mannequin fires off the EquipCurrentArmor() function, it will incorrectly force-equip this rogue armor piece onto itself, and since it is now out of sync with the rest of the script, it will continue to force-equip a duplicate of this same offending armor infinitely, every single time the Event OnCellLoad() is called (or OnCellAttach, whatever you have on the script). This is because, even though the mannequin did not physically have the offending armor piece in its inventory at the time, since the form ID is stored in the slot system, it will be "given" this armor to equip (this is a standard feature in skyrim).

It is also important to note that the OnItemAdded is suspended in a cached state because, even if you try to apply a failsafe to the OnItemRemoved() event block, it will have no effect on the OnItemAdded() function call when it eventually fires, since it will only return the value of the variable in its cached state.



Edit: updated script
Spoiler

Scriptname DCVR_AH_MannequinActivatorScript extends ActorImport UtilityMessage Property MannequinArmorWeaponsMessage  AutoForm[] Property ArmorSlot  Auto HiddenForm Property EmptySlot  Auto HiddenEvent OnInit()ArmorSlot = New Form[30]EndEventEvent OnCellAttach()Self.BlockActivation()EquipCurrentArmor()MoveTo(GetLinkedRef())Self.EnableAI(FALSE)EndEventEvent OnActivate(ObjectReference TriggerRef)Self.OpenInventory(TRUE)MoveTo(GetLinkedRef())Self.EnableAI(FALSE)EndEventEvent OnItemAdded(Form akBaseItem, int aiItemCount, ObjectReference akItemReference, ObjectReference akSourceContainer)Actor akPlayer = Game.GetPlayer()If ((akBaseItem as Armor) || (akBaseItem as Weapon) || (akBaseItem as Ammo))  AddToArmorSlot(akBaseItem)Else  MannequinArmorWeaponsMESSAGE.Show()  Self.RemoveItem(akBaseItem, aiItemCount, true, akPlayer)EndIfRegisterForSingleUpdate(0.1)EndEventEvent OnItemRemoved(Form akBaseItem, int aiItemCount, ObjectReference akItemReference, ObjectReference akSourceContainer)RemoveFromArmorSlot(akBaseItem)RegisterForSingleUpdate(0.1)EndEventEvent OnObjectUnequipped(Form akBaseObject, ObjectReference akReference)RemoveFromArmorSlot(akBaseObject)EndEventEvent OnUpdate()EquipCurrentArmor()EndEventFunction EquipCurrentArmor()Int i = 0Self.UnequipAll()While (i < 30)  If (ArmorSlot[i] != EmptySlot)   Self.EquipItem(ArmorSlot[i])  EndIf  i += 1EndWhileEndFunctionFunction AddToArmorSlot(Form akBaseItem)Int i = 0Bool FoundEmptySlot = FalseWhile (i < 30) && (FoundEmptySlot == False)  If (ArmorSlot[i] == EmptySlot)   ArmorSlot[i] = akBaseItem   FoundEmptySlot = True  EndIf  i += 1EndWhileEndFunctionFunction RemoveFromArmorSlot(Form akBaseItem)Int i = 0Bool FoundMatchingSlot = FalseWhile (i < 30) && (FoundMatchingSlot == False)  If (ArmorSlot[i] == akBaseItem)   ArmorSlot[i] = EmptySlot   FoundMatchingSlot = True  EndIf  i += 1EndWhileEndFunctionFunction ResetMannequinVars()Int i = 0While (i < 30)  ArmorSlot[i] = EmptySlot  i += 1EndWhileEndFunction
User avatar
Louise
 
Posts: 3407
Joined: Wed Nov 01, 2006 1:06 pm

Post » Wed Jun 20, 2012 9:58 pm

Your script looks like it works, congratulations! However, your explanation of the bug confused me, so I opened up the mannequin script and took a look for myself. This is a more in-depth explanation of the bug:

OnItemAdded:

Calls the AddToArmorSlot function, logging the item by using the properties.

Force equips the item. Equipping an item not in possession of the actor will create the item in actor's inventory)

In this case it doesn't seem to create any bugs.


OnObjectUnequipped:

Calls the RemoveFromArmorSlot function, removing the item from the properties.


OnCellLoad:

Calls the EquipCurrentArmor function, force-equipping all items logged in the properties.

This is the reason for the duplication bug.



The force-equip in OnItemAdded checks whether or not the mannequin has the item (it does) to determine if the item should be created in the inventory (it shouldn't), and then queues the item to be equipped once out of menu mode.

Removing the item before closing the menu does nothing, because the script uses an OnObjectUnequipped event instead of OnItemRemoved. So OnObjectUnequipped does not run when you add an item to the inventory and then remove it before closing the menu (the item wasn't equipped yet).

Once the menu is closed, the force-equip from the OnItemAdded will try to equip the item. Since the item doesn't exist in the inventory, and the check for whether or not there actually is an item in the inventory has passed, nothing happens.

When the EquipCurrentArmor function is called, the mannequin will force equip all the armor logged in the properties. Since the item that was removed is still in the properties (OnObjectUnequipped never ran, remember?) and the mannequin doesn't actually have the item in its inventory, the item is created (item duplication!) and then equipped.

So actually, a quick fix to the original script would be to just change "OnObjectUnequipped" to "OnItemRemoved".
User avatar
Sanctum
 
Posts: 3524
Joined: Sun Aug 20, 2006 8:29 am

Post » Wed Jun 20, 2012 8:45 pm

firstly, changing to onitemremoved will break the slot system when you equip a an armor that force unequips another of the same type (for example, equipping a second cuirass will unequip the existing one without unregistering it from the armor slot system)


secondly, when you remove an item in the menu, the even onitemadded is cached, i stress the word cached here as this is what causes the bug to kickstart its domino effect.

even without an item removed block, and with the vanilla unequip block, on paper it looks as if the code is kosher, but in reality the way that the events fire out of order and in the suspended cache state, it will execute the OnItemAdded block as if the item is in the inventory of the mannequin when in fact it is not. trust me, i have tested this a million times.

the reason why it keeps duping it over and over again is because when the EquipArmor function is called it is pulling the form ID from the slot list and equipping that form from thin air (since it does not exist in its inventory. this happens during the OnCellLoad block.

and of course as it "gives" itself this armor, it self perpetuates the glitch because it is also firing off the OnItemAdded block each time it gives itself a new armor.

the OnUnequip event block (or even a preventative OnItemRemoved block) has absolutely no effect to a cached OnItemAdded function call (because that script is executed in a cached suspended state prior to any of those events or functions taking effect regardless if they fire before the actual OnItemAdded code is executed, hence why all values return incorrectly true).


you can test this yourself by using a simple variable check.


in the OnItemAdded put a condition for it to only fire the AddToArmor if the variable returns a value of 0 (default value)

then create an OnItemRemoved, and inside that function, set the variable to a value of 1.

so in game you add an armor to the mannequin, then remove it before closing out the menu. from what we know the OnItemRemoved will change the variable's value to 1 before the OnItemAdded's code can finish (which only executes when the menu closes)

on paper it looks as if the condition should return false because we changed the value to 1, but in reality, since it is cached, it will return a value of 0 (the value that was initialized when the OnItemAdded function first started, and went into suspension)

in short, OnItemAdded does not execute in real time, which is why i had to add the extra array + the OnUpdate failsafe. OnUpdate will always execute in real time, but needs the array since it can only fire once (in case the user adds multiple items at once, the array is needed to store the multiple form IDs)
User avatar
Steve Fallon
 
Posts: 3503
Joined: Thu Aug 23, 2007 12:29 am

Post » Wed Jun 20, 2012 3:32 pm

OK, I agree with you when you say that the quick fix I suggested would break the slot system. I disagree with you when you say that the OnItemAdded event is cached, unless I'm totally misunderstanding what you mean by that.

I made a video showing a test I just did for the mannequins. It shows a couple of changes to the mannequin script that fixes the duplication bug, and it should even address the problem you brought up with the fix I suggested. And it should also prove that the OnItemAdded event isn't cached.

http://youtu.be/pXRXzIVA9UU

EDIT: I also meant it when I said congratulations in my first post. Your script looks good, and I would definitely use it.
User avatar
Charles Weber
 
Posts: 3447
Joined: Wed Aug 08, 2007 5:14 pm

Post » Thu Jun 21, 2012 5:06 am

in my original test i had mine set up similarly with the onitemremoved.

the main difference was that i had EquipCurrentArmor() called at the end of the OnItemAdded block which would never fire until after the menu closed. this is what i meant by cached. even though it runs through the block, it is not in real time. otherwise the mannequin would equip each piece as you add it.

i had a debug messagebox inside the equiparmor function which returned a value of a variable (default value = http://forums.bethsoft.com/topic/1367712-mannequin-duplication-bug-kill-it-with-fire/0) which was set during the onitemremoved function (value set to 1)

if it executed in real time it should have displayed the current variable (value = http://forums.bethsoft.com/topic/1367712-mannequin-duplication-bug-kill-it-with-fire/1), instead it displayed value = 0

i don't trust onitemadded and onitemremoved to function properly and in order, if some of the code is held in a cached state waiting until after the menu closes

if it works, it works, but i just cant trust it if the equiparmor wont fire in real time



Edit: i don't mean to come off as defensive, i just really want to get to the bottom of this (its driving me insane! LOL)
ideally i would love to use a simplified script, as the one i posted is much like the title - killing a bug using a flamethrower. if i can use a shoe to kill the bug, i would
User avatar
Christie Mitchell
 
Posts: 3389
Joined: Mon Nov 27, 2006 10:44 pm

Post » Wed Jun 20, 2012 3:37 pm

HA! turns out all that was needed to offset the "cache" error was to call EquipArmor in the onItemRemoved as well, so that in the cache queue, the last call wins (and guarantees a more updated function call)

this definitely cuts out a lot of fat. dont need the update and the extra array
User avatar
Jennifer Munroe
 
Posts: 3411
Joined: Sun Aug 26, 2007 12:57 am

Post » Wed Jun 20, 2012 4:01 pm

I actually liked how you called EquipCurrentArmor from the OnUpdate event. But whatever works! :thumbsup:
User avatar
Kelsey Hall
 
Posts: 3355
Joined: Sat Dec 16, 2006 8:10 pm

Post » Wed Jun 20, 2012 5:56 pm

HA! turns out all that was needed to offset the "cache" error was to call EquipArmor in the onItemRemoved as well, so that in the cache queue, the last call wins (and guarantees a more updated function call)

this definitely cuts out a lot of fat. dont need the update and the extra array


here is what i ended up with (THANK YOU randomnoob, this is much simpler)

Great work folks. I take it this works the exact same as the vanilla, only without the dupe bug.
If so, would you mind if I included it in a mod ?

Credit given where credit's due :D
User avatar
Robert DeLarosa
 
Posts: 3415
Joined: Tue Sep 04, 2007 3:43 pm

Post » Thu Jun 21, 2012 5:34 am

this fixes 5 issues with the vanilla script

1. OnCellAttach - better stability for wandering mannequin
2. Dupe bug
3. Naked bug
4. raises armor slot limit from 10 to 30 (a lot of armors coming out now are using extra body nodes and 10 is no where near enough)
5. allows weapons and arrows to equip also

you can use and upload this script if you want (thats why i shared it publicly here), but make sure you also credit SluckyD and Daemonjax because some of their code is also in this script
SluckyD's mannequin fix script - http://skyrim.nexusmods.com/downloads/file.php?id=10652


the difference between his and mine is that i didn't cap the mannequin inventory max of 10 items. mine can still allow unlimited mannequin inventory and 30 max simultaneous equips

also, if you plan on using this as a vanilla script fix, you need to have code in the Init to covert to the array (refer to sluckyD's script). if you plan on using this for standalone mannequins, just make sure the script is named something differently (whatever name of your script in the header)

one last thing, you will also either need to create a new warning message that says "place only weapons and armor on mannequin" or edit the existing one if doing a vanilla fix (or just delete the message property and use a debug messagebox to avoid have to use an esp)
User avatar
Albert Wesker
 
Posts: 3499
Joined: Fri May 11, 2007 11:17 pm

Post » Wed Jun 20, 2012 4:31 pm

Thanks, that's great AD. I'll use it as a standalone.
User avatar
Horror- Puppe
 
Posts: 3376
Joined: Fri Apr 13, 2007 11:09 am


Return to V - Skyrim