[LE] Script bug and optimization questions

[xWilburCobbx]

My mod is nearing completion so now I am doing some script touch ups before I go all out, place it around the game world, and ship it. I have a couple of unrelated problems, answers to ANY of them would be appreciated.

 

The entire script:

 

 

 

Scriptname AAAxWCxINVxPurchaseScript extends ObjectReference  
{Used to buy a property to generate income.}

Actor property PlayerREF auto

float property fCost auto

GlobalVariable Property gTotalPaid auto
GlobalVariable Property gCnt auto
GlobalVariable Property gCntTotal auto

int property ModObjID auto

MiscObject Property mCoin auto

Message property mMenuP auto
Message property mMenuFailP auto

MusicType property muBuy auto

Quest property qIncByInt auto

AAAxWCxINVxAliasControl property AliasQS auto
AAAxWCxINVxIncomeScript property IncQS auto

Event OnActivate(ObjectReference akActionRef) ;Checks to see if the player has the money and shows the menus
	self.Disable()	;Stops player from spamming activate and prevents bugs
	if PlayerREF.GetGoldAmount() >= fCost
		BuyMenu()
	else
		mMenuFailP.Show(fCost)
		self.Enable()
	endif
EndEvent

Function BuyMenu(int aiButton = 0) ;Takes the player's money/adds 1 to the objective/runs an income update from a remote script
	aiButton = mMenuP.Show(fCost)
	if aiButton == 0 ;Yes
		PlayerREF.RemoveItem(mCoin, fCost as int)
		gTotalPaid.Mod(fCost)
		qIncByInt.ModObjectiveGlobal(1, gCnt, ModObjID, gCntTotal.GetValue(), true, true)
		IncQS.UpdStatVal()
		if qIncByInt.IsRunning() == False ;Starts the income quest if its disabled
			qIncByInt.Start()
		endif
		Debug.Notification("Property purchased!")
		muBuy.Add() ;Plays sound effect
		DeleteAll()
	else ;No
		self.Enable()
	endif
endFunction

Function DeleteAll() ;Fades/Deletes any custom sign objects and controls the alias from a remote script
	ObjectReference LRef1 = self.GetNthLinkedRef(1).Disable(true)
	ObjectReference LRef2 = self.GetNthLinkedRef(2).Disable(true)
	ObjectReference LRef3 = self.GetNthLinkedRef(3).Disable(true)
	LRef1.Delete()
	LRef2.Delete()
	LRef3.Delete()
	AliasQS.AliasCtrl()
	Self.Delete()
endFunction 

 

 

 

(Problem 1: Bug)

 

I can't reproduce the bug, but just encase, I updated the script. The trigger originally only disabled it self on line 39:

if aiButton == 0
                self.Disable() ;Stops player from spamming activate
		PlayerREF.RemoveItem(mCoin, fCost as int)

This was sufficient, but then the bug happened and I don't know why! So now I just disable the trigger immediately and manually re-enable it when necessary.

 

The bug only happened once and I don't know if anyone is familiar with it. The script is ran via an activatable trigger, once activated it shows either a Y/N purchasing menu, or a menu that just tells you that you can't afford it (It does this by comparing a value set in the properties). I was messing around and spamming wait, I would wait an hour at a time (Its important for the mod. It gives money every hr on update) till I had enough money to get the purchase message.

 

I immediately activated one of the triggers as soon as I finished waiting, and it activated the purchase message twice in a row! The first one made the purchase and took the money required for it, the second one took a completely random amount of money (it was supposed to be 5,000 gold as defined in the script properties, it only took 499) and the rest of the values set in the properties processed normally.

 

Did I perform the proper fix for this strange bug? Is this just a fluke that happens in the papyrus engine every once in a while? Or is there an issue with my script that I just don't see?

 

(Problem 2: Functions)

 

I am getting familiar with using functions, I don't typically make them unless I need to run/reference a particular piece of the script multiple times, or remote call a specific set of processes from another script.

 

"Function DeleteAll()" is the only one that has a process that runs once and doesn't need to be remote called for any reason. Is it a good idea to split up the script into blocks of functions that do specific things? Or should custom functions only be used for re-runs/remote calls?

 

I feel like splitting them up is cleaner then just having a stack that runs on and on like this:

 

 

if aiButton == 0
	PlayerREF.RemoveItem(mCoin, fCost as int)
	gTotalPaid.Mod(fCost)
	qIncByInt.ModObjectiveGlobal(1, gCnt, ModObjID, gCntTotal.GetValue(), true, true)
	IncQS.UpdStatVal()
	if qIncByInt.IsRunning() == False ;Starts the income quest if its disabled
		qIncByInt.Start()
	endif
	Debug.Notification("Property purchased!")
	muBuy.Add() ;Plays sound effect
	ObjectReference LRef1 = self.GetNthLinkedRef(1).Disable(true)
	ObjectReference LRef2 = self.GetNthLinkedRef(2).Disable(true)
	ObjectReference LRef3 = self.GetNthLinkedRef(3).Disable(true)
	LRef1.Delete()
	LRef2.Delete()
	LRef3.Delete()
	AliasQS.AliasCtrl()
	Self.Delete()
else 

 

 

Edited July 31, 2020 by smashballsx88

[testiger2]

instead of self.Disable() use a boolean toggle or use states

something like this would work

boolean lock example:

bool bLocked

Event OnActivate()
	if !bLocked
		bLocked = true ; lock OnActivate() to prevent spamming
		
		; rest of code goes here
		; use bLocked = false to unlock
	endif
EndEvent

About the function question:

 

Of course using functions has its pros and cons but most of the time it is perfectly fine to use functions even when they are only called once in your script.

 

A few situations come to mind where you might want to avoid them:

- you need to keep the string count down (every function increases the string count in the save) should be of no concern in most cases unless you add hundres or throusands of functions with your scripts

 

- avoid overhead

there may be times where you have resource intensive tasks (while loops for example) where you are better off not using functions.

Generally speaking function statements increase the amount of instructions the cpu has to process (start/exit adress, function parameters etc. google function overhead if you want an in depth explanation)

This is normally not a problem for bigger functions but on small functions the overhead may even outweigh the actual function and you are better of just using a direct statement if speed is of concern (keyword micro-optimization)

 

The Papyrus engine is also fps-linked. That means that per thread a maximum of 1 function call per frame can be made.

 

 

So to sum it up:

Your example should be fine using functions.

You can avoid functions where speed is priority but in speed is that much of a priority you most likely will find other bottlenecks in your code before this one.

And don't forget that you also have to read your code or may want others to read your code. One function line is often easier to read that big a block of text (in your script for example)

 

 

 

[xWilburCobbx]

Thank you for taking the time to answer my questions!

 

1)

I can switch to using a bool, seems like it would be faster and cleaner.

 

My only concern is I don't know if setting a variable outside a function will set it for every instance of that script ran by the specific object.

 

So lets say the first run sets bLock to true, but it bugs out and runs OnActivate twice. Will the second one read true and stop?

 

2)

I went ahead and just eleminated functions that weren't necessary, and just space out and comment on blocks of code to organize it. I decided that unecessary functions don't provide much benefit for its added slowness, and my new method is just as organized, if not more so.

Edited August 1, 2020 by smashballsx88

[testiger2]

1) You already have Auto Properties in your script. You Can treat Auto Properties as variables with outside accsess. Meaning they provide the same functionality as variables but can be accessed by other scripts or the CK.

 

I think you misunderstand the way the instancing works

When you attach a script to an object it receives one instance of the script but not multiple.

When you create multiple instances of said object they each get a independant instance of the script.

 

When you happen to spam the activate button there are not multiple instances beeing called but the same instance calls the OnActivate event multiple times. This is why you want to use a lock variable

 

- user spams button

- 1st iteration of OnActivate gets called, the following are queued

- 1st iteration sets lock = true and proceeds

- 2nd, 3rd, etc iteration enters OnActivate and sees lock == true -> skips locked code and leaves ( does nothing)

- 1st iteration finishes and sets locked = false

 

2) The examples where you want to eliminate functions i gave you are very specific cases and may have been a bit misleading.

It is best-practice to keep your code in small logical functions that have a specific task. Your BuyMenu and Deletall functions were totally fine in that regard.

When i mentioned the overhead on small functions i meant the ratio between the function statement and the actual code.

So while very short functions may produce more overhead in relative terms, very large functions still produce more overhead in absolute terms.

 

Again this falls under the case of mirco-optimization and you should worry about the functionality of your code befor this.

 

There is a phrase in programming saying: premature optimization is the root of all evil

[maxarturo]

Just to elaborate on testiger2 post, that is well explained.

"So lets say the first run sets bLock to true, but it bugs out and runs OnActivate twice. Will the second one read true and stop?"

There are circumstances where the PC may be too busy and / or it will fire twice something (the infamous weird game engines issues).

One example which i have personally experienced is with a 'Global Variable' not changing because the system was way too busy at the time of the call and it just jumped it / skipped it, this f**** up everything, the whole quest / mod !!!.

* Just one Gloval Variable...

To eliminate any of those possibilities been caused by the game engine / PC system, you need to use 'States', since changing a script's 'State' is one of the fastest functions there are, and they are the most reliables.

Using testiger2 script example i'm adjusting to states.

Auto State WaitingPlayer
Event OnActivate()
      GoToState("Activated") ; lock OnActivate() to prevent spamming
 
      ; rest of code goes here

      GoToState("WaitingPlayer")  ; to unlock OnActivate()
endif
EndEvent
EndState
 
State Activated
Event OnActivate()
      ; Do Nothing
EndEvent
EndState

Even if the 'OnActivate()' fires twice the second OnActivate it will ALWAYS fire in the "Empty State".

Edited August 1, 2020 by maxarturo

[xWilburCobbx]

1)

Yeah I was suspicious of that. I wasn't sure if the script was just 1 instance that can run multiple times, or if it just re-initiated everytime it ran.

 

Now I know to use that to my advantage, and using a bool makes a lot more since to me.

 

2)

I see, guess I jumped the gun on that one. As you can probably tell, I am no programmer, but toddlers can make papyrus scripts so I am comfortable with it.

 

I am just now learning some basic concepts of programming, and my biggest concern after many projects I tossed, is going beyond just functionality.

 

I want to make sure my scripts wont slow the game down too much, especially when other mods are doing stuff. I am trying to make everything as clean and professional as possible, so in the future I have the skills to write good scripts that behave.

 

I have dumped a lot of research to ensure my current scripts do whay they need to do in the shortest way possible, and keep them from save bloating.

[xWilburCobbx]

Just to elaborate on testiger2 post, that is well explained.

This explains a lot! Yeah I am not too familiar with the bugs of the papyrus engine itself.

 

I can always make it use a state, I will read documentation on it and touch up my knowledge of states before I just start using them.

 

I do have portions of other scripts that take data from some globals. I am not on my PC right now, but it is a function that gets called by this very script.

 

IncQS.UpdStatVal()

 

It takes values from 2 globals, runs a math operation with them and stores the result in a 3rd global just meant for text replacement.

 

Should this go in a state? I feel like it would be pointless to switch that into a state, and extremely problematic as the script has other functions that need to be called when it updates.

Edited August 1, 2020 by smashballsx88

[maxarturo]

"Should this go in a state? Or states mostly good for stopping the script from re-running something while its already running? I feel like this is already prevented by putting the OnActivate() in an empty state."

That depends on what your script does and how it does it, and how busy the system is at that time.

States can be use to "for stopping the script from re-running...", but they have multiple functions.

One of the primary uses / functions of a 'State' is to make a script (the same script / the same activator) do something else when you re-activate it.

Example:

Auto State Sequence01

Event OnActivate()

          ......Do something

          ......Do something else

 

Now we want the same script / activator to do something else the next time the player activates it

 

          GoToState("Sequence02")

EndEvent

EndState

 

 

 

Next time the player activates this it will run the bellow

 

State Sequence02

Event OnActivate()

          ......Do something

          ......Do something else

 

Now we want the same script / activator to do the functions on 'Sequence01' or jump to an other "SEQUENCE"

 

          GoToState("Sequence01")  ;Or the next sequence

EndEvent

EndState

If you need more info on this or in-game functionable scripts as examples, don't hesitate to ask for it.

Have a happy modding.

Edited August 1, 2020 by maxarturo

[xWilburCobbx]

That depends on what your script does and how busy the system is at that time.

Yeah I got a little excited and skimmed through the concept of a state on the wiki, I will make sure to read it thoroughly when I go off work for the next day or 2.

 

I can see so much potential for this! Other then preventing my strange bug, I don't have a use for it else where quiet yet. Especially since Function UpdStatVal() needs to be ran by this script and it's home script.

 

Luckily if UpdStatVal() is fumbled, its totally fine as long as it is ran again, its not serious.

 

gTotalPaid.Mod(fCost) is in the script I showed you, and its the absolute most important one that CAN NOT fail under any circumstances.

Edited August 1, 2020 by smashballsx88

[NexusComa2]

What bug? ... You never told us what it is saying or doing. You just keep saying "the bug".

As far as disabling that don't sound wise. Let me introduce you to one of my favorite flags.

It is called: First Time Flag. I'll use an int here to show you but you could also use a bool.
I like to use a int as it gives me the choice of using it for a multi flag. if > 0 ... if > 1.
But the concept remains the same. This little flag is useful in so many ways.
One of them is being able to totally stop spamming.

 

Int ftf = 0

 

Event Whatever()

If(ftf ==(0))

ftf =(1)

bla bla bla

ftf =(0)

Endif
EndEvent

In your case:

int ftf = 0

utility ;For the wait command. I usually don't use a wait with this but in this case it may be helpful.

Event OnActivate(ObjectReference akActionRef) ;Checks to see if the player has the money and shows the menus
if ftf == 0
ftf = 1
if PlayerREF.GetGoldAmount() >= fCost
BuyMenu()
wait(2.0)
ftf = 0
else
mMenuFailP.Show(fCost)
wait(2.0)
ftf = 0
endif
endif
EndEvent

 

Edited August 2, 2020 by NexusComa2