Weapon rack repair

[DayDreamer]

It has been over a decade since several different attempts by several programmers. Having just reviewed and repaired them again, best to write things down for posterity.

Weapon racks have 2 main parts:

1. a visible part. It uses WeaponRackTriggerSCRIPT. Unlike most visible actuators (such as seats), it does not cause any action. Instead, it merely triggers as a weapon is passed near it. Usually, that trigger is something being added to the rack, or to another rack nearby, and must be ignored. Sometimes, that trigger is a player removing a weapon or armor item from the rack.
2. an invisible trigger marker placed directly in front of the visible part. It uses WeaponRackActivateSCRIPT, and has ACTIVATOR in its name. When triggered, the script places a weapon into the front of the visible part using MoveToNode(). The visible part has many such named attachment "nodes".

Because of the unusual design, there were many locations that simply had the wrong script on the objects. It became apparent that various designs were attempted, and some old designs with the wrong scripts or variables remained in the files. So a significant US*P change to the scripts was adding a CheckConfiguration() function to log places with bad setup so that they could be repaired later.

Although the design expects that the visible part has a pointer to the invisible part, and vice versa, there was no check. (A check was added.) Many locations have no invisible part. Others had/have the invisible part in front of the wrong rack, or even underground. Again, there is no easy check.

There are many flavors of racks. Most do not actually allow a player to add a weapon to the rack. But they use the same scripts, so the scripts have to account for all possibilities.

Some have PlayerHouse in the name, and allow a player to add a weapon to the rack. Others are specific to locations. A few OLD and Nor designs are unused.

Some are used as backing plaques, or as decorations, and their scripts were deleted from them. Others used another technique to disable the rack parts, such as pointing the rack to itself, or to the player object. There is not much implementation consistency.

Finally, the ACTIVATOR can point to a StartingWeapon. This can also be armor.

All of the item pointers are stored in the invisible WeaponRackActivateSCRIPT property PlayersDroppedWeapon, instead of the visible rack itself. Therefore, every rack with a starting item requires an invisible ACTIVATOR, even though most racks do not accept player weapons so the activator is otherwise unused. Inefficient and cumbersome, and not all Bethesda developers understood the design. Many racks were missing an ACTIVATOR. Many more have an unused ACTIVATOR that does nothing.

Unfortunately, there were originally two methods of pointing to the StartingWeapon: GetLinkedRef() or GetLinkedRef(WRackStartingWeapon). However, only the rare WeaponRackDaggerDisplayACTIVATORPlayerHouse specified the latter, and it was never used. So that complication was removed from the scripts.

 

[DayDreamer]

Vanilla scripts had serious programming issues that often left the starting item on the floor.

• The invisible WeaponRackActivateSCRIPT was initialized by OnCellAttach().
• The visible rack WeaponRackTriggerSCRIPT was initialized by OnLoad(). (Also, an older attempt using OnCellLoad() was commented out.)

OnCellAttach() runs reliably every time the player enters a cell/room. Also, OnCellAttach() always runs before OnLoad(). The invisible part was setup before the visible part, but relied upon the visible part in its setup.

Moreover, OnLoad() only runs the first time into a cell/room, and can stick around for a lengthy time (in memory) as the player leaves and re-enters. The scripts would fail because they were out of step with each other.

Instead, US*P uses OnCellAttach() in both scripts. ActivateSCRIPT only does basic checking, and no longer handles starting items. TriggerSCRIPT ensures that the objects are synchronized by setting up the visible part, then calling the invisible part to handle any starting item.

TriggerSCRIPT still uses OnLoad() for Hearthfires houses. When built at the workbench, as the 3D appears, an OnLoad() event occurs. After some basic checking (because racks can also be enabled by other processes), it uses the same OnCellAttach() setup function to synchronize.

Another issue is object reference persistence. A starting item or player dropped weapon reference was not cleared by removals. Some starting items are dummy items that upgrade after the cell/room resets. Worse, items taken by the player persist even after being sold and removed from the game.

Therefore, the visible rack's OnTriggerLeave() tells the ActivateSCRIPT to clean up. This code also repairs racks that were blocked by objects from nearby racks.

Finally, OnReset() was replaced by OnCellDetach(). This reset vanilla references and flags by leaving a cell/room.

This also aided in deployment and testing. By removing persistent references, replacement objects would upgrade. As the objects were reliably re-initialized, they could be tested simply by leaving and re-entering a cell/room.

 

Edited June 14 by DayDreamer

[DayDreamer]

Vanilla weapon rack scripts have no thread coordination. It appears that there was an initial attempt; each script has an "auto" state specified. But any other vanilla states that may have been tried have been removed.

US*P handles thread coordination with both states and flags.

WeaponRackActivateSCRIPT is simplest. OnCellAttach() checks the configuration, and on failure sets its state to "Inactive". This prevents any interaction with the rack. After fixing the rack, leaving and re-entering the room/cell will resume the default state.

WeaponRackTriggerSCRIPT adds two states:

• "ActivatorBusy" attempts to prevent both OnLoad() and OnTriggerLeave().
• "TriggerDisabled" contains OnLoad(), but continues to restrict OnTriggerLeave().

To this end, the very first statement is GoToState("ActivatorBusy") in OnCellAttach(), OnLoad(), and OnTriggerLeave(). This is the primary concurrency defense.

Every Papyrus function call may allow a different thread to access the object scripts. These functions are ubiquitous and essential, such as GetLinkedRef().

As explained in the second post (above), OnCellAttach() always runs before OnLoad(). Because OnCellAttach() might not have finished before OnLoad() begins, setup would happen twice concurrently, with temporarily incorrectly set variables, and all sorts of log spam. Therefore, the OnLoad() must be prevented from running.

When the rack is disabled, OnLoad() does not run until the rack is enabled. Thus, OnLoad() is used for a repeat setup whenever the rack is enabled while the player is in the cell/room.

In addition, both states prevent OnTriggerLeave(). During setup, items may temporarily swipe past the trigger. Sometimes, items in adjacent racks will swipe past the trigger.

Also, cross-script function calls are synchronized by the video frame rate. Therefore, as TriggerSetup() calls ActivatorSetup(), there is a secondary concurrency defense:

	Bool wasHandled = OnUpdateHandled
	OnUpdateHandled = True
	If wasHandled
	;~	Trace(Self + CallingEvent + "ActivatorSetup() was handled.")
		Return
	EndIf

The game engine may also interrupt thread execution by time slice. As Papyrus does not have an atomic test and set function, note an obvious interrupt window between the first and second statements.

Because this test is placed at the beginning of ActivatorSetup() that has already been delayed to another video frame, it is very unlikely that these statements will be interrupted for time. OnUpdateHandled only transitions here from false to true, or true to true, and can be considered idempotent.

Thus far, reports of thread interruption are rare. The consequences here are merely script log/trace entries and a weapon that drops on the floor.

 

Edited June 18 by DayDreamer

[DayDreamer]

Another tiny, but significant, WeaponRackActivateSCRIPT change was to split its vanilla AlreadyInit into two parts: OnUpdateHandled and PlacedItemInit.

AlreadyInit was set in vanilla to mean both things, resulting in confusion and sometimes inoperable racks.

OnUpdateHandled means only that the invisible rack part has begun initializing after the visible rack, and is a concurrency check. Previously, an attempt to synchronize the visible and invisible parts used OnUpdate as a delayed concurrency mechanism. Multiple calls to RegisterForSingleUpdate() result in only one update. Clever, but better handled by using states to prevent concurrent event handling.

PlacedItemInit means that PlayersDroppedWeapon contains the StartingItem (or the player's item), or has been deliberately set to None.

The difference is especially important in automatically reparing racks. The visible rack's trigger can be obstructed by an item from another rack. This can be detected using GetTriggerObjectCount(), and comparing it to PlacedItemInit and PlayersDroppedWeapon.

Edited June 18 by DayDreamer

[DayDreamer]

Many vanilla weapons and armor obstruct adjacent racks.

The US*P trigger area on the visible rack is significantly reduced in size, now more like a central button than covering much of the visible rack.

All vanilla weapons and armor were checked to ensure that they obstruct that central button. As the player takes an item from the rack, the button OnTriggerLeave() signals that something has been taken.

However, that may be an item from a adjacent rack. This was especially a problem for CoA racks, where weapons criss-cross over the center rack.

Every vanilla weapon was checked to ensure that it does not cross an adjacent button after placement. In many cases, large weapon heads were moved upward to appear balanced on pegs at the top, or downward to appear hanging from a peg (or pegs) by the handle. Various swords are flipped by 90 or 180 degrees, depending on which rack is used.

Bows were particularly problematic. We used a CK warehouse to put every bow on a long row of racks to position their node attachment points without obstructing each other.

Staves are another thing entirely. More code was written over the past decade for detecting and placing staves than any other item.

But nothing can be done about non-vanilla items. Therefore, considerable checking exists to resuscitate racks after an offending item has been removed.

 

Edited June 18 by DayDreamer

[Arthmoor]

I think it's fair to say we can stop worrying about rack activators obstructing each other since those types of displays are literally everywhere in the game and they're working just fine.

If there's obsolete code that was only there for the benefit of LE then that's fine to remove that. So long as removing it doesn't cause further problems.

This talk of the VM running faster is an old rumor begun on reddit after the 1.6 update but there's no evidence to support it at all. Behavior of the game in general would have been radically altered and easily noticed had this actually happened.

I think it's also worth noting that the issues in both Highmoon Hall and Warmaiden's have yet to be reproduced by anyone else on the team. So I don't think rushing into yet another complete rewrite of these scripts is warranted right now.

[BlackPete]

Quote

I think it's also worth noting that the issues in both Highmoon Hall and Warmaiden's have yet to be reproduced by anyone else on the team. So I don't think rushing into yet another complete rewrite of these scripts is warranted right now.

It occurred to me that I haven't noticed problems with the weapon racks in either of the two locations for quite some time now, and I've been playing quite a bit since the issues were occurring off and on. Arthmoor's theory about it being an engine issue is seeming more and more like it might be true. Though I don't want to interfere with any decisions being made, I thought it might be important to mention this.

Edited June 24 by BlackPete

[DayDreamer]

On 6/23/2024 at 3:00 PM, Arthmoor said:

rack activators obstructing each other

As I've explained above, that's because we fixed:

1. the nif size of the trigger. Taleden made it a bit too small, and I'd had to enlarge it a bit, but it's still a button instead of covering most of each rack.
2. lots of weapons, so they don't cross any adjacent rack's trigger.

If you turn on all the debug lines, you'll find that the great axe in Warmaiden's is positioned handle upward. As it is mounted in the US*P orientation, it swings around head upward, and triggers adjacent racks. That's why we have a state to prevent the triggers.

 

On 6/23/2024 at 3:00 PM, Arthmoor said:

benefit of LE then that's fine to remove that

Yes. As I mentioned, the code path executed by BlackPete was only for backward compatibility with USKP 1.3.3; so that's what I've removed, and changed the internal code documentation to match.

 

On 6/23/2024 at 3:00 PM, Arthmoor said:

rushing into yet another complete rewrite of these scripts

The main change for v6.0, as I'd mentioned elsewhere, is to move the state change out of CheckConfiguration() into its callers; as the first statement, so that it happens as soon as possible. That plugs a cross-thread interrupt window. The CK documentation now says each script function call can have a "short" delay.

The only other significant change that I've made is to both CheckConfiguration() functions to make them return the value of the other rack part. They return None for failure. The code is otherwise line by line identical. This is to reduce invisible side effects.

These are not "complete" re-writes, or even major ones. As diff shows very few lines have changed. But the changes are important!

Anyway, the previous discussion was on BethSoft, but those links don't work anymore. It took me a few days of studying the code. I'd long since forgotten what we'd done a decade ago.

(I'd saved many of the earlier 2012-13 efforts, Taleden's v2, v3, v4, etc. Lots of stuff to work my way through jogging memory.)

 

[DayDreamer]

On 6/23/2024 at 3:00 PM, Arthmoor said:

VM running faster is an old rumor begun on reddit

I don't read reddit, so I don't know anything about that.

I merely looked at the traces provided. I examined the concurrency issues demonstrated by walking through the code, step by step. Concurrency between the activator and trigger parts both running OnCellAttach() at nearly the same time, affecting each other.

I've speculated (in BlackPetet's report topic) that the VM is running faster, or perhaps in more rapid scheduling time slices, because that's what the evidence provided suggests. It isn't happening on my older machine.

I've provided a theoretical fix (v6.0), that testing by BlackPete says immediately fixed his problem. That's the best I can do to help.

---

I'll note that I've had complaints about other scripts (Wilderness Interactions) not running on cells after the Touring Carriage has passed. I've narrowed down the issue to cells and objects not unloading as often as they did on older machines. Newer machines have a lot more main and graphics memory. Touring Carriages exacerbates the script problems, as it passes many cells fairly rapidly during a trip. (Great for testing though.)

The only fix in that case is Save to Desktop, and reload.

Thankfully, we also updated the critter scripts to be OnCellAttach() as well. So they are still working!