[LE] I have a script that works fine most of the time. On the 4th use, the script stops cold. (source gist included)

[monsto]

Here's the entire script source.

I posted this thing yesterday, thinking I could workaround this problem by just deleting the pedestal in a different manner... but since this script completely stops, it seems there's a larger problem that truly needs to be addressed.

This script is attached to a working in-game mannequin replacement.

• Activate it to get a UIExtensions Listmenu
• Menu choices are: Show inventory, Change mannequin (body type presets), toggle statue/moving, toggle pedestal tall/short, KILL (remove entirely).

Selecting "Change mannequin"

• gives a UIExtensions Listmenu of preset names
• disables original
• places the new
• transfers the inventory from orig to new
• deletes original.

All mannequins have a pedestal. Above, "delete original" means I delete the pedestal, then delete self, then return.

The offending moment comes on line 209, deleting the pedestal before deleting self.

I did the registerforupdate to see if the update would run after the lockup. While "change 8" is never output to the console, "5sec update" never comes either... it seems that the script has completely just stopped running for whatever reason.

"Sometimes", the output stops at "change 7" but I can continue to activate/use the new mannequin. When I do this, "5sec update" plays, and the old pedestal stays there, dead.

"Sometimes", after the stop, I can spawn in new mannequins as if nothing is different,

"Sometimes", I can't activeate anything. Other mannequins, and a chest is all i have in the test cell, but neither of them are usable.

Secondarily, I have a completely separate script that I made that has a bunch of helper functions... things like press a key to toggle fly/clip mode, move selected references by a relative distance with a simple console command, etc... "sometimes" that helper script is completely gone after this mannequin script stops. As if it never ran in the first place.

notify and conmsg are defined #262. Tehy're basically just typesaver functions for output to console or debug.

This is pretty f*#@ing frustrating as the entire thing works like a charm and is about half the size of the original SPODUM. This one error makes it problematic, leaving dead pedestals everywhere, not to mention the script just outright dying.

I'm also open to some kind of workaround that avoids deleting the pedestal, or even doing some other method for mannequin change.

 

[Reneer]

There is a major lack of sanity checks in your code. In particular, you never check to see if "BUMPed" is actually filled in a lot of places. What is showing up in your Papyrus logs?

[monsto]

  On 7/21/2019 at 1:08 AM, Reneer said:

There is a major lack of sanity checks in your code. In particular, you never check to see if "BUMPed" is actually filled in a lot of places. What is showing up in your Papyrus logs?

 

At the end, there's absolutely nothing relevant. Lemme generate a new one and make a gist.

 

BUMPed always works at the point of generation. It's property is an ingame miscitem (I should prob make it a static) and it 100% shows.

[monsto]

  On 7/21/2019 at 1:08 AM, Reneer said:

There is a major lack of sanity checks in your code. In particular, you never check to see if "BUMPed" is actually filled in a lot of places. What is showing up in your Papyrus logs?

 

Here's the papyrus log.

 

It had previously been spammed by unrelated, unused scripts. I cleared them out just now and whittled the log from 50k lines to it's now svelte 2k lines, and I'm wondering if the entry at #1849 is relevant. It's 2 sec before the end of the file, and probably right on time as to when the stop at "change 7" occurs.

 

[edit] oh wait, no it's irrelevant. There's a bare delete in setpedestal(). onInit of a new mannequin, that delete runs before putting down a new pedestal. So that error occurs after the lockup.

Edited July 21, 2019 by monsto

[Reneer]

  On 7/21/2019 at 1:11 AM, monsto said:

At the end, there's absolutely nothing relevant. Lemme generate a new one and make a gist.

 

BUMPed always works at the point of generation. It's property is an ingame miscitem (I should prob make it a static) and it 100% shows.

You should probably call Disable() before you call Delete() on the Objectreference as per the notes in the Delete wiki page. Or maybe you can try switching to DeleteWhenAble to see if that helps.

[monsto]

  On 7/21/2019 at 1:53 AM, Reneer said:

 

You should probably call Disable() before you call Delete() on the Objectreference as per the notes in the Delete wiki page. Or maybe you can try switching to DeleteWhenAble to see if that helps.

 

Good to know. Will add it.

[ReDragon2013]

Your script is based on "aaSLuckMannHuman.psc" that was compiled in year 2012.

sLuckyD has made some mistakes caused by not knowing the weaknesses of Skyrim papyrus implementation.

 

1) Do not overwhelming the OnInit() event. Follow code snippet as sample:

 

  Reveal hidden contents

 

EVENT OnInit()
    Debug.Trace("OnInit() - has been reached.. " +self)
    RegisterForSingleUpdateGameTime(0.0)
ENDFUNCTION


EVENT OnUpdateGameTime()
    Utility.Wait(0.1)

  ;  WHILE !self.Is3DLoaded()
  ;      Utility.wait(0.5)
  ;  ENDWHILE

    IF ( BUMPed  )
    ELSE
        setPedestal()
    ENDIF

    invSlot = new Form[10]
    self.EnableAI(setAI)
    Utility.wait(0.1)
    Pose()

    converted = 1
    RegisterForMenu("InventoryMenu")
ENDEVENT


EVENT OnCellAttach()  ; when the cell loads due to player visibility...
IF self.IsDisabled()
    RETURN    ; - STOP -    do nothing mannequin has been switched off
ENDIF
;---------------------
IF (converted == 1)
ELSE
    Debug.Trace("OnCellAttach() - has been reached.. " +self)
    RegisterForSingleUpdateGameTime(0.0)
ENDIF
ENDEVENT


EVENT OnLoad()
    self.EnableAI(setAI)
    Utility.wait(0.1)

    IF ( BUMPed )
        self.MoveTo(BUMPed, 0.0, 0.0, Math.ABS(pedset), TRUE)
        Pose()
    ENDIF
ENDEVENT

 

 

 

2) DO NEVER USE construct as follow in papyrus !!!

    WHILE !self.Is3DLoaded()
        Utility.wait(0.5)
    ENDWHILE

3) Whenever a script has placed an object or actor to Skyrim world you have to use something like this:

    BUMPed.Disable()
    BUMPed.Delete()
    BUMPed = None

to remove such kind of reference. Keep in mind the native function "Delete()" marks the reference only for delete, it never made them deleted.

 

4) Whenever you load a game the following vanilla script "PF_MannequinStay_000D7510" will be running, if not using a mod that changes the original package.

 

  Reveal hidden contents

 

;BEGIN FRAGMENT CODE - Do not edit anything between this and the end comment
;NEXT FRAGMENT INDEX 3
Scriptname PF_MannequinStay_000D7510 Extends Package Hidden
{v1.4 ReDragon 2014}

;BEGIN FRAGMENT Fragment_2
Function Fragment_2(Actor akActor)
;BEGIN CODE
    myF_Move(akActor, 2)
;END CODE
EndFunction
;END FRAGMENT

;BEGIN FRAGMENT Fragment_1
Function Fragment_1(Actor akActor)
;BEGIN CODE
    myF_Move(akActor, 1)
;END CODE
EndFunction
;END FRAGMENT

;BEGIN FRAGMENT Fragment_0
Function Fragment_0(Actor akActor)
;BEGIN CODE
    myF_Move(akActor, 0)
;END CODE
EndFunction
;END FRAGMENT
;END FRAGMENT CODE - Do not edit anything between this and the begin comment

;-----------------------------------
FUNCTION myF_Move(Actor aRef, Int i)
;-----------------------------------
; Use this way to trace the script action, not using the main debug files Papyrus.?.log
; http://www.creationkit.com/OpenUserLog_-_Debug

;;    Debug.OpenUserLog("PF_MannequinStay_000D7510")
;;    Debug.TraceUser("PF_MannequinStay_000D7510", " Fragment_" +i+ "() - Move to editorLocation.. " +aRef, 0)

;;;    Debug.Trace("PF_MannequinStay: Fragment_" +i+ "() - Move actor to editorLocation.. " +aRef)

IF ( aRef )
    aRef.MoveToMyEditorLocation()
ENDIF
ENDFUNCTION

 

 

 

adjusted script "aaBumHumanScript" a bit rewritten, not yet compiled or tested

  Reveal hidden contents

 

Scriptname aaBumHumanScript extends Actor
{Better Unlimited Mannequins by Monsto [v0.1]}
; https://forums.nexusmods.com/index.php?/topic/7832768-i-have-a-script-that-works-fine-most-of-the-time-on-the-4th-use-the-script-stops-cold-source-gist-included/

  Idle PROPERTY idleBoyRitual auto

  Static PROPERTY aaBumPedDwemer24h01   auto
  Static PROPERTY aaBumPedDwemer88h02   auto
  Static PROPERTY aaBumPedSolitude32h01 auto
  Static PROPERTY aaBumPedSovngard58h01 auto

  MiscObject PROPERTY aaBumPedestal        auto
  MiscObject PROPERTY aaBumPedestalShort   auto
  MiscObject PROPERTY aaSLuckMannInvisiPed auto
  MiscObject PROPERTY aaBumDrop            auto

  FormList PROPERTY aaBumList auto
  Form[] invSlot

  Float pedset    = -99.0
  Int   converted                        ; default=0

  Bool setAI     = TRUE
  Bool pedLarge  = TRUE

  ObjectReference BUMPed


;-- EVENTs --

EVENT OnInit()
    Debug.Trace("OnInit() - has been reached.. " +self)
    RegisterForSingleUpdateGameTime(0.0)
ENDFUNCTION


EVENT OnUpdateGameTime()
    Utility.Wait(0.1)

    WHILE !self.Is3DLoaded()
        Utility.wait(0.5)
    ENDWHILE

    IF ( BUMPed  )
    ELSE
        setPedestal()
    ENDIF

    invSlot = new Form[10]
    self.EnableAI(setAI)
    Utility.wait(0.1)
    Pose()

    converted = 1
    RegisterForMenu("InventoryMenu")
ENDEVENT



EVENT OnCellAttach()  ; when the cell loads due to player visibility...
IF self.IsDisabled()
    RETURN    ; - STOP -    do nothing mannequin has been switched off
ENDIF
;---------------------
IF (converted == 1)
ELSE
    Debug.Trace("OnCellAttach() - has been reached.. " +self)
    RegisterForSingleUpdateGameTime(0.0)
ENDIF
ENDEVENT


EVENT OnLoad()
    self.EnableAI(setAI)
    Utility.wait(0.1)

    IF ( BUMPed )
        self.MoveTo(BUMPed, 0.0, 0.0, Math.ABS(pedset), TRUE)
        Pose()
    ENDIF
ENDEVENT


EVENT OnItemAdded(form akBaseItem, Int aiItemCount, ObjectReference akItemReference, ObjectReference akSourceContainer)
    IF (self.GetNumItems() > 10)
        self.removeItem(akBaseItem, aiItemCount, TRUE, Game.GetPlayer())
        debug.notification("Only TEN (10) Items may be given to mannequins.")
    ELSE
        self.addToInvSlot(akBaseItem)
        Equip()
    ENDIF
ENDEVENT


EVENT OnItemRemoved(form akBaseItem, Int aiItemCount, ObjectReference akItemReference, ObjectReference akDestContainer)
    self.removeFromInvSlot(akBaseItem)
ENDEVENT


EVENT OnActivate(ObjectReference akActionRef)
    myF_Menu()
ENDEVENT


EVENT OnMenuClose(String MenuName)        ; SKSE required!
    self.Equip()
ENDEVENT


EVENT OnUpdate()
    notify("5 sec Update")
ENDEVENT


; -- FUNCTIONs -- 10

;-------------------
FUNCTION ForceKill()
;-------------------
    ; empty.. maybe a place holder
ENDFUNCTION


;------------------------
FUNCTION conmsg(String s)  ; shortcut to print a console message
;------------------------
    ConsoleUtil.PrintMessage("||  " + s)
ENDFUNCTION


;------------------------
FUNCTION notify(String s)  ; notify user with both console and debug messages
;------------------------
    ConsoleUtil.PrintMessage("// " + s)
    Debug.Notification("BUMs:  " + s)
ENDFUNCTION


;--------------
FUNCTION Pose()
;--------------
    self.PlayIdle(idleBoyRitual)
ENDFUNCTION


;---------------
FUNCTION Equip()
;---------------
IF UI.IsMenuOpen("InventoryMenu")            ; SKSE required!
    RETURN    ; - STOP -    safety first
ENDIF
;---------------------
    self.UnEquipAll()
    Utility.wait(0.1)

int i = 0
    WHILE (i < 10)                    ; cur <= 10
        form fm = invSlot[i]
        IF ( fm )
            self.EquipItem(fm, False, False)
        ENDIF
        i = i + 1
    ENDWHILE
ENDFUNCTION


;------------------------------------------
FUNCTION removeFromInvSlot(Form akBaseItem)
;------------------------------------------
int i = 0
    WHILE (i < 10)
        IF (invSlot[i] == akBaseItem)
            invSlot[i] = None
            RETURN    ; - STOP -
        ENDIF
;        ----------------------
        i = i + 1
    ENDWHILE
ENDFUNCTION


;-------------------------------------
FUNCTION addToInvSlot(Form akBaseItem)
;-------------------------------------
int i = 0
    WHILE (i < 10)
        IF invSlot[i]
            ; slot already in use
        ELSE
            invSlot[i] = akBaseItem
            RETURN    ; - STOP -
        ENDIF
;        ----------------------
        i = i + 1
    ENDWHILE
ENDFUNCTION


;---------------------
FUNCTION setPedestal()
;---------------------
    self.BlockActivation(TRUE)

; remove older placed pedestal
    IF ( BUMPed )
        BUMPed.Disable()
        BUMPed.Delete()
        BUMPed = None
    ENDIF

; switch pedestal types depends on settings
    IF ( pedLarge )
        pedset = -99.0
        BUMPed = self.PlaceAtMe(aaBumPedestal as Form, 1, False, False)
    ELSE
        pedset = -56.0
        BUMPed = self.PlaceAtMe(aaBumPedestalShort as Form, 1, False, False)
    ENDIF

    BUMPed.MoveTo(self as ObjectReference, 0.0, 0.0, pedset, TRUE)
    BUMPed.SetAngle(0.0, 0.0, BUMPed.GetAngleZ())

    self.MoveTo(BUMPed, 0.0, 0.0, Math.ABS(pedset) + 2.0, TRUE)
ENDFUNCTION


;------------------
FUNCTION myF_Menu()
;------------------
    UIListMenu m = UIExtensions.GetMenu("UIListMenu") as UIListMenu        ; m = configMenu

; -=- MENU LOGIC HERE -=-
    
    m.AddEntryItem("Inventory",        -1, 0, False)                ; a0
    m.AddEntryItem("Change Mannequin", -1, 1, False)                ; a1

IF self.IsAIEnabled()
    m.AddEntryItem("Toggle [Living] vs Statue",      -1, 2, False)    ; a2
ELSE
    m.AddEntryItem("Toggle Living vs [Statue]",      -1, 2, False)
ENDIF

IF ( pedLarge )
    m.AddEntryItem("Toggle Pedestal: [Tall]/ Short", -1, 3, False)    ; a3
ELSE
    m.AddEntryItem("Toggle Pedestal: Tall /[Short]", -1, 3, False)
ENDIF

    m.AddEntryItem("K I L L", -1, 4, False)                            ; a4

    m.OpenMenu()
    int i = m.GetResultInt()                                            ; i = configChoice
    conmsg("menu choice = " + i)
    
    IF (i == 0)                        ; (0) Inventory
        self.OpenInventory(TRUE)
        Utility.Wait(0.25)            ; -- maybe better to use wait here -- ReDragon
;---------
    ELSEIF (i == 1)                    ; (1) change Mannequin
        notify("change")
        myF_Change()
;---------
    ELSEIF (i == 2)                    ; (2) toggle AI
        setAI = !setAI

        IF ( setAI )
            self.EnableAI(TRUE)
            notify("Mannequin now Living.")
        ELSE
            self.EnableAI(False)
            notify("Mannequin now Statue.")
        ENDIF
;---------        
    ELSEIF (i == 3)                    ; (3) toggle pedestal
        pedLarge = !pedLarge
        setPedestal()

        IF ( pedLarge )
            notify("Pedestal now Tall.")
        ELSE
            notify("Pedestal now Short.")
        ENDIF
;---------        
    ELSEIF (i == 4)                    ; (4) pick up all items
        myF_Remove()
    ENDIF
    
; -=- END MENU LOGIC -=-

    IF ( BUMPed )
        self.Equip()
        self.MoveTo(BUMPed, 0.0, 0.0, Math.ABS(pedset) + 2.0, TRUE)
    ELSE
        self.Disable()
        self.Delete()
    ENDIF
ENDFUNCTION


;--------------------
FUNCTION myF_Remove()  ; internal helper
;--------------------
    self.RemoveAllItems(Game.GetPlayer(), TRUE, TRUE)
    notify("Mannequin inventory moved to you.")
        
    BUMPed.Disable(TRUE)
    BUMPed.Delete()
    BUMPed = None
ENDFUNCTION


;--------------------
FUNCTION myF_Change()  ; internal helper
;--------------------
    int i = showmenu()
    conmsg("change 3")            ; *3

IF (i == -1)
    conmsg("change 4d")
    RETURN    ; - STOP -
ENDIF
;---------------------
    ;Utility.wait(0.5)
            
    conmsg("Chosen: > " +i+ " > " + (aaBumList.GetAt(i)).GetName())
    conmsg("change 4a")            ; *4a
    Utility.wait(0.2)
    
    self.Disable()
    conmsg("change 4b")            ; *4b
    Utility.wait(0.2)
    
    actor newmann = self.PlaceActorAtMe(aaBumList.GetAt(Choice) as ActorBase, 4, None)
    conmsg("change 4c")            ; *4c
    Utility.wait(0.2)

    conmsg("change 5")            ; *5
    self.RemoveAllItems(newmann, TRUE, TRUE)        
    notify("Mannequin changed.")        

    conmsg("change 6")            ; *6
    RegisterForSingleUpdate(5.0)
    Utility.wait(0.2)
    
    conmsg("change 7")            ; *7
    BUMPed.Disable()
    BUMPed.Delete()
    BUMPed = None

    conmsg("change 8")            ; *8
    utility.wait(0.2)

    UnRegisterForUpdate()
    conmsg("change 9")            ; *9
ENDFUNCTION


;----------------------
Int FUNCTION showmenu()  ; SKSE required!
;----------------------
    UIListMenu m = UIExtensions.GetMenu("UIListMenu") as UIListMenu        ; m = listMenu

int iMax = aaBumList.GetSize()
int i = 0
    WHILE (i < iMax)
        form fm = aaBumList.GetAt(i)
        m.AddEntryItem(fm.GetName(), -1, -1, False)
        i = i + 1
    ENDWHILE

    conmsg("showmenu 1")
    m.OpenMenu()
    conmsg("showmenu 2")

    RETURN m.GetResultInt()
ENDFUNCTION

 

 

Edited July 21, 2019 by ReDragon2013

[monsto]

Excellent notes and info.

 

It's annoying that standards that you mention (points 1, 2, 3) are only found by the handing down from one generation to the next. I've been piddling with papyrus for I dunno 3-4 years, and have never seen anything like this. It truly belongs on a ckwiki page, something like "Papyrus conventions for cleaner scripting".

 

For point 2 on your list, what's an alternative to that?

 

I'm also a fan of some of the 'programming habits' you've displayed. Comment lines for breaking up the menu elseifs (Elesif? Holy s***...), the comment "skse required", things like that.

 

I left the boilerplate decompiler heading as a credit to the original author and idea In the script. I think the only thing remaining from the original is a handful of lines, like maybe 20.

 

I see exactly what and why about overwhelming init(). I'm assuming that by using an update, it allows the task to be threaded and therefore reducing hitches and framerate drops. I may take that piece of advice when it's time to streamline, but it's working right now and I'm reluctant to mess with it.

 

I have no idea of what is the significance of, or when or how to use, fragment scripts. Is there a primer somewhere?

 

All in all I appreciate your very good papyrus response.

Edited July 21, 2019 by monsto

[Reneer]

  On 7/21/2019 at 9:04 AM, ReDragon2013 said:

2) DO NEVER USE construct as follow in papyrus !!!

    WHILE !self.Is3DLoaded()
        Utility.wait(0.5)
    ENDWHILE

 

The only problem with the above code is that the object might be disabled, its cell might be unloaded, etc (and thus the loop will keep on looping until the object's 3D is actually loaded). What would fix / mitigate this issue would be to have code like this:

float futuretime = Utility.GetCurrentRealTime() + 5.0
While (Self.Is3DLoaded() == false && Utility.GetCurrentRealTime() < futuretime)
; do nothing here, just loop
endWhile
if (Utility.GetCurrentRealTime() > futuretime && Self.Is3DLoaded() == false)
; bad, time to abort / retry.
endif

[monsto]

  On 7/21/2019 at 8:49 PM, Reneer said:

 

  On 7/21/2019 at 9:04 AM, ReDragon2013 said:

2) DO NEVER USE construct as follow in papyrus !!!

    WHILE !self.Is3DLoaded()
        Utility.wait(0.5)
    ENDWHILE

 

The only problem with the above code is that the object might be disabled, its cell might be unloaded, etc (and thus the loop will keep on looping until the object's 3D is actually loaded). What would fix / mitigate this issue would be to have code like this:

float futuretime = Utility.GetCurrentRealTime() + 5.0
While (Self.Is3DLoaded() == false && Utility.GetCurrentRealTime() < futuretime)
; do nothing here, just loop
endWhile
if (Utility.GetCurrentRealTime() > futuretime && Self.Is3DLoaded() == false)
; bad, time to abort / retry.
endif

 

Alright so basically you're saying that it's risky, which I get.

 

Good thoughts. Thank you.