Overhauling the weapon rack scripts

[DayDreamer]

Here's my inversion of Tal's checklist, just for the shield. The 4th clause is a fix for taking a shield, with the following failures:

• (triggerRef == PlayersDroppedWeapon) is false, because triggerRef doesn't match the shield. This should have been fixed by the new starting setup. But say it's not, somehow the value is of another item that was hiding behind the shield. Well, one of them should have dropped to the floor!
• PlacedItemInit should be true by the time a scripted item could be on the rack at all, but PlayersDroppedWeapon will not be None if a shield is on the rack. So this test alone is enough. What other value could legally be in PlayersDroppedWeapon?
• iTOC will also not be 0 if any other CoA weapon is still in place. So this doesn't help anyway.

... the shield trigger initialized as soon as the player grabs a pre-placed shield, without having to empty the whole rack.

But why? In that rare instance, as long as the initialization TOC checks allow the shield to be removed (they do), then it can be removed. And then we are home free!

[Sclerocephalus]

And yet, even with that shield rack shortcut in place, a user who newly installs USKP with a set of wooden racks full of battleaxes will have the same problem: those activators will not re-enable until they empty the rack so that iTOC==0. So we're not eliminating that issue, only reducing it slightly in the particular special case of picking up a shield off a CoA rack that hasn't been initialized for USKP yet.

 

Yes, but fully occupied CoA racks are very common in the game. The only rack type where Beth almost ever filled all slots. Thus, it's not that slightly a reduction actually.

[Sclerocephalus]

We are all in agreement it's going to throw a log error in the worst case: player has stored a scripted object into a rack with an existing shield, then loads the new patch, then removes the scripted object first.

 

I'm not objecting here, and I told you both already that I will think about it when I'm free to concentrate on the scripts again. It's rather pointless at the moment to carry on with this, as there's some work left to do - unless somebody wants to help me with my collision mesh. No ? Well, ...

[DayDreamer]

And yet, even with that shield rack shortcut in place, a user who newly installs USKP with a set of wooden racks full of battleaxes will have the same problem: those activators will not re-enable until they empty the rack so that iTOC==0. So we're not eliminating that issue, only reducing it slightly in the particular special case of picking up a shield off a CoA rack that hasn't been initialized for USKP yet.

Another problem area would be 2 crossed swords, with no shield starting item. Until you remove both the swords, the current code won't let you put a shield there. (The last test clause in the setup.)

 

Yes, but fully occupied CoA racks are very common in the game. The only rack type where Beth almost ever filled all slots. Thus, it's not that slightly a reduction actually.

That's arguing contrary to your position. If the rack is fully populated, then the PlayersDroppedWeapon will have been filled by the StartingItem, and the 1st test will pass.

So there's almost no place in the game this test will help? Unless there's a very good example of an actual problem rack we can test, I'm taking it out of mine!

 

I'm not objecting here, and I told you both already that I will think about it when I'm free to concentrate on the scripts again. It's rather pointless at the moment to carry on with this, as there's some work left to do - unless somebody wants to help me with my collision mesh. No ? Well, ...

No, no, please carry on -- we've got it covered. Thanks for all your help in understanding the issue!

[Arthmoor]

I feel I should make it clear that while the efforts to sort out this mess once and for all are well worth the discussion that we are NOT taking on any major changes to the system for USKP 2.0 so it doesn't do any good to start getting more eyes on the problem when we're going forward with the 3.4 scripts as they are. It appears as though there may have been the impression that we're holding the beta for this to get done but that's not the case - we're only waiting on those meshes Sclero is working on before going live.

 

Any further changes to the system in an actual release will have to wait for our next cycle after this one.

[Sclerocephalus]

That's arguing contrary to your position. If the rack is fully populated, then the PlayersDroppedWeapon will have been filled by the StartingItem, and the 1st test will pass.

 

Do you know what crap they put on those racks ? Even at high levels, you'll get two iron greatswords and an iron shield. Consequently, nobody leaves them on the racks and sells the crap, and that is responsible for the mess: they start with three linked items, but zero information on where they are ...

 

EDIT:

Also, this is the only rack that can hold a shield, so they are pretty much all in regular use.

 

EDIT2:

If I don't find a workaround, you can take that check out - but not without having tried . There always should be very good reasons for a loss of user-friendliness.

 

EDIT3 (sorry, the coffee machine is glitching out ...):

What you can take out is the "(PlacedItemInit && (MountedItem == None)" check. As you may already have noticed, this check is complete nonsense, since this can never be true when the trigger is active. Well, this is a "backdoor" to hook up some of my rescue scripts (occasionally, you have to misuse racks deliberately to see how they react - and where an error could potentially come from; our users do the same - and then report it as a bug ... thus, better safe than not). While preparing a small fix for PinceShroob, I found a better way to do that though.

[taleden]

I feel I should make it clear that while the efforts to sort out this mess once and for all are well worth the discussion that we are NOT taking on any major changes to the system for USKP 2.0 so it doesn't do any good to start getting more eyes on the problem when we're going forward with the 3.4 scripts as they are. It appears as though there may have been the impression that we're holding the beta for this to get done but that's not the case - we're only waiting on those meshes Sclero is working on before going live.

 

Any further changes to the system in an actual release will have to wait for our next cycle after this one.

 

That's understandable of course, but there is one change to the scripts as packaged in the v2 beta (at least as of when I downloaded it) that really definitely should be made before release: the fail-safe counter on the loop in PlaceItem()! That loop is currently still just "While !item.Is3DLoaded() , Utility.Wait() , EndWhile" and that's going to bite more than one user if left as-is.

[taleden]

That's arguing contrary to your position. If the rack is fully populated, then the PlayersDroppedWeapon will have been filled by the StartingItem, and the 1st test will pass.

 

If the rack was fully populated while these scripts are installed, then yes, PlayersDroppedWeapon should be the only test required to filter out bogus OnTriggerLeave() events. But what if the rack was first initialized on vanilla scripts, or USKP v1.3.3 scripts, and only later revisted with these new scripts?

[Sclerocephalus]

That's understandable of course, but there is one change to the scripts as packaged in the v2 beta (at least as of when I downloaded it) that really definitely should be made before release: the fail-safe counter on the loop in PlaceItem()! That loop is currently still just "While !item.Is3DLoaded() , Utility.Wait() , EndWhile" and that's going to bite more than one user if left as-is.

 

Already prepared as v3.4a, compiled and notified Arthmoor. What we also did is to fill the base objects of all iron and steel weapons, the iron shield, the long bow and the hunting bow in the exclusion list. The UDGP will add the standard crossbow.

[Sclerocephalus]

But what if the rack was first initialized on vanilla scripts, or USKP v1.3.3 scripts, and only later revisted with these new scripts?

 

That's what's actually causing the mess: three linked items and no information left over from the vanilla scripts that could be used to determine their whereabouts.

 

I'll work on the modification to the trigger volumes before considering any script changes. If that concept works (chances are good), there won't be any check needed at all on the trigger script, not even for the trigger object count (simple logics: if only the placed item is in the trigger, TOC will always be zero anyway if that item is grabbed, so what sense would this check make then ?). Thus, thinking about workarounds might not even be necessary and therefore would be a waste of time right now.

 

And while I'm at it, I'll also add a node for the crossbow, because it looks horrible when it is moved to the bow node.

[taleden]

That's what's actually causing the mess: three linked items and no information left over from the vanilla scripts that could be used to determine their whereabouts.

 

Well, for each of those objref pointers, if IsDeleted() then they are in a container or inventory or destroyed, so they can't still be mounted. And if not IsDeleted(), then GetParentCell() should reliably tell whether they're still nearby. The only problem that remains then is if the player grabs a pre-placed item and drops it on the ground in the same cell, or mounts it on a different rack in the same cell. But I wonder if you could just use GetDistance() to handle that? If the item is still on the same rack, that should be close to 0. If it fell off on the ground it might be a little higher, but it seems to me it wouldn't be that bad to just leave it there and call it non-mounted, and let he player pick it up and re-mount it once if they want to; after doing that, it should stick no problem since PlayersDroppedItem/PlacedItemInit will be set.

[taleden]

Already prepared as v3.4a, compiled and notified Arthmoor. What we also did is to fill the base objects of all iron and steel weapons, the iron shield, the long bow and the hunting bow in the exclusion list. The UDGP will add the standard crossbow.

 

You mean the non-exclusion list, so that pre-placed iron swords don't mistakenly say they can't be mounted? That sounds like a bandaid on an underlying problem that still remains: the only thing special about a pre-placed iron sword is that it has a persistent reference. If that causes your chest test to think it can't safely be placed, then maybe nothing with a persistent reference can ever be mounted. It's true that most of those are iron and steel, but there are plenty of non-scripted artifacts (Volendrung for example) which have persistent references and the player might want to mount.

 

Edit: naturally, as soon as I hit post, something occurred to me: I think the reason the test misidentified the iron sword as problematic is because I already had one in inventory at the beginning. When I grabbed the pre-placed one, then I had two of them, and one of them was sent to the test chest and back to get its reference. Then one of them was dropped by DropObject(), and that reference was compared to the first. But I bet DropObject() dropped the other sword, and that's why the reference pointers were unequal, causing the test to think it was a scripted item. Therefore I bet the test will only misidentify persistent references if the player has more than one of that item. Luckily that avoids problems with artifacts, but I bet there will still be bug reports about not being able to mount otherwise ordinary dwarven, elven or other gear, just because the user managed to get their hands on more than one at a time, and one of them was persistent.

 

So as a workaround, instead of sending the item to the chest, then back to the player, and then trying to drop it from the player, would it work to move the chest to the player (but disable its reference so its not visible), send the item to the chest, and then drop the item from the chest? That should guarantee that the reference received by the chest is the same one that DropObject() finds, which should let your reference comparison test work as intended.

[Sclerocephalus]

You mean the non-exclusion list, so that pre-placed iron swords don't mistakenly say they can't be mounted? That sounds like a bandaid on an underlying problem that still remains: the only thing special about a pre-placed iron sword is that it has a persistent reference. If that causes your chest test to think it can't safely be placed, then maybe nothing with a persistent reference can ever be mounted. It's true that most of those are iron and steel, but there are plenty of non-scripted artifacts (Volendrung for example) which have persistent references and the player might want to mount.

 

Yes, I meant the exclusion (EDIT, God dammit) exception list.

 

 

You mean the non-exclusion list, so that pre-placed iron swords don't mistakenly say they can't be mounted? That sounds like a bandaid on an underlying problem that still remains: the only thing special about a pre-placed iron sword is that it has a persistent reference. If that causes your chest test to think it can't safely be placed, then maybe nothing with a persistent reference can ever be mounted. It's true that most of those are iron and steel, but there are plenty of non-scripted artifacts (Volendrung for example) which have persistent references and the player might want to mount.

 

Edit: naturally, as soon as I hit post, something occurred to me: I think the reason the test misidentified the iron sword as problematic is because I already had one in inventory at the beginning. When I grabbed the pre-placed one, then I had two of them, and one of them was sent to the test chest and back to get its reference. Then one of them was dropped by DropObject(), and that reference was compared to the first. But I bet DropObject() dropped the other sword, and that's why the reference pointers were unequal, causing the test to think it was a scripted item. Therefore I bet the test will only misidentify persistent references if the player has more than one of that item. Luckily that avoids problems with artifacts, but I bet there will still be bug reports about not being able to mount otherwise ordinary dwarven, elven or other gear, just because the user managed to get their hands on more than one at a time, and one of them was persistent.

 

So as a workaround, instead of sending the item to the chest, then back to the player, and then trying to drop it from the player, would it work to move the chest to the player (but disable its reference so its not visible), send the item to the chest, and then drop the item from the chest? That should guarantee that the reference received by the chest is the same one that DropObject() finds, which should let your reference comparison test work as intended.

 

 

You're right.

 

DropObject prefers unequipped items, while other functions do it the other way around (GDB!).

 

Simply unequipping the item should solve it. Equipping the item is actually never needed; that's just claimed to be required to sort out anything unequippable beforehand, then have an easy job with running GetEquippedItemType(). In my opinion, that's the most remarkable feature about the vanilla scripts: The player is happily doing the job of the lazy programmer ...

 

Anyway, to be safe, I can make it work with up to 128 items of the same kind quite easily (Would it be appropriate to display a message from the 129th item on: "You ..., why are you carrying hundreds of ... around ?").

 

 

EDIT2:

By the way, this will solve a very old bug!

 

EDIT3:

When I think about it ... much of the complexity of this is only needed because they didn't find the time to design proper triggers!

 

EDIT4:

 

Arrrghhh .... half an hour of wasted time!

 

This bug will only occur if one of the multiplicated items in your inventory is persistent. For anything that is not persistent, OnItemAdded() returns "none" anyway, and this case is already handled (these items are left through). If the item is persistent, it cannot be unique (otherwise you would have had to cheat to have more than one in your inventory), it cannot have been taken from a dummy-fed rack (since the link makes the dummy persistent but not the items it returns), and therefore is either one of the dozens of iron daggers that have that stupid DefaultDisableHavokOnLoad script attached, a preplaced weapon that was grabbed from a rack or a silver sword, but the latter should be excluded from being placed on a rack anyway (at least intermiittently, until all the most recent improvements have been implemented). I can't remember of anything else that is both persistent and has multiple instances in the game world. Thus, our exception list already covers all the cases that require special treatment.

[taleden]

Anyway, to be safe, I can make it work with up to 128 items of the same kind quite easily (Would it be appropriate to display a message from the 129th item on: "You ..., why are you carrying hundreds of ... around ?").

 

I don't understand this limitation; no matter how many copies the player has, he can only have one of them equipped, and RemoveItem(baseForm, .., chest) ought to prefer that instance, right? Then once it's in the chest, just drop it from the chest instead of moving it back to the player first, and you don't have to worry about it getting mixed up with any other copies the player has, because the chest will only ever have one at a time.

[Sclerocephalus]

and RemoveItem(baseForm, .., chest) ought to prefer that instance, right?

 

That's unfortunately not safe ...

 

From the notes on the CK Wiki: "The function seems to have a preference for equipped items. Or if not equipped items then the first instance of the item that's in inventory (which is the same thing, since when you equip something, it's the first instance of the object in the inventory)."

 

As you say, it "ought to prefer", and the behaviour is not always conclusive. DropObject, on the other hand, always selects an unequipped item. That's the reason for the long-known bug where the "wrong" item ends up on the rack when you have two of its type in your inventory and one of them equipped.

 

But read my edits above ...

 

EDIT:

Sorry, forgot about your question with the limitations: that's the maximum size of an array.

[taleden]

That's unfortunately not safe ...

 

From the notes on the CK Wiki: "The function seems to have a preference for equipped items. Or if not equipped items then the first instance of the item that's in inventory (which is the same thing, since when you equip something, it's the first instance of the object in the inventory)."

 

As you say, it "ought to prefer", and the behaviour is not always conclusive. DropObject, on the other hand, always selects an unequipped item. That's the reason for the long-known bug where the "wrong" item ends up on the rack when you have two of its type in your inventory and one of them equipped.

 

But read my edits above ...

 

EDIT:

Sorry, forgot about your question with the limitations: that's the maximum size of an array.

 

Have you ever observed RemoveItem() to not take the equipped form, if any instance of the specified form is equipped? I haven't, but you've tested this stuff more so maybe you have.

 

But it actually doesn't matter in this case; no matter which instance RemoveItem() decides to take, that will be the only instance of the form that is in the chest, so DropObject() on the chest is guaranteed to drop the same instance that was seen arriving in OnItemAdded(). That is what lets your pointer comparison work correctly to see if the item that is eventually dropped is both persistent and scripted, and should therefore be rejected (for now, until the scripted item fix is implemented). The problem with the pre-placed iron sword was that the item was moved from the chest back to the player, where it got mixed up with the other sword so DropObject() chose the wrong one.

 

Meanwhile, I understood the significance of the number 128, what I meant was why do you need an array for this at all?

[Sclerocephalus]

Have you ever observed RemoveItem() to not take the equipped form, if any instance of the specified form is equipped? I haven't, but you've tested this stuff more so maybe you have.

 

But it actually doesn't matter in this case; no matter which instance RemoveItem() decides to take, that will be the only instance of the form that is in the chest, so DropObject() on the chest is guaranteed to drop the same instance that was seen arriving in OnItemAdded(). That is what lets your pointer comparison work correctly to see if the item that is eventually dropped is both persistent and scripted, and should therefore be rejected (for now, until the scripted item fix is implemented). The problem with the pre-placed iron sword was that the item was moved from the chest back to the player, where it got mixed up with the other sword so DropObject() chose the wrong one.

 

Meanwhile, I understood the significance of the number 128, what I meant was why do you need an array for this at all?

 

As I'm not convinced that RemoveItem() always prefers an unequipped item, I would have had to test all items of this type carried by the player. Since it takes a while until the container sends the reference (the counter on a loop to monitor this, starting at zero, was always at least at 1 when the reference was finally submitted), it would have been best to let the alias script on the container collect all references in an array, then return this at once. Everything else would probably take inacceptably long.

 

EDIT:

I thought about the implications of having more than two instances of the same base object in the inventory. I recently did Dustman's Cairn again and returned with nine silver greatswords ...

 

Now, I actually can't see whether the item is persistent and scripted; the test relies on such objects returning a borked reference when dropped, which is always different from that returned by OnItemAdded(), so what I actually do is compare them and decide to exclude the item when they are different and the reference returned by OnItemAdded() is not none (it will return none for all non-peristent items). Now when I do exclude that, I still don't know whether the other item(s) behave the same, so I will have to run the test on them again.

 

EDIT:

Something else: please have a look at them; I have added the checks and a few more comments (all "neatly folded"). In case I did forget something essential after all, this is the last chance now to add it:

 

Weapon Rack Scripts for debugging_v3-4a.7z

 

[taleden]

No, I really think you're overcomplicating this. I just adapted my own copy of the scripts to use this scheme, and it works just fine. RemoveItem() never once failed to take the equipped item even when I had others in inventory, and the function to ask the container which item was dropped never had to loop even once, it had always already received OnItemRemoved().

 

Logically, the performance should be pretty much the same: in your current scheme, you RemoveItem() to the chest, wait for OnItemAdded(), then RemoveItem() back to the player, and then DropObject() on the player. With this change, you MoveTo() chest to player, RemoveItem() to the chest, DropObject() on the chest and wait for OnItemRemoved(). The only extra thing you end up having to do is SetActorOwner() on the dropped item, because I guess DropObject() sets the ownership to the chest which did the dropping.

 

I'll post my implementation of this design momentarily, just have to tidy it up a bit.

[Sclerocephalus]

No, I really think you're overcomplicating this. I just adapted my own copy of the scripts to use this scheme, and it works just fine. RemoveItem() never once failed to take the equipped item even when I had others in inventory, and the function to ask the container which item was dropped never had to loop even once, it had always already received OnItemRemoved().

 

Logically, the performance should be pretty much the same: in your current scheme, you RemoveItem() to the chest, wait for OnItemAdded(), then RemoveItem() back to the player, and then DropObject() on the player. With this change, you MoveTo() chest to player, RemoveItem() to the chest, DropObject() on the chest and wait for OnItemRemoved(). The only extra thing you end up having to do is SetActorOwner() on the dropped item, because I guess DropObject() sets the ownership to the chest which did the dropping.

 

I'll post my implementation of this design momentarily, just have to tidy it up a bit.

 

 

Err .. so actually you want only to make sure that the test runs on the equipped item ?

 

But when you have more than one item in inventory, there's still the problem that that one might need to be excluded

Nevermind, got it.

 

I need a nap!

[Sclerocephalus]

The only extra thing you end up having to do is SetActorOwner() on the dropped item, because I guess DropObject() sets the ownership to the chest which did the dropping.

 

Which I would have to run on the object reference. Which implies that I cannot do it before dropping it ...

Making the chest owned by the player should do it as well.

[Sclerocephalus]

This should do it, but a few bits are still missing:

	ObjectReference PlayerItemRef
	ObjectReference ReferenceFromQuest = None
	ObjectReference TestContainer = ReferenceTestContainer.GetReference()

	USKPWeaponRackItemHandlingScript ItemHandler = USKPWeaponRackItemHandlingQuest As USKPWeaponRackItemHandlingScript


	TestContainer.MoveTo (PlayerRef)
	TestContainer.DisableNoWait()

	ReferenceFromQuest = ItemHandler.CheckItemRef (PlayerItem)
	PlayerItemRef = TestContainer.DropObject (PlayerItem, 1)
	
        TestContainer.Enable()
	TestContainer.MoveToMyEditorLocation()
	
	If ReferenceFromQuest && (PlayerItemRef != ReferenceFromQuest)

		PlayerRef.AddItem (ReferenceFromQuest, 1, True)
		Self.Enable()

		USKPWeaponRackWeaponDisallowedMessage.Show()
		
		If (USKPWeaponRackExceptionList.HasForm (PlayerItem) == False)
			USKPWeaponRackExclusionList.AddForm (PlayerItem)
		EndIf

	Else

Does MoveTo actually work when it has no 3D ?. It's premanently persistent, but this doesn't imply that it's always loaded.

 

I'm off for testing.

[taleden]

My version is pretty similar, except I did it in a function on the item handling quest:

ObjectReference Function DropFromPlayer(Form akItem)
	ObjectReference chest = ReferenceTestContainer.GetReference()
	USKPWRTestContainerAliasScript chestScript = (ReferenceTestContainer As USKPWRTestContainerAliasScript)
	If !akItem || !chest || !chestScript
		Return None
	EndIf
	chest.DisableNoWait()
	chest.MoveTo(PlayerRef)
	chestScript.CatchAddedItem(akItem)
	chestScript.CatchRemovedItem(akItem)
	PlayerRef.RemoveItem(akItem, 1, True, chest)
	ObjectReference refOnAdd = chestScript.GetAddedItem()
	ObjectReference refFromDrop = chest.DropObject(akItem, 1)
	ObjectReference refOnDrop = chestScript.GetRemovedItem()
	If refOnDrop
		refOnDrop.SetActorOwner(PlayerRef.GetActorBase())
	EndIf
	chest.RemoveAllItems(PlayerRef, True, True)
	chest.MoveToMyEditorLocation()
	chest.EnableNoWait()
	DroppedItemWasScripted = (refOnAdd && ((refOnAdd != refFromDrop) || (refOnAdd != refOnDrop))) || (refFromDrop != refOnDrop)
	Return refOnDrop
EndFunction ; DropFromPlayer()

MoveTo() doesn't seem to mind if it's disabled, in fact if you have a reference to an item in inventory it'll even pull it out into the world with no complaint. I didn't initially have it return the chest to its start point or re-enable it, but I guess maybe that's a good idea.

 

Incidentally, dropping the item from the chest also means that the chest's OnItemRemoved() will get a valid in-world reference pointer even for scripted items. So if you just use that one instead of the return from DropObject(), then you've pretty much already implemented the scripted item fix; all you have to do then is trim down OnTriggerLeave() to not call methods on the received reference, and you also don't have to bother at all with refOnAdd or refFromDrop.

 

Also, I tried an almost identical version of the above which used an alias on the player instead of a test chest, and checked OnItemUnequipped() (triggered by player.UnequipItem()) instead of OnItemAdded() to detect persistent references which might be scripted items. Everything seemed to work just the same way, and that would avoid needing the chest at all. The only downside is that if the item is not persistent, then OnItemUnequipped() gets no pointer so you have to just DropObject() and allow that maybe it won't drop the same copy you just unequipped, although it might not matter in that case. For persistent references the behavior is identical, since you can use the unequipped reference to MoveTo() instead of calling DropObject() at all.

 

Edit: I should add, I haven't yet seen a way to detect non-persistent scripted items on the first try. Using SKSE's StringUtil one could cast to string and check if it starts with something other than '[ObjectReference', but otherwise things like Silver Sword and Wuuthrad always get through the first time, but as soon as you pick it up and try to place it again, then it is detected and refused (I guess because its reference has become persistent by then, although I'm not sure where, since PlayersDroppedWeapon should be cleared when it's retrieved). If you really want to make sure those don't go on racks yet, I guess you could pre-fill your exclusion list with Silver [Great]Sword, Red Eagle's Fury and Wuuthrad. But that might not be necessary since scripted items only seem to cause broken pointers if they are already persistent when dropped.

[Sclerocephalus]

My version is pretty similar, except I did it in a function on the item handling quest:

ObjectReference Function DropFromPlayer(Form akItem)
	ObjectReference chest = ReferenceTestContainer.GetReference()
	USKPWRTestContainerAliasScript chestScript = (ReferenceTestContainer As USKPWRTestContainerAliasScript)
	If !akItem || !chest || !chestScript
		Return None
	EndIf
	chest.DisableNoWait()
	chest.MoveTo(PlayerRef)
	chestScript.CatchAddedItem(akItem)
	chestScript.CatchRemovedItem(akItem)
	PlayerRef.RemoveItem(akItem, 1, True, chest)
	ObjectReference refOnAdd = chestScript.GetAddedItem()
	ObjectReference refFromDrop = chest.DropObject(akItem, 1)
	ObjectReference refOnDrop = chestScript.GetRemovedItem()
	If refOnDrop
		refOnDrop.SetActorOwner(PlayerRef.GetActorBase())
	EndIf
	chest.RemoveAllItems(PlayerRef, True, True)
	chest.MoveToMyEditorLocation()
	chest.EnableNoWait()
	DroppedItemWasScripted = (refOnAdd && ((refOnAdd != refFromDrop) || (refOnAdd != refOnDrop))) || (refFromDrop != refOnDrop)
	Return refOnDrop
EndFunction ; DropFromPlayer()

MoveTo() doesn't seem to mind if it's disabled, in fact if you have a reference to an item in inventory it'll even pull it out into the world with no complaint. I didn't initially have it return the chest to its start point or re-enable it, but I guess maybe that's a good idea.

 

Incidentally, dropping the item from the chest also means that the chest's OnItemRemoved() will get a valid in-world reference pointer even for scripted items. So if you just use that one instead of the return from DropObject(), then you've pretty much already implemented the scripted item fix; all you have to do then is trim down OnTriggerLeave() to not call methods on the received reference, and you also don't have to bother at all with refOnAdd or refFromDrop.

 

Also, I tried an almost identical version of the above which used an alias on the player instead of a test chest, and checked OnItemUnequipped() (triggered by player.UnequipItem()) instead of OnItemAdded() to detect persistent references which might be scripted items. Everything seemed to work just the same way, and that would avoid needing the chest at all. The only downside is that if the item is not persistent, then OnItemUnequipped() gets no pointer so you have to just DropObject() and allow that maybe it won't drop the same copy you just unequipped, although it might not matter in that case. For persistent references the behavior is identical, since you can use the unequipped reference to MoveTo() instead of calling DropObject() at all.

 

Edit: I should add, I haven't yet seen a way to detect non-persistent scripted items on the first try. Using SKSE's StringUtil one could cast to string and check if it starts with something other than '[ObjectReference', but otherwise things like Silver Sword and Wuuthrad always get through the first time, but as soon as you pick it up and try to place it again, then it is detected and refused (I guess because its reference has become persistent by then, although I'm not sure where, since PlayersDroppedWeapon should be cleared when it's retrieved). If you really want to make sure those don't go on racks yet, I guess you could pre-fill your exclusion list with Silver [Great]Sword, Red Eagle's Fury and Wuuthrad. But that might not be necessary since scripted items only seem to cause broken pointers if they are already persistent when dropped.

 

 

Actually, you don't even have to move the container around. No problems with enabling and disabling and no worries about whether something has 3D or not (well, if MoveTo is not problematic, there wouldn't have been anything to worry about, but anyway).

 

I tried the following and, suprisingly, it works. There was but one command that was not really predictable: dropping the item from the chest (which stands in a holding cell). Beyond that, only the line that sends the player item back had to be removed from the container alias script:

Function HandlePlayerItem (Form PlayerItem, ObjectReference TriggerMarker)

;This function checks whether the player item can be placed (it will reject oversized staves from display cases, and several problematic unique items, such as the
;ebony blade, from all racks), then calls the PlaceItem function to run the actual placement procedure. It is called from the RunPlayerActivation function.


	Bool StaffNotExcluded = True
	
	Int PlayerItemCount
	
	ObjectReference PlayerItemRef
	ObjectReference ReferenceFromQuest = None
	ObjectReference TestContainer = ReferenceTestContainer.GetReference()

	USKPWeaponRackItemHandlingScript ItemHandler = USKPWeaponRackItemHandlingQuest As USKPWeaponRackItemHandlingScript


	ReferenceFromQuest = ItemHandler.CheckItemRef (PlayerItem)
	PlayerItemRef = TestContainer.DropObject (PlayerItem, 1)
	
	If ReferenceFromQuest && (PlayerItemRef != ReferenceFromQuest)

		PlayerRef.AddItem (ReferenceFromQuest, 1, True)
		Self.Enable()

		USKPWeaponRackWeaponDisallowedMessage.Show()
		
		If (USKPWeaponRackExceptionList.HasForm (PlayerItem) == False)
			USKPWeaponRackExclusionList.AddForm (PlayerItem)
		EndIf

	Else

		PlayerItemCount = PlayerRef.GetItemCount (PlayerItem)

		If (PlayerItemCount > 0)
			PlayerRef.RemoveItem (PlayerItem, PlayerItemCount, False, TestContainer)
		EndIf

		PlayerRef.AddItem (PlayerItemRef, 1, True)
		PlayerItemRef = PlayerRef.DropObject (PlayerItem, 1)

		If (PlayerItemCount > 0)
			TestContainer.RemoveAllItems (PlayerRef, True, True)
		EndIf

		If PlayerItemRef.HasKeyword (WeaponTypeStaff) && TriggerMarker.HasNode ("WRDisplayCase01")
			If PlayerItemRef.HasNode ("Staff3rdPerson:0") || PlayerItemRef.HasNode ("3rdPersonStaff04:1")
				StaffNotExcluded = False
			EndIf
		EndIf

		If StaffNotExcluded
			PlaceItem (PlayerItemRef, TriggerMarker)
		Else
			USKPWeaponRackWrongStaffMessage.Show()
			PlayerRef.AddItem (PlayerItemRef, 1, True)
			Self.Enable()
		EndIf

	EndIf

EndFunction

This can probably be further simplified.

[Sclerocephalus]

Edit: I should add, I haven't yet seen a way to detect non-persistent scripted items on the first try. Using SKSE's StringUtil one could cast to string and check if it starts with something other than '[ObjectReference', but otherwise things like Silver Sword and Wuuthrad always get through the first time, but as soon as you pick it up and try to place it again, then it is detected and refused (I guess because its reference has become persistent by then, although I'm not sure where, since PlayersDroppedWeapon should be cleared when it's retrieved). If you really want to make sure those don't go on racks yet, I guess you could pre-fill your exclusion list with Silver [Great]Sword, Red Eagle's Fury and Wuuthrad. But that might not be necessary since scripted items only seem to cause broken pointers if they are already persistent when dropped.

 

As soon as the attached script starts running, an object reference becomes persistent. Many scripts that are attached to weapons and armor add perks to the player or attach effects, and they start running when the item is picked up, and again when it is dropped. Earlier in this thread, when I was struggling to understand Wuuthrads behaviour, I had the idea of unequipping it first (because the script adds and removes the perk in iOnEquip and OnUnequip events), so I added a line to the activator script to unequip the item before placing it. Though, this didn't make any difference...

 

EDIT:

As you already mentioned it, what would happen if a "transient reference" is cast as a string ?

[taleden]

What problem did you have with dropping the item directly from the chest? I didn't try doing that without first moving the chest to the player (while keeping it disabled), but I guess maybe it wouldn't work if you tried to drop the item into the other cell that isn't even loaded at the time. In my version where the chest was at the player, chest.DropObject() worked just fine.

 

Attached is my latest, based on your v3.4a scripts. It will work with or without the extra plugin loaded. If the new ESP is there, then HandlePlayerItem() will use a reference alias on the player instead of the test chest, and it will allow scripted items to be placed.

 

If the new ESP is not enabled, then it uses your existing item handler quest and test chest, and it blocks scripted items (except ones that aren't persistent yet, but yours does the same I think).

 

The logic for both modes is almost identical, and the chest version is pretty similar to yours I think, except that it drops from the chest instead of moving back to the player and doing all that business with moving extra copies out of the way etc, because I didn't find that any of that was necessary.

Taledens Weapon Rack Item Fix v3.7z