mrclint Posted March 31 Share Posted March 31 (edited) Heya, So I run an If condition to check whether my script is eligible or not to execute. !ThisActorClosest.hasSpell(BrokenArmorDistr) && !ThisActorClosest.isdead() <-- Is the NPC already processed or dead? Do not run. This should be first, because no use to check the rest if these are true. ThisActorClosest.GetDistance(PlayerRef) < 50000 && ThisActorClosest.GetDistance(PlayerRef) > 5000 <-- If the NPC isn't processed and not dead I want to check if the NPC is not close by. ThisActorClosest.GetDistance(PlayerRef) < 5000 && !PlayerRef.HasLOS(ThisActorClosest) <-- If they are, though, I want to check LOS on the NPC to see if I am looking at em. But I do not wanna run LOS if the upper is true. What is the smartest and most efficient way of writing this? As one line? Or as multiple? I don't wanna involve LOS if not necessary. I also gonna go for more checks later on, like if they have quest items on them, if they are in cities and so on. So one line feels like it can grow very big. Edited March 31 by mrclint Link to comment Share on other sites More sharing options...
xkkmEl Posted March 31 Share Posted March 31 (edited) You can't make the code work if you can't read it. Writing it so it makes sense should be your priority. For efficiency, there is not enough computation here to worry about and for the most part, the shortcut evaluation of "&&" and "||" will take care of it. For readability, I'd write it as a cascading elseif: if ThisActorClosest.hasSpell(BrokenArmorDistr) elseif ThisActorClosest.isdead() elseif ThisActorClosest.GetDistance(PlayerRef) >= 50000 elseif ThisActorClosest.GetDistance(PlayerRef) <= 5000 elseif !PlayerRef.HasLOS(ThisActorClosest) else do_what_needs_doing_here() endif Edited March 31 by xkkmEl 2 Link to comment Share on other sites More sharing options...
dafydd99 Posted March 31 Share Posted March 31 I'll try to talk in general terms here, as papyrus is a bit of an odd one for optimisation, and this might help some others too. I'm assuming you mean most efficient from the point of view of returning quickest in most situations. Here's a few things to bear in mind... Nearly all function calls have to wait for a frame refresh to run. The exceptions here are your own local script functions (on 'self'), or these few non-delayed functions... https://ck.uesp.net/wiki/Category:Non-delayed_Native_Function . This is a HUGE slow down, and usually the reason for slowness in scripts on you'd think should be fast. Any time you call one of these slow functions you'll get that wait, so if you're calling those three ThisActorClosest.GetDistance(PlayerRef) calls, better to store it in a temporary variable eg - float myDistance=ThisActorClosest.GetDistance(PlayerRef) and use that. The overhead for the temp variable is almost nothing. if you're ever doing a simple && condition the optimiser SHOULD drop out after the first false statement as all must be true to continue. (Nearly all languages would do this, but rarely they've been written differently). Because of this always put the LEAST likely condition first (or possibly the shortest running query, if eg it's from a local function call, or local boolean test). If you're uncertain, you can 'force' the optimiser to drop out after running the first condition by placing the rest in an embedded if statement. Sometimes this makes the code a little clearer too - Code clarity vs optimisation is often a trade off, generally I'd go for clartiy over optimistation except in highly performance dependent code, usually stuff that's being run hundreds of (or sometimes way more) times. ArrayLists are MUCH slower than arrays, so if you need to do a lot of operations or tests on an ArrayList, it might be worth caching it in a simple array first. As xkkmEl said, it's a little hard to work out exactly what you're trying to do in your statements. if you break it out into some unoptimised code we may be able to help make an optimal solution for your specific case. 1 Link to comment Share on other sites More sharing options...
xkkmEl Posted March 31 Share Posted March 31 (edited) I'll add to dafydd99: 99.99% of optimizations make no perceptible difference. Optimizing at the expense of readability is almost always a loosing proposition. Only optimize when you see evidence of a performance problem. Only optimize bits of code that run the most often (none of the others will be perceptible). Dafydd99 is however right to point out that there are operations in Papyrus that are a lot more expensive than others and it is a good idea to be strategic about where and when you use them; put enabling conditions in front, not after. 15 minutes ago, dafydd99 said: if you're ever doing a simple && condition the optimiser SHOULD drop out after the first false statement as all must be true to continue. This actually is not a matter of "optimiser". It's part of the language definition. Either the "&&" and "||" operators are shortcut, as in Papyrus and nearly all computer languages, or it is not. PS: Apparently, Wikipedia prefers the term "short-circuit" to "shortcut". Edited March 31 by xkkmEl 1 Link to comment Share on other sites More sharing options...
dafydd99 Posted March 31 Share Posted March 31 Fair enough. Bethesda don't seem to have published much about papyrus, so it's hard to be sure of the exact language definition. The closest I could find was this... https://ck.uesp.net/wiki/Statement_Reference (good to see some form of EBNF there!) - though I haven't really done an exhaustive search. I'm guessing, and I'm not really certain here, that our papyrus scripts are compiled into what is just a tokenised form of the script (probably with no optimisations) and that's parsed by an in-game .pex interpreter - it's possible the only full definition of the language exists in the implementation of this interpreter. 1 Link to comment Share on other sites More sharing options...
xkkmEl Posted March 31 Share Posted March 31 Documentation is deficient. No arguments there. The compiler does not have any form of optimiser as far as I can tell. Under the hood, pex files are handled by something akin to a byte-code interpreter. Just like Java, JS, python and many others. There is also a wiki page on operators that states clearly that "&&" and "||" are short-circuit: https://ck.uesp.net/wiki/Operator_Reference 1 Link to comment Share on other sites More sharing options...
mrclint Posted March 31 Author Share Posted March 31 Alright, after I posted the first message I went back to my code and made it like this: Function NPCControlEligible() ;Check if NPC is eligible for armor cloning If NextActor != PrevActor ;if this actor is not the same as the previous ThisActorClosest = GetClosestActorFromRef(NextActor, true) Else ThisActorClosest = GetRandomActorFromRef(NextActor, 9000, true) ;else we get a random actor EndIf PrevActor = NextActor Float ActorDistance = ThisActorClosest.GetDistance(PlayerRef) If !ThisActorClosest.hasSpell(HasBeenProcessed) && !ThisActorClosest.isdead() ;is not in process or has been processed and is alive If ActorDistance < MaxDistance && ActorDistance > MinDistance AddSpellToNPC() ElseIf ActorDistance < MinDistance && !PlayerRef.HasLOS(ThisActorClosest) AddSpellToNPC() EndIf Else NextActor = PlayerRef EndIf RegisterForSingleUpdate(2) ;for now. EndFunction I made a float of actordistance and ordered them of, first importance, and then of what I think is the least demanding. Because in another iteration of the script I heavily relied on HasLOS and it lagged as expected. The thing is, this runs from my quest alias and should attach a spell on (almost) every single NPC in the game, so it must, essentially, run almost all of the time. And I also want to have alot more checks than I could in the other iteration. But is there another clever/fast solution to finding actors that isn't in your sight? Link to comment Share on other sites More sharing options...
xkkmEl Posted March 31 Share Posted March 31 Use a cloak spell. All your conditions work in the condition system. Link to comment Share on other sites More sharing options...
mrclint Posted April 4 Author Share Posted April 4 I ended up using SPID to queue every actor to a controller, which then loops through them all. Link to comment Share on other sites More sharing options...
PeterMartyr Posted April 4 Share Posted April 4 On 4/1/2025 at 7:12 AM, xkkmEl said: Under the hood, pex files are handled by something akin to a byte-code interpreter. Just like Java, JS, python and many others. Agree to disagree OK mate, I see java and C# has 80-90% compiler 10-20% interpreter, C++ and papyrus 100% compiler and python 100% interpreter, and write my code accordingly for each, and there compilers/interpreter requirements BUT I will concede it is compile code like C# and Java designed for different operating systems at the same time, so may be that would better comparison than JS or Python EDIT since it does work in different OP, its gotta have some percentage interpreter in it, I was just generalising like you, and never thought of this before?... I happy to share my thoughts, but do not wanna go into deep dive over it) BUT from outside looking in, it does behave like Java or C# Link to comment Share on other sites More sharing options...
Recommended Posts