Jump to content

Overhauling the weapon rack scripts


Sclerocephalus

Recommended Posts

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

 

In my testing of the broken objref pointer bug, the string cast of a pointer to a scripted item depended on where the item was at the moment, and whether DropObject() had ever made it drop before. I don't want to derail this thread with the particulars of that bug though, since it's a little involved; that's why I started that other thread on the CK forums (and I'm still hoping someone from Bethesda will come and enlighten us, but I guess at this point they probably don't much care about bugs in skyrim anymore).

Link to comment
Share on other sites

In my testing of the broken objref pointer bug, the string cast of a pointer to a scripted item depended on where the item was at the moment, and whether DropObject() had ever made it drop before. I don't want to derail this thread with the particulars of that bug though, since it's a little involved; that's why I started that other thread on the CK forums (and I'm still hoping someone from Bethesda will come and enlighten us, but I guess at this point they probably don't much care about bugs in skyrim anymore).

 

I sent messages to a few people with which I have been discussing this issue to at least some extent in the past, and asked them to have a look at your thread. The low interest is a pity. I can't believe that this could have gone unnoticed.

 

BTW, on the "Bugs in Skyrim" topic:

http://www.afkmods.com/index.php?/topic/3781-the-critter-thread/?p=147556

Link to comment
Share on other sites

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?

I've compared the vanilla scripts, and the USKP 1.3.3c scripts, with 2.0.0 (line-by-line). This was made much harder by the fact that Sclero whimsically rearranges his code -- even between his own versions.

 

The vanilla scripts already put the starting items into PlayersDroppedWeapon. It is done on every OnCellAttach.

 

1.3.3c also does it OnCellAttach, and sets the various new flags. But has the infinite loop that caught a fair number of folks now.

 

2.0.0 is broken, as has been well documented. After it's been released with known errors, all I can do is say "I told you so". And have the Unofficial Unofficial patch ready.

 

I'm thinking that I'm going to change mine to remove any expectation of the contents of PlayersDroppedWeapon with regard to starting items. In fact, I'm thinking of removing starting items OnCellDetach. That will eliminate any persistence problems from stashing them in a property. And I'll not have to worry about OnReset (as much).

 

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.

The distance check could be a problem for items moved between close racks. But clever! I'll look into it.

 

However, I've been testing on exteriors where the items have fallen to the ground. It's really common in exteriors. ATM, I don't remember why? Do you?

 

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.

I'm about to do the 3-way diff to integrate yours with mine. It will take awhile (I always check by hand afterwards), and I'm sure I'll have questions.

Link to comment
Share on other sites

I'm thinking that I'm going to change mine to remove any expectation of the contents of PlayersDroppedWeapon with regard to starting items. In fact, I'm thinking of removing starting items OnCellDetach. That will eliminate any persistence problems from stashing them in a property. And I'll not have to worry about OnReset (as much).

 

All starting items are pre-placed in the CK, so those references have their own FormIDs -- doesn't that mean they are already always persistent, whether or not they're in a script property? They certainly behave that way to my eyes, for example causing OnItemAdded() to receive an ObjectReference pointer, which it doesn't for non-persistent items.

 

Also,

 

2.0.0 is broken, as has been well documented. After it's been released with known errors, ...

 

Which known errors? He did at least finally add a counter to the loop in PlaceItem() so it can't get stuck there, are there other serious bugs still remaining? Or just places it could be optimized?

Link to comment
Share on other sites

The vanilla scripts already put the starting items into PlayersDroppedWeapon. It is done on every OnCellAttach.

 

.. but never controlled whether their information came from a valid trigger event or from an entirely unrelated event. They also did not properly update when the item was grabbed (they could not detect form which rack the event came). Add traces to the vanilla scripts and compare the readout with your own rack manipulations.

 

1.3.3c also does it OnCellAttach, and sets the various new flags. But has the infinite loop that caught a fair number of folks now.

 

Yes, and I take the blame. While I could not foresee that GetParentCell() would by no means return what was expected, I could have easily prevented the loop from getting stuck with a sanity check. As for the "transient references" problem, a sanity check was entirely useless. If this could have been foreseen in your opinion, I'll take the blame again.

 

I did invest much time in finding a workaround, which is still too restrictive, but on the safe side. And it is well documented that it works.

 

2.0.0 is broken, as has been well documented.

 

While you don't get tired to state this, I never got a proof from you, so I have honestly no idea what is wrong or where this is documented. I understand that I make horrible mistakes. I obviously have to realize that I can't do any better. My programming style is entirely unconventional and my scripts are a pain for you to read ? Well, agreed. But why does this have to mean that they're not likely to work ?

 

I'm not a professional programmer (and I am also otherwise just average) which must have been obvious to you from the beginning, since you have been around on this site for quite a long time now more or less regularly. And nonetheless, you have been watching for most of the time. Why did you never say a word ?

 

In fact, I'm thinking of removing starting items OnCellDetach. That will eliminate any persistence problems from stashing them in a property. And I'll not have to worry about OnReset (as much).

 

They acquire their persistence when they get linked to another object reference in the CK, so whether or not their references are filled in script properties doesn't matter. While there's no safe way to check this in the CK, it can be easily controlled in TES5Edit.

 

However, I've been testing on exteriors where the items have fallen to the ground. It's really common in exteriors. ATM, I don't remember why? Do you?

 

That's because most racks in exterior cells are filled via dummies.

 

Unfortunately, dummies are far out of reach for scripting. The engine sets the pointer from the activator to a weapon that is drawn by chance from a levelled list, so that the rack never "sees" the dummy, but only the weapon. Although the dummy is linked from the activator, checking the link never returns the reference of the dummy, but always an "FF...." reference from an item created at runtime (i.e. from the leveled list).

 

Dummies are handled at low priority (they are not of critical importance to game mechanics) and in situations of high workload, their handling may be skipped entirely. In conditions of normal workload, the engine handles dummies easily. If you get problems with your mod active (certainly not the most appropriate test conditions), you have to find a specific workaround. As an example, you may consider to disable the starting items when the carriage starts its journey (you know the route) and re-enable them after arrival. Since they are persistent anyway, using their references as properties in your scripts doesn't do any harm. Or replace the dummies with real weapons. In the few places where engine workload might become an issue, the dummies draw the items from lists that produce middle- or high-level stuff only at a rather small chance, so this would probably go unnoticed by most players.

Link to comment
Share on other sites

They acquire their persistence when they get linked to another object reference in the CK, so whether or not their references are filled in script properties doesn't matter. While there's no safe way to check this in the CK, it can be easily controlled in TES5Edit.

 

Is this documented on the CK wiki or elsewhere?

 

As I mentioned earlier, my understanding of the meaning of "persistent" means that any item pre-placed in a cell in the CK is always persistent, because it has an unchanging reference FormID that gets the parent file's load order prefix, and every time that item is dropped to the world it will get that same reference FormID again. A non-persistent item, on the other hand, has no reference FormID until one is assigned to it at run-time, with the prefix FF. These non-persistent references are in fact recycled (try dropping two non-persistent items, click them in the console to see their reference IDs, then pick them both up and drop in reverse order), and that's the characteristic that I always thought "non-persistent" actually meant: every time it goes into the world, it gets a new reference with a potentially new reference FormID.

 

So if I'm right, then anything you place in the CK is automatically always persistent, whether or not it has (or is the target of) any linked references. And if I'm wrong, I stand ready to be educated.

Link to comment
Share on other sites

Persistent has a more specific meaning than just "I have a stable form ID."

 

For Oblivion, it was literally a placed reference that is maintained in memory at all times and whose data is kept in the save game when the player first encounters the reference in game. It was pretty much a yes or no issue. Persistent references could not be moved from their spot once the player had seen them because of this, and really, not much of anything could be done except using a script on them.

 

I'm fairly certain that both Fallout games handle persistence the same way.

 

Enter Skyrim. Persistence has different degrees now.

 

If a reference is selected as the target for an AI pack, linked ref, or virtually anything of this nature that does NOT involve a script, TES5Edit will show it as "persistent" but it doesn't fit the old definition. These references can still be moved about even if the player has seen them and that includes load doors as well. So it's probably best to think of those as "partially persistent" since the AI packs etc won't work if that flag is removed by TES5Edit. They are probably maintained in memory somewhere too but their positioning data is generally not baked into the save.

 

If a reference is set to be used as an alias in a quest, then while the quest is not running the ref remains as normal and can be relocated at will. When the quest is running, the reference is now persistent and cannot be moved and MAY get written to the save during this time as well. Once the quest is over, aliases marked as "Forced Reference" will remain permanently persistent. All others will return to their usual status.

 

If a reference is selected as a Papyrus script property, everything changes. Now you're back to the classic definition of persistence. These references are maintained in memory at all times, and cannot be moved. Sometimes even scripts are incapable of repositioning them. With a new twist too. The positioning data is baked into the save game from the moment the game starts and therefore no such object can ever be relocated. It makes no difference what type of script it is either.

 

At no point is simply placing an object into the CK render window going to make a reference persistent though. You have to do one of the above things first before that can happen.

Link to comment
Share on other sites

Is this documented on the CK wiki or elsewhere?

 

As I mentioned earlier, my understanding of the meaning of "persistent" means that any item pre-placed in a cell in the CK is always persistent, because it has an unchanging reference FormID that gets the parent file's load order prefix, and every time that item is dropped to the world it will get that same reference FormID again. A non-persistent item, on the other hand, has no reference FormID until one is assigned to it at run-time, with the prefix FF. These non-persistent references are in fact recycled (try dropping two non-persistent items, click them in the console to see their reference IDs, then pick them both up and drop in reverse order), and that's the characteristic that I always thought "non-persistent" actually meant: every time it goes into the world, it gets a new reference with a potentially new reference FormID.

 

So if I'm right, then anything you place in the CK is automatically always persistent, whether or not it has (or is the target of) any linked references. And if I'm wrong, I stand ready to be educated.

 

That's correct.

 

Now, from what he have found out so far, there are different levels of persistence, and with each level, restrictions to what you can and cannot do with the item become more severe. Arthmoor can probably tell you a lot more than myself on "persistence levels".

 

Items that are linked to enable markers, or are enable parents themselves for other objects are impossible to move, except by a script. For example, I wanted  to move the hoe out of sight that leans on the cart in the garden behind Honeyside. No matter what I did, the CK accepted the changes (and they were also present when I checked in TES5Edit), but in the game, the engine refused to move it at all - just as if the esp (although activated) didn't even exist (I finally inverse-parented it to its enable marker and flagged "initially disabled", which had pretty much the same effect as a deletion). The same happens with many weapon racks in Hearthfires homes, where one side of the CoARack plaques is always the enable marker for the other side and the shield plaque (although HF is even worse, because those enable parents/plaques do also appear as properties in the HF scripts). This is also why we didn't manage to solve the Lakeview Manor weapon rack bug for a long time, even after we had evidence of what caused the bug (and this is another engine bug, by the way: the engine has problems with calculating angles) :

 

http://www.afkmods.com/index.php?/topic/3759-god-dammit-bethesda-or-why-the-lakeview-manor-weapon-rack-cant-be-fixed/

 

The worst so far are havok items. Once they have started havoking (usually upon the first visit to a cell), they often are impossible to move away from their final position, even by a script.

 

--------------------------------------

 

Something else:

 

I told you that my script sorted a crossbow out, but I was wrong. It actually was an enhanced crossbow, and this has a script running on it. Interestingly, it could be placed once, but on the second try, it returned a trransient reference and got sacked:

First time:

[10/29/2013 - 02:51:42AM] [WeaponRackActivateScript < (000DF577)>]: beginning to handle player item.
[10/29/2013 - 02:51:42AM] Item handling quest: Player item sent to test container.
[10/29/2013 - 02:51:42AM] Test container: Player item [Form < (0300F19E)>] received from player; Ref = None
[10/29/2013 - 02:51:42AM] Test container: Player item returned to player.
[10/29/2013 - 02:51:42AM] Item handling quest: Player item reference submitted by test container.
[10/29/2013 - 02:51:42AM] [WeaponRackActivateScript < (000DF577)>]: Player item ref from quest = None
[10/29/2013 - 02:51:42AM] [WeaponRackActivateScript < (000DF577)>]: Player item dropped. Returned ref = [DLC1EnhancedCrossBowAddPerkScript < (FF001AE1)>]
[10/29/2013 - 02:51:42AM] [WeaponRackActivateScript < (000DF577)>]: Checking 3D of [DLC1EnhancedCrossBowAddPerkScript < (FF001AE1)>]
[10/29/2013 - 02:51:42AM] [WeaponRackActivateScript < (000DF577)>]: [DLC1EnhancedCrossBowAddPerkScript < (FF001AE1)>] loaded.
[10/29/2013 - 02:51:42AM] [WeaponRackActivateScript < (000DF577)>]: Placed Item = [Form < (0300F19E)>]; Ref = [DLC1EnhancedCrossBowAddPerkScript < (FF001AE1)>]
[10/29/2013 - 02:51:42AM] [WeaponRackActivateScript < (000DF577)>]: Trigger Marker [WeaponRackTriggerSCRIPT < (000DF572)>] in empty state.
[10/29/2013 - 02:51:42AM] [WeaponRackActivateScript < (000DF577)>]: Player item [WEAPON < (0300F19E)>] placed on trigger [WeaponRackTriggerSCRIPT < (000DF572)>].


When I grabbed it, the reference was already borked:

[10/29/2013 - 02:52:26AM] [WeaponRackActivateScript < (000DF577)>]; Leaving Item = [Form < (0300F19E)>]; Ref = [ObjectReference < (FF001AE1)>]
[10/29/2013 - 02:52:26AM] [WeaponRackActivateScript < (000DF577)>]; Mounted Item = [Form < (0300F19E)>]; Ref = [DLC1EnhancedCrossBowAddPerkScript <Item 23 in container  (00000014)>]
[10/29/2013 - 02:52:26AM] [WeaponRackActivateScript < (000DF577)>] enabled; Trigger in 'ActivatorBusy' state.
[10/29/2013 - 02:52:26AM] [WeaponRackActivateScript < (000DF577)>]: TOC = 0; StartingItemHasBeenGrabbed = True.

On the next try to place it, it failed.
Link to comment
Share on other sites

EDIT: Ninja'd by Arthmoor!

All starting items are pre-placed in the CK, so those references have their own FormIDs -- doesn't that mean they are already always persistent, whether or not they're in a script property? They certainly behave that way to my eyes, for example causing OnItemAdded() to receive an ObjectReference pointer, which it doesn't for non-persistent items.

In some respects. If you delete an item that's pre-placed, it isn't removed from memory. It just has a flag.

All items assigned to a Property or a variable become temporarily persistent. Including new objects created from dummies. So they don't delete or disappear, as long as they are in the Property.

http://www.creationkit.com/Persistence_(Papyrus)

The cell resets are problematic, and don't seem to be working in 2.0, as I tried to explain to Sclero -- but he knows so much more than I do about everything.... ;)

There's a reason the original developer had lines that assigned None to his temporary variables after he accessed various linked references. Unfortunately, Sclero didn't understand them, so he took them out. I've put them back in mine.

 

Which known errors? He did at least finally add a counter to the loop in PlaceItem() so it can't get stuck there, are there other serious bugs still remaining? Or just places it could be optimized?

He did? And here I was going to thank you for putting in some of the things that he'd specifically refused to do!

I see you fixed the OnLoad in the activator script (returned it to OnAttach). I wish you'd taken all my code for the OnLoad in the trigger script.

Yet, even in your script, you haven't put the old states back or fixed the lockups. Or the badly ordered flag sets. Or any number of other items I've fixed in my code.

But then there'd be no need for my patch, and I've have wasted $5,000+ unbillable hours last week, and even more time over the weekend.

And I see you've removed the line (from your v2) to place player items. I'd fixed that already, too.

 

So if I'm right, then anything you place in the CK is automatically always persistent, whether or not it has (or is the target of) any linked references. And if I'm wrong, I stand ready to be educated.

No, actually, that's not what persistence means at all. Read the page.

OK, now I'd like to discuss your patch. Is there some reason that you didn't use the same quest as already in 2.0? It seems to me that could be done.

Also, for efficiency and clarity, it seems that the whole sequence of calls to set variables and retrieve them from the chest script could simply be a new single call to the chest script with a return -- all the setup could be done there, there's no reason the playerRef.RemoveItem couldn't be done there, too.

Every one of those chestScript.foo calls is interrupted by definition (a scheduler conflict resolution) and is usually delayed until the next frame (1/60 second), for coordination between scripts -- even though you're the only one accessing that script. So this really slows down the script. See http://forums.bethsoft.com/topic/1409991-best-practices-papyrus/ and http://www.creationkit.com/Threading_Notes_(Papyrus)

Would you mind me re-writing it a bit?

Link to comment
Share on other sites

Attached are logs from a short test run through Breezehome, the Warmaiden's and Hjerim. For those who are interested, I have included the papyrus log, but the weapon rack log is more interesting here (there's nothing at all about racks on the papyrus log, but a companions quest keeps spamming), because I ran the test with the new trigger meshes. Have a look at the TOCs:  they are either 1 (full) or 0 (empty). The few exceptions are exclusively from crossbows, because they are still placed clipping through wooden racks and spanning two full slots (fixing this requires two new nodes).

 

 

Test_v34a.7z

Link to comment
Share on other sites

While you don't get tired to state this, I never got a proof from you, so I have honestly no idea what is wrong or where this is documented.

When I've given you explicit instructions on how to replicate, you told me there must be a mod conflict -- specifically that my mod conflicted. Well, I've replicated the problem merely walking from Whiterun to Riverwood. No carriage. Still locks up. Still has weapons lying on the ground. Not every time. But reproducible.

And USKP has to work with all other plugins, even those that load a lot of graphics quickly, or use a lot of script time. Convenient horses has an option to speed up the horse and loads even more graphics than carriages -- and a lot more folks use Convenient Horses!

Funny thing, my code doesn't have this problem. In test after test after test. Not once!

You refused to add conditions to your loop -- that both Talenden and I told you were needed -- except now I'm told you have stealthily done so.

Well, since Arthmoor has already said he won't take Talenden or my patches this cycle, presumably he won't take the same thing secretly from you.

I understand that I make horrible mistakes. I obviously have to realize that I can't do any better. My programming style is entirely unconventional and my scripts are a pain for you to read ? Well, agreed. But why does this have to mean that they're not likely to work ?

Nobody said you couldn't do better. But you refuse to take advice. Hell, when 2 of us laid out exactly why a piece of code wasn't needed or working yesterday, you just denied it. How do we reason with you?

And no, there are all kinds of methods of coding. And every problem usually has more than one solution. But when told your code doesn't work and how to fix it, it's pretty annoying to be told -- "don't touch that, leave it the same."

Why did you never say a word ?

Because I was busy doing other things and didn't know until I was testing 2.0 beta. That's why we have betas -- everybody gets together and tests.

 

That's because most racks in exterior cells are filled via dummies.

 

... In conditions of normal workload, the engine handles dummies easily. If you get problems with your mod active (certainly not the most appropriate test conditions), you have to find a specific workaround.

Or fix the code.

As an example, you may consider to disable the starting items when the carriage starts its journey (you know the route) and re-enable them after arrival. Since they are persistent anyway, using their references as properties in your scripts doesn't do any harm. Or replace the dummies with real weapons. In the few places where engine workload might become an issue, the dummies draw the items from lists that produce middle- or high-level stuff only at a rather small chance, so this would probably go unnoticed by most players.

Nobody is going to do this. It would conflict with other mods. And it would be a PITA to try to do all over the world.

I understand you mostly tested indoors, or by CoC, or by fast travel. Well that's why all get as many eyes on the issues as we can....

You've done a great job with the nif nodes, and really understand the issues for various houses. And we thank you and appreciate it. But being a bit more open to suggestions would be helpful.

Link to comment
Share on other sites

He did? And here I was going to thank you for putting in some of the things that he'd specifically refused to do!

 

I never specifically refused to to that. I even told you that I would do it.

I only asked you a question related to a check, because you had talked a lot about errors before, without getting specific though, so I only wanted to know whether you suspected a specific issue about the reference you were so vehemently pointing at. Maybe that I just misunderstood you, but i t was a honest question and after your reaction I did not really fancy bringing the conversation back on that topic.

 

Also, even those minor modifications have been tested last weekend, just to make sure. More serious changes wouldn't have been accepted by Arthmoor anyway, not at this point.

Link to comment
Share on other sites

There's a reason the original developer had lines that assigned None to his temporary variables after he accessed various linked references.

 

If you mean function-scope variables, and if the function doesn't keep looping for a long time, then I don't see the point either -- as soon as the function exits those variables go poof, so whatever references they were pointing to should stop being persistent at that point (unless they're still held in some other active variable). Or are you saying that function-scope variables stay in memory after the function exits, unless explicitly cleared to None? Or by "temporary variables" did you mean script-scope variables? Those I would understand None-ing out, or better yet, not using script-scope variables for temporary storage if at all possible.

 

He did? And here I was going to thank you for putting in some of the things that he'd specifically refused to do!

 

Most of the changes you see between my v2 and v3 downloads are actually changes from Sclero's v3.4 -> v3.4a; I diffed those before re-applying my tweaks and the changes were extensive, although a lot of them were just restoring comments (why would you ever delete informative comments, even from a non-debug version? They have no performance overhead anyway...)

 

I see you fixed the OnLoad in the activator script (returned it to OnAttach). I wish you'd taken all my code for the OnLoad in the trigger script.

Yet, even in your script, you haven't put the old states back or fixed the lockups. Or the badly ordered flag sets. Or any number of other items I've fixed in my code.

 

That's all Sclero's I believe. All I did in my last download was re-edit the top of HandlePlayerItem(), re-disable the method calls on the pointer in the function called by OnTriggerUpdate(), and totally replace the item handler helper quest and chest alias scripts.

 

And I see you've removed the line (from your v2) to place player items. I'd fixed that already, too.

 

That I just plum forgot to re-apply when I switched to Sclero's v3.4a.  :facepalm:

 

OK, now I'd like to discuss your patch. Is there some reason that you didn't use the same quest as already in 2.0? It seems to me that could be done.

 

I did at first, but had some trouble with adding the PlayerRef alias to the quest while it was already running in my testing save (where I bought and furnished Honeyside and gathered all the persistent scripted item references, just so I wouldn't have to do that every single time I tweaked the script and retested). Maybe I was overlooking something simple but at the time it was more expedient to just make a new quest that would start fresh on load and fill the alias properly.

 

Also, that way I was able to rewrite both his original quest/chest scripts (along the lines that he was already going with the dropping from chest idea) as well as add my alternative quest/playerref scripts, and you folks could see both versions side by side if you wanted.

 

Also, for efficiency and clarity, it seems that the whole sequence of calls to set variables and retrieve them from the chest script could simply be a new single call to the chest script with a return -- all the setup could be done there, there's no reason the playerRef.RemoveItem couldn't be done there, too.

Every one of those chestScript.foo calls is interrupted by definition (a scheduler conflict resolution) and is usually delayed until the next frame (1/60 second), for coordination between scripts -- even though you're the only one accessing that script. So this really slows down the script. See http://forums.bethsoft.com/topic/1409991-best-practices-papyrus/ and http://www.creationkit.com/Threading_Notes_(Papyrus)

 

I'm not sure if you're speaking to me or Sclero on that point. My implementation of the chest concept just has one function on the quest script, which makes four calls into the chest alias script. Two of those (CatchAddedItem() and GetAddedItem()) are only necessary if you're trying to exclude scripted items, so if/when people are ready to fix that bug, those two can just be removed. But the quest script doesn't set or retrieve any variables from the chest script at all, so maybe you were talking about Sclero's version.

 

Edit: That said, you're probably right that the logic in the quest's function could be moved into HandlePlayerItem(), although that only saves you the one function call. It does save a whole script file though. Anyway, I wasn't really focused on optimization at this point, just proof-of-concept.

Link to comment
Share on other sites

I'm not sure if you're speaking to me or Sclero on that point. My implementation of the chest concept just has one function on the quest script, which makes four calls into the chest alias script. Two of those (CatchAddedItem() and GetAddedItem()) are only necessary if you're trying to exclude scripted items, so if/when people are ready to fix that bug, those two can just be removed. But the quest script doesn't set or retrieve any variables from the chest script at all, so maybe you were talking about Sclero's version.

This here was in your USKPWeaponRackItemHandlingScript source files. I assumed it was yours, thought I'd remembered something similar from an earlier post. The posts have been flying by....

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)
At least 8 external calls. Could be 1 call to the chestScript.

I also don't know why you bother to disable and enable the chest. Since this all works disabled, could just have a initially disabled chest. It won't disappear on you! :)

The USKPWRPlayerMonitorQuest version is shorter, but this time calls into the Alias script:

aliasScript.CatchUnequippedItem(akItem)
aliasScript.CatchDroppedItem(akItem)
PlayerRef.UnequipItem(akItem, False, True)
ObjectReference refOnUnequip = aliasScript.GetUnequippedItem()
ObjectReference refFromDrop = None
If refOnUnequip
	refOnUnequip.MoveTo(PlayerRef, 0, 0, 160, False)
Else
	refFromDrop = PlayerRef.DropObject(akItem, 1)
EndIf
ObjectReference refOnDrop = aliasScript.GetDroppedItem()
DroppedItemWasScripted = (refOnUnequip && (refOnUnequip != refOnDrop)) || (refFromDrop && (refFromDrop != refOnDrop))
At least 7 external calls. Again, could be consolidated in the USKPWRPlayerAliasScript.

Edit: That said, you're probably right that the logic in the quest's function could be moved into HandlePlayerItem(), although that only saves you the one function call. It does save a whole script file though. Anyway, I wasn't really focused on optimization at this point, just proof-of-concept.

Good idea. Anyway, I've integrated this into mine, but would like pointers how and where to initially test before putting it up to grab.

Also, didn't realize that earlier this evening 2.0 was finally released, so just finished downloading. Based on Arthmoor's warning about using beta saves, will have to make some new tests.

Link to comment
Share on other sites

That I just plum forgot to re-apply when I switched to Sclero's v3.4a.  :facepalm:

Here's how I fixed that:

If StartingItem

	If (StartingItemHasBeenGrabbed == False) && StartingItem.IsEnabled()

		If (PlacedItemInit && (PlayersDroppedWeapon != StartingItem))
			; has been previously placed, so repair mismatch
			StartingItemHasBeenGrabbed = True
			Trace (Self + "ActivatorSetup() not starting item; set StartingItemHasBeenGrabbed = True.")

			If PlayersDroppedWeapon
				PlaceItem(PlayersDroppedWeapon, TriggerMarker)
			EndIf

		ElseIf StartingItem.IsDeleted()
			StartingItemHasBeenGrabbed = True
			Trace (Self + "ActivatorSetup() deleted item; set StartingItemHasBeenGrabbed = True.")
		Else
			Cell parentCell = StartingItem.GetParentCell()
			If parentCell && parentCell == GetParentCell() && CheckFor3D (StartingItem)
				HandleStartingItem (StartingItem, TriggerMarker)
			EndIf
		EndIf

	EndIf

ElseIf PlayersDroppedWeapon
	PlaceItem(PlayersDroppedWeapon, TriggerMarker)
EndIf
Link to comment
Share on other sites

At least 8 external calls. Could be 1 call to the chestScript.

I also don't know why you bother to disable and enable the chest. Since this all works disabled, could just have a initially disabled chest. It won't disappear on you!

 

It could be 1 call to chestScript, but that code would still have to use self.GetReference() to manipulate the chest itself (MoveTo, DropObject) and those calls would still be considered external to the alias script, no? So if you really want to eliminate as many external calls as possible, I guess you'd have to put the script directly on the chest form or its placed reference so that it can extend ObjectReference, instead of on the quest alias like it is now.

 

And yes, the chest can just start disabled and stay that way. Those calls are just to make the demo work even if it's added to a test save that already had an enabled chest, since I didn't think overriding "starts disabled" would apply in that case.

Link to comment
Share on other sites

II did at first, but had some trouble with adding the PlayerRef alias to the quest while it was already running in my testing save (where I bought and furnished Honeyside and gathered all the persistent scripted item references, just so I wouldn't have to do that every single time I tweaked the script and retested). Maybe I was overlooking something simple but at the time it was more expedient to just make a new quest that would start fresh on load and fill the alias properly.

That makes sense. Didn't know the current USKP quest wasn't being started and stopped properly.

 

Two of those (CatchAddedItem() and GetAddedItem()) are only necessary if you're trying to exclude scripted items, so if/when people are ready to fix that bug, those two can just be removed.

Oh, I'd assumed that your new code also fixed that, as did your old code. I'll have to take some time and compare.

Because there are a lot of folks who want to fix that bug! Arthmoor just pointed folks at your post over on the official site. Nice report. Hopefully some more discussion and testing will ensue.

 

Edit: That said, you're probably right that the logic in the quest's function could be moved into HandlePlayerItem(), although that only saves you the one function call. It does save a whole script file though. Anyway, I wasn't really focused on optimization at this point, just proof-of-concept.

I'll look at it. The performance hit was what is causing the outdoor problems, so my variant is all about improving performance!

 

It could be 1 call to chestScript, but that code would still have to use self.GetReference() to manipulate the chest itself (MoveTo, DropObject) and those calls would still be considered external to the alias script, no? So if you really want to eliminate as many external calls as possible, I guess you'd have to put the script directly on the chest form or its placed reference so that it can extend ObjectReference, instead of on the quest alias like it is now.

No, GetReference() is a Non-delayed Native Function: http://www.creationkit.com/Category:Non-delayed_Native_Function

 

And yes, the chest can just start disabled and stay that way. Those calls are just to make the demo work even if it's added to a test save that already had an enabled chest, since I didn't think overriding "starts disabled" would apply in that case.

So which code is yours, the first or the second? Or do you both use the same chest?
Link to comment
Share on other sites

No, GetReference() is a Non-delayed Native Function: http://www.creationkit.com/Category:Non-delayed_Native_Function

 

So which code is yours, the first or the second? Or do you both use the same chest?

 

GetReference() may be, but when you get that reference back and then call MoveTo() and DropObject() on it, aren't those still external calls to the ObjectReference script which is implicitly attached to the chest reference, and which the chest alias script is not directly extending? Hence the suggestion to convert it to an ObjectReference script and attach it to the chest or its reference instead of using the quest alias at all. Then all those calls would be local.

 

Both pairs of quest/alias scripts in my v3 download are mine, it's just the activator and trigger scripts that are mostly Sclero's with my small adjustments. The two quests and their respective aliases do pretty much exactly the same thing, so only one or the other is needed. I only included both as a demonstration that it could be done either way, and because I wasn't sure which strategy would be preferred in the long run. The chest version is what Sclero was already using in v3.4a so I just replaced the two scripts without overriding his quest or chest in my ESP; the player alias version is added by my ESP.

 

Using the chest has the advantage that you can always guarantee that the reference which is unequipped is exactly the one that is eventually dropped and mounted, but it requires that chest reference and calls to MoveTo() and a failsafe RemoveAllItems(). Using the player alias has the advantage that no chest is required and if something goes wrong the item can't get lost in some inaccessible chest, however if the equipped item is not persistent, then the reference which is dropped and mounted might not be exactly the same one that was equipped, although it of course must be the same base object. Maybe the player could never notice that, but I wasn't sure about things like tempered weapons -- if the player explicitly wanted to mount their Legendary sword but had a Fine one on-hand too, they might be annoyed that they have to drop one before the other one will mount. I didn't actually test that though so I don't know if that's a problem, it was just a thought, so I figured I'd go ahead and see if the basic concept even worked using a player alias instead of a chest, and it does.

Link to comment
Share on other sites

Maybe the player could never notice that, but I wasn't sure about things like tempered weapons -- if the player explicitly wanted to mount their Legendary sword but had a Fine one on-hand too, they might be annoyed that they have to drop one before the other one will mount. I didn't actually test that though so I don't know if that's a problem, it was just a thought, so I figured I'd go ahead and see if the basic concept even worked using a player alias instead of a chest, and it does.

They'll notice. It's documented as the equipped item.

 

However, another thought occurred to me -- will a stolen item get whitewashed by the chest? That would be bad. That's with the player owning the chest. Of course, the converse would be worse (accidentally making it stolen).

Link to comment
Share on other sites

After another test run (no crossbows this time), I stiil get the following message of a shield rack in Hjerim starting with TOC = 2:

[WeaponRackActivateScript < (00102914)>] disabled on cell attach; Trigger marker in empty state; PlacedItemInit = False; TOC = 2

After taking the shield from the rack, TOC went back to 1, which was quite surprising because the left and right plaques are filled with identical weapons in identical positions relative to the trigger. Hence, they aren't the culprits and the trigger is basically working.

 

I went to Hjerim again and emptied the rack, but TOC remained at 1 as expected.

 

I suspected the open lid of the display case placed in front of and below the shield rack, but there's no way that it swings through the trigger volume. Also, I couldn't have activated the rack with the lid open.

 

It seems thus that the shield trigger requires some more tweaking.

Link to comment
Share on other sites

After another test run (no crossbows this time), I stiil get the following message of a shield rack in Hjerim starting with TOC = 2:

[WeaponRackActivateScript < (00102914)>] disabled on cell attach; Trigger marker in empty state; PlacedItemInit = False; TOC = 2

After taking the shield from the rack, TOC went back to 1, which was quite surprising because the left and right plaques are filled with identical weapons in identical positions relative to the trigger. Hence, they aren't the culprits and the trigger is basically working.

 

I went to Hjerim again and emptied the rack, but TOC remained at 1 as expected.

 

I suspected the open lid of the display case placed in front of and below the shield rack, but there's no way that it swings through the trigger volume. Also, I couldn't have activated the rack with the lid open.

 

It seems thus that the shield trigger requires some more tweaking.

 

I remember reading that although OnTriggerEnter and Leave are more reliable, OnTrigger is also supposed to fire periodically as long as something is in the trigger volume. For debugging at least, could you use that to find out what it is that's making your TOC=1 when it shouldn't be?

Link to comment
Share on other sites

I remember reading that although OnTriggerEnter and Leave are more reliable, OnTrigger is also supposed to fire periodically as long as something is in the trigger volume. For debugging at least, could you use that to find out what it is that's making your TOC=1 when it shouldn't be?

 

Good idea. Will try.

Link to comment
Share on other sites

Also, there's the possibility that the trigger volume is too close to the rack, and detecting the rack itself -- or the wall.

 

We've found with NPCs riding carriages that the specified collision boundary isn't an exact indicator of its edges. The engine fudges (a lot) to simplify calculations.

Link to comment
Share on other sites

Also, there's the possibility that the trigger volume is too close to the rack, and detecting the rack itself -- or the wall.

 

In fact, the original trigger "cylinder" overlaps with the wall in many cases (but not with the rack), but the wall does not contribute to the TOC. The triggers  also disregard overlaps with their own trigger meshes (this occurs with the wooden racks, where all the protruding parts are inside the trigger). While this is certainly an interesting discovery, it is not difficult to understand how they do it: all objects have parameters which define their interactions with the game world and depending on their settings, some object types (e.g. all static meshes such as the wall) are sorted out.

 

What I am presently testing though are the new triggers. The new shield trigger is a flat box in the shield's plane which is positioned at a good distance from the rack itself. Doing it that way had of course foreseeable consequences, but all all shield racks, except for this one in Hjerim, work. And even better: the weapons mounted on either side which previously did always interfere have stopped doing so.

 

 

We've found with NPCs riding carriages that the specified collision boundary isn't an exact indicator of its edges. The engine fudges (a lot) to simplify calculations.

 

Although this depends on the collision mesh used (there are three substantially different types of triggers used: simple box shapes, convex vertices shapes and compressed mesh shapes), there are some simple ways to improve it. There's a mesh parameter called "radius" which reflects the accuracy of havok calculations relative to the actual mesh. Most "radius" parameters in Beth's meshes have meanwhile been found to be substantially too large and this was one of the reasons for the many floating clutter objects. If you have specific issues with the carriages, that would be a good point to start: just reduce the value stepwise and see what happens.

 

In my new meshes, I have set this to zero at the moment, because the shapes are very simple (they are all boxes) and I want them explicitely to be considered by the engine as they are (I may need to change this though, in case this leads to performance issues).

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×
×
  • Create New...