Script API broken for WorkshopScript.psc

[1000101]

Changes have been made to the public API in WorkshopScript.psc.  I am sure other scripts are also affected but this is the one that concerns me as this is the one I am interfacing with.

 

Specifically:

Script:  WorkshopScript.psc

Proper Function Prototype:  Function DailyUpdate( bool bRealUpdate = true )

Broken Function Prototype:  Function DailyUpdate( bool bRealUpdate = true, bool bResetHappiness = false )

 

Result:  Mods relying on the proper unbroken (AKA vanilla) API will receive errors in the log to-wit, "error: Incorrect number of arguments passed. Expected 2, got 1" and the function will not be called breaking the expected behaviour.

Resolution:  Revert API and find a different way to pass this information or warn users to not use UFO4P as it can potentially break other mods by breaking the API and compatibility.

 

Additional information:  Not needed, open the vanilla script source and the UFO4P version and see that UFO4P changes the function prototypes.

[Scrivener07]

I can confirm that I get the same result as the original poster. Any script that was compiled against the vanilla papyrus source will throw a runtime error when a user has UFO4P installed because the function definition has been changed.

This is the vanilla function definition.

function DailyUpdate(bool bRealUpdate = true)

This is the UFO4P definition.

function DailyUpdate(bool bRealUpdate = true, bool bResetHappiness = false)

As an example, if a mod was compiled with the vanilla sources and a user installs UFO4P these errors are thrown in the papyrus log.
 

[03/28/2017 - 02:29:55PM] UFO4P_Test
[03/28/2017 - 02:29:55PM] error: Incorrect number of arguments passed. Expected 2, got 1. 
stack:
	[AAA_UFO4P_Test (09000F99)].UFO4P_Test.OnQuestInit() - "C:\Users\Scrivener07\AppData\Local\Temp\PapyrusTemp\UFO4P_Test.psc" Line 7

Scriptname UFO4P_Test extends Quest

WorkshopScript Property WS Auto Const Mandatory

Event OnQuestInit()
	Debug.Trace("UFO4P_Test")
	WS.DailyUpdate(false)
EndEvent

 

[Sclerocephalus]

This is a tough one because the ResetHappiness function (on WorkshopParentScript) could be called from unlocked threads in the vanilla version of the scripts (as a result, the happiness at any of your settlements could suddenly drop to the start value). As a concequence of locking this (to prevent aforementioned issues from happening), the DailyUpdate function had to be modified to prevent it from hanging in a dead lock when called by ResetHappiness itself.

The current solution is actually a workaround to the workaround already.

I'll see what i can do.

 

EDIT: Have added it to the tracker:

https://afktrack.afkmods.com/index.php?a=issues&i=22234

[Scrivener07]

Thanks for taking a look into this

[Sclerocephalus]

This is much farther reaching than I had in mind:

As already mentioned, the ResetHappiness function could be called from unlocked threads in the vanilla version of WorkshopParentScript, but some threads that were calling it were already locked (in the UFO4P 2.0.1 version, it is only called from locked threads).

In the vanilla version of WorkshopScript, the DailyUpdate function was not locked, so there was no issue when ResetHappiness called DailyUpdate. In pretty much every UFO4P version however, the DailyUpdate function is locked. [NB: There may be no obvious reason to lock sole functions that are running on spearate instances of WorkshopScript - but the issue was not created on WorkshopScript; it was created on WorkshopParentScript when any number of DailyUpdates on separate workshops were calling sensitive functions at the same time.]

Unlocking the DailyUpdate function is not an option for us, as this would mess up the workshop stats [and not only the stats, it's actually the data on which the workshop scripts are oprating] at pretty much all times. Means that I must find another way to circumvent potential dead locks when ResetHappiness calls DailyUpdate.

[Sclerocephalus]

Which leads me to the question why you aren't simply using the UFO4P scripts as the basis for your mods because you will be running into exactly the same problem (and then have to reinvent the wheel).

Unless you do not lock the DailyUpdate function, of course, but I don't see any good reason for not doing this.

This would explain however why there a still people complaining oaccasionally that the pip-boy stats fix doesn't work for them although they have UFO4P installed.

[1000101]

Wow, I wasn't expecting such quick feedback.

As to why I am not using UFO4Ps scripts as my base...because I always use vanilla then check against an unofficial patches changes for pitfalls.  Also, the real question isn't why am I not using UFO4P but why is UFO4P breaking the API and compatibility?

I am not trying to be hostile simply that as a professional software developer I always develop and test in a clean environment.  In this case, a clean environment means no mods which haven't been made by the original game developers.  If it wasn't the UFO4P but the OFO4P then I would have it included while developing my mod(s).

 

Anyway, thanks for looking into this, I do use the UFO4P during regular play throughs and appreciate the time you guys spend fixing someone elses mistakes.

[Sclerocephalus]

In this case, the current solution is the only one that has no flaws, because I have to tell the affected thread itself that it is safe for it to call back a non-public function on WorkshopParentScript.

I can offer a workaround with a minor flaw, but there is a good chance that this flaw won't matter. This is currently being tested.

[1000101]

What about reverting the vanilla function prototype and have it call an internal function with the additional parameter?  Since you are updating any script that would call it anyway, those calls can use the new internal API.

[Sclerocephalus]

13 hours ago, 1000101 said:

What about reverting the vanilla function prototype and have it call an internal function with the additional parameter?  Since you are updating any script that would call it anyway, those calls can use the new internal API.

 

That's what I did with the remaining API problems on WorkshopParentScript. Many of them date back to to late spring/early summer last year, to very early UFO4P versions when I didn't realize that API compatibility would become that much of an issue in FO4. It never was an issue in Skyrim!

The public versions of all functions I had to add on WorkshopParentScript were modified to use the vanilla prototype (so all external scripts are calling public functions now even if they might have been calliing private versions before) while the private versions were modified as needed (they now have a '_private' appended to their names) and are only called from within WorkshopParentScript itself. Those changes went live with UFO4P 2.0 (so there ahould be far less compatibility problems now; if I didn't overlook something on that script, there actually shouldn't be any left at all).

Now to the current issue: The underlying issue (i.e. the one that actually required the current modification) was a call of the locked DailyUpdate function (on WorkshopScript) from a locked thread on WorkshopParentScript. The call would always come from the ResetHappiness function; that function is always the last task to be carried out by whichever thread that is calling it. Also, the call of the DailyUpdate function was the first action of the vanilla ResetHappiness function, and it did not call any other functions on external scripts.

I therefore solved the issue by splitting the ResetHappiness function in two parts: the first part (which is using the vanilla prototype) starts a short timer on WorkshopScript. This ends the thread on WorkshopParentScript. When the timer event fires on WorkshopScript, it calls the DailyUpdate function with bResetHappiness = true. This bool tells the function to call the remainder of the ResetHappiness function (i.e. the second part which contains the actual code of the vanilla ResetHappiness function) before it finishes running.

Now, since the last part is running on a separate thread anyway, it actually doesn't matter which instance of the DailyUpdate function calls ResetHappiness on WorkshopParentScript - as long as it is an instance of the same WorkshopScript and as long as there is no delay. Therefore, I solved the current issue as follows:

• The DailyUpdate function has been reverted to the vanilla prototype
• bResetHappiness has been added as a script variable to WorkshopScript. Default value is 'false'
• When the timer event catches the call from WorkshopParentScript, it sets bResetHappiness to 'true' and calls the DailyUpdate function. Now, there's no guarantee that this function call will be the instance that calls back WorkshopParentScript (because there may be another thread waiting in the lock): that's the flaw I was referring to. However, it won't matter here which instance is making the call (as explained above).
• Once DailyUpdate has called WorkshopPArentScript, it resets bReseHappiness to 'false'.

 

[Arthmoor]

Just want to point out that the reason API issues didn't come up in Skyrim is because we never had to change the number of arguments on any functions we edited. There were always other solutions.

FO4 makes this considerably more difficult to avoid since there's so much involved in the Workshop system that's intertwined with itself.

[Sclerocephalus]

Fix works as intended. Will be included in the next UFO4P version.

[1000101]

Awesome work, thanks guys.

[Scrivener07]

Great job Sclerocephalus, thanks for your work!