irswat Posted January 27, 2017 Share Posted January 27, 2017 I saw it but it's not really relevant, nor does it indicate that the function is slow, so for those two reasons I don't think that's what he was referring to.no offense man, im just trying to help, but if we are referring to the same post: If it's OK to wait until after some item has been transferred then you can get the reference and the only problem is that if the player has a habit of storing massive numbers of things in containers then grabbing them all with "Take All" you'll run into some performance issues (that can be minimized by careful use of states but not eliminated). I think the above is exactly what cdcooley was talking about, correct me if im wrong cdcooley. Link to comment Share on other sites More sharing options...
Lazauya Posted January 27, 2017 Author Share Posted January 27, 2017 That warning is actually wrong. Stack dumps are just a debugging scheme the game uses to signal the Papyrus system is falling behind. No information is actually lost. It's true that a Take All operation can temporarily overwhelm the system, but it's not nearly as big of a problem as people originally thought. No queued calls are lost. You do want to make sure the event code runs as quickly as possible. If you're only wanting to give durability to armor and weapons then make sure your code starts with something like this to quickly skip over other types of items. (And yes, casting a base formid to a type is the fastest way to do that check.) Code in those event handlers are one of the rare cases where every last bit of optimization is justified and function calls (like GetType) should be avoided unless absolutely needed. if !(akBaseItem as Weapon || akBaseItem as Armor) return endif Hm, thanks for the clarification. Link to comment Share on other sites More sharing options...
cdcooley Posted January 27, 2017 Share Posted January 27, 2017 By performance issues I meant that while the transfer of items is almost instant it also queues a (potentially) huge number of events. The events can't run in parallel due to object locking and basically every function call made in the event takes one frame. If you have a relatively short event with just 6 function calls that still means you can only process 10 items per second (assuming the player is getting 60 FPS). Since some players store hundreds of items in containers that means it could take quite a while to process all of them and the scripts will seem unresponsive because of the multi-second delays. That's why the AddInventoryFilter is important (when it can be used). InventoryFilters can actually prevent the event getting called for uninteresting items (but requires you to be able to explicitly list all of the interesting ones). If you want all armor or all weapons (or both) then you can't use a filter but starting the event with checks that only use casting the variable to check its type allow the script to complete in a single frame for items that aren't interesting (and even potentially allow more than one of those events to be processed in the same frame as long as you have a group that are all uninteresting). Link to comment Share on other sites More sharing options...
lofgren Posted January 27, 2017 Share Posted January 27, 2017 (edited) I am referring to this comment: Container currently accessed... You could wait till the player transfers an item.  Using OnItemAdded and OnItemRemoved on a player alias script you will be able to use the akSourceContainer parameter in OnItemAdded and akDestContainer parameter in OnItemRemoved to obtain the container being interacted with.  If the akDestContainer value is NONE then the item was dropped or simply removed with no destination (some scripts do this).  If akSourceContainer value is NONE then the item was picked up. But if the player doesn't transfer an item, then you will have no idea what container they are currently accessing. You might be able to use OnCrosshairRefChange (an SKSE event) to obtain the object that the player is pointing at and hope that this is the container that they actually opened.  But experience has shown me that this event will process with everything in range that it points to and even with the best filtering can still backup papyrus and cause stack dumps.  I don't recommend it. A third approach could be to set up a hotkey that the user can use to open containers instead of the normal activation.  By using your hotkey, they could then trigger GetCurrentCrosshairRef just prior to using Activate to simulate having used the real activation key.  Drawback is that if they don't use your key, then you won't know if they accessed the container.  Also, there are some cases where the object interacted with is not the actual container and so the returned object will not be a valid container. If there is any other way, I don't know it. I would suggest a combination of the first and third approaches.  The first as a backup in case the player doesn't use the hotkey to open a container and as a filter to confirm that the returned object is the actual container.  It would also cover those players that don't use SKSE.  Hm... I'll look into these, but they seem a little slow. I though of using the crosshair ref tactick. Would these be able to account for inventories the player doesnt look at? I.e., if a script opens a container menu for the player? Later the OP also commented that your method would be "still a little slow," and I commented that it would be extremely slow compared to the earlier posted methods. This was before cdcooley had even posted in the thread and before discussion of performance issues had arisen. Additionally, the performance issues that OnItemAdded/Removed suffers are not due to it being "slow." Edited January 27, 2017 by lofgren Link to comment Share on other sites More sharing options...
Recommended Posts