ID:2393377
 
(See the best response by Kaiochao.)
Code:
            if(status_tab_on)
statpanel("Status")
if(statpanel("Status")) if(!afk)
for(var/obj/SFocus/Age/ADis in src)
ADis.suffix="Age [round(Age,0.1)] \[Soul Age [round(Real_Age,0.1)]\]"
stat(ADis)
if(GodKi) for(var/obj/God_Ki/GK in src)
GK.suffix="[AllowGodSense ? "\[Enabled\]":"\[Hidden\]"]"
stat(GK)
for(var/obj/SFocus/Skill_Leeching/LDis in src)
LDis.suffix="[Skill_Leech_On ? "On" : "Off"]"
stat(LDis)
for(var/obj/SFocus/Lethality/LDis in src)
LDis.suffix="[Lethality ? "On" : "Off"] [Spar ? "\[Pulling Punches\]" : ""]"
stat(LDis)
if(Temperature) stat("Status: [Temperature]")
// if(Poisoned>Immunity) stat("Status: Poisoned x[Poisoned-Immunity]")
stat("-------------")
if(WeightedStats>SoftStatCap*7) stat("--(Total Stats Capped)--")
if(Gravity<=10&&Health<0) Health=0
if(Ki<0) Ki=0
if(src)
//StatNameAssign()
for(var/obj/SFocus/Battle_Power/SFBP in src)
SFBP.suffix="(Multiplier)"
if(FBMAchieved==1) SFBP.suffix="(Multiplier) (FBM Achieved)"
if(FBMAchieved==2) SFBP.suffix="(Multiplier) (Ascension Achieved)"
if(Super_Tsufu_Learned) SFBP.suffix="[SFBP.suffix.] (Super Tuffle)"
stat(SFBP)
for(var/obj/SFocus/Energy/SFEn in src)
if(BaseMaxKi>EnergyHardCap*KiMod) SFEn.suffix="[round(Ki)] \[[KiMod]x\] [BaseMaxKi>EnergySoftCap*KiMod? "(Hard Capped)":""]"
else if(BaseMaxKi>EnergySoftCap*KiMod) SFEn.suffix="[round(Ki)] \[[KiMod]x\] [BaseMaxKi>EnergySoftCap*KiMod? "(Soft Capped)":""]"
else SFEn.suffix="[round(Ki)] \[[KiMod]x\]"
stat(SFEn)
for(var/obj/SFocus/Strength/SFStr in src) stat(SFStr)
for(var/obj/SFocus/Endurance/SFEnd in src) stat(SFEnd)
for(var/obj/SFocus/Force/SFFor in src) stat(SFFor)
for(var/obj/SFocus/Resistance/SFRes in src) stat(SFRes)
for(var/obj/SFocus/Speed/SFSpd in src) stat(SFSpd)
for(var/obj/SFocus/Offense/SFOff in src) stat(SFOff)
for(var/obj/SFocus/Defense/SFDef in src) stat(SFDef)

sleep(Stat_Lag)


Problem description:

https://gyazo.com/ccc95276103dc0360c6e11497d0bf75d

(took like 5-10 seconds after this for info to populate)

I adjusted stat lag in game to see if that was the cause, and even with stat lag at 0.1, the time to load the tab is PAINFULLY slow.

In game experience does not slow or lag while tabs are loading, it is merely the tabs


Bump?
You have so many for loops in that one tab, i have total in my game, very painful to look at.
I'm not really an expert, but I suspect having lots of for(var/obj/SFocus/Age/ADis in src)-type loops is not very fast. You may want to experiment with keeping the stat objects in a list of some sort, perhaps a mapping of type -> stat object.
So the stat obj are the actual stats but they are able to be interacted with on the Status panel in order to focus certain stats during training.

That is the reason they are obj
The DB Finale/RP/whatever source has always had issues with tabs becoming laggy.
You don't have to loop just to display it... just display it, if you have a variable pointing at the object you can use that to show it.

var/obj/myobj = new()
stat(myobj)
I'm confused, isn't that what I did with

for(var/obj/SFocus/Speed/SFSpd in src) stat(SFSpd)
No, you're storing the object in src.contents and looping over that to get it. That's terribly slow, especially repeatedly. You want to store the object reference as a variable outright and hold on to it.
In response to Chewyy
Best response
for(var/T/X in Y) is essentially a search through everything inside Y for all objects that match the type T of X. At worst case, it checks every single thing inside Y, basically calling istype(X, T). And it doesn't stop after the first match, because it's a loop. This takes time; more time the more things you have inside Y.

If you only expect one instance of the thing you're searching for, you can use var/T/X = locate() in Y instead. This involves a single linear search through everything in Y and stops after the first one, since it isn't a loop. Best case, it only checks a single thing in Y. Worst case, it checks everything before it finds the one you want.

Even better than locate() would be to keep a direct reference to your object. Whether you add a var to src for it, or use an associative list associating its type to an instance of it (as suggested above), these take advantage of a "hash table" (basically... no search necessary) to get the object you want.


The key to efficiency is to do as little as possible. Ideally, you wouldn't use Stat() at all, instead using a something like a grid, updating grid cells as things change rather than repeating a check to update everything (even things that haven't changed) all at once.
One other thing that's slow here is that you're setting the suffix for these items each time during the loop instead of doing it when the actual var controlling it is turned on or off. It's pointless calculation and assignment that's resulting in a lot of appearance lookups that aren't needed.
Okay, these are for stats that are refreshing for them, but I understand the that interface obj doesn't need to be refreshed each time.

Can you show me an example of what one stat would look like?
I'm not sure what you're asking.
I'd really try to limit your use for for() loops man, you could do a single for(var/obj/SFocus/Focus in src)
And follow it up by a lengthy
if(istype(Focus, path)) [stuff here[ else if(istype(

I mean, it would not be much better but at least it takes a single loop, not 20.


Edit:
I'm entirely unsure and this can be disregarded, but if there is a performance impact from using lots of istype() checks, it could possibly be turned into 'if(Focus.type == PathHere)'
In response to Chewyy
Chewyy wrote:
Okay, these are for stats that are refreshing for them, but I understand the that interface obj doesn't need to be refreshed each time.

Can you show me an example of what one stat would look like?

/Path/To/Mob/Stat()
statpanel("Status")
if(statpanel("Status")) if(!afk)
stat(src.SFStr)
stat(src.SFEnd)


I think this is about what you're looking for actually? This is of course assuming all variables are stored in src, in which case you can apply the function I used and just finish the list I began with the first two.

You just need a new place to do the suffixes in. You'd *probably* want to do it in the objects you're using (SFStr, SFEnd etc)
Path/To/Mob/Stat()
if(statpanel("Status") && !afk)
stat(src.SFStr) //src isn't necessary here.
stat(src.SFEnd) //or here. It's already inferred.


The code you posted wouldn't compile and also wouldn't do what you think it would even if it did. Also, statpanel() creates the panel and returns 1 if usr.client?.statpanel == the supplied name of the panel.

(Yes, usr is valid in Stat() by design. Usr and src may not be the same thing because usr.client.statobj may not be the src of the object where Stat() occurs.)
Also to OP: If all else fails and big-ass stat menus are taking forever, you can speed the process up by using the little-known form for statpanel() that allows you to pass an associative list, or just a list of values. This will allow you to store that list and only update it when things are changed. This will save you a large amount of overhead from repeatedly calling stat() for each line item and repeatedly accessing the variables needed.

Once you are using this format, there are a huge number of optimizations you can do as a result of switching to a lazy-updater rather than using the polling pattern.

You can actually call statpanel() multiple times to append to the same panel:

var/list/statlist = list("flim"=4,"flam"=9,"walla"=7,"bim"=10,"bam"=12)


statpanel("herp")
stat("derp",5)
statpanel("herp",statlist)


The above will display "derp", followed by all the contents of statlist as their own stats as though you individually called stat() for each item in the list.
Let's make your code better step by step.

At first glance, I can see the following improvement, Chewwy. Instead of looping through my contents multiple times, I can loop through them just once.

This is what Laser50 was referring to when he said,

I'm entirely unsure and this can be disregarded, but if there is a performance impact from using lots of istype() checks, it could possibly be turned into 'if(Focus.type == PathHere)'


for (var/obj/SFocus/focus in src)
if (istype(focus, /obj/SFocus/Battle_Power)
// Battle_Power specific
var/obj/SFocus/Strength/SFBP = focus
SFBP.suffix="(Multiplier)"
if (FBMAchieved == 1)
SFBP.suffix="(Multiplier) (FBM Achieved)"
else if (FBMAchieved == 2)
SFBP.suffix="(Multiplier) (Ascension Achieved)"
if (Super_Tsufu_Learned)
SFBP.suffix="[SFBP.suffix.] (Super Tuffle)"
stat(SFBP)
else if (istype(focus, /obj/SFocus/Energy)
// Energy-specific
var/obj/SFocus/Strength/SFStr = focus
if (BaseMaxKi > (EnergyHardCap * KiMod))
SFEn.suffix="[round(Ki)] \[[KiMod]x\] [BaseMaxKi>EnergySoftCap*KiMod? "(Hard Capped)":""]"
else if (BaseMaxKi > (EnergySoftCap * KiMod))
SFEn.suffix="[round(Ki)] \[[KiMod]x\] [BaseMaxKi>EnergySoftCap*KiMod? "(Soft Capped)":""]"
else
SFEn.suffix="[round(Ki)] \[[KiMod]x\]"
stat(SFEn)
else
stat(focus)


We've taken a a step in the right direction, and there are a couple more left.

As Nadrew and others have said, you're looping through a fairly generic list that contains a lot of stuff.

No, you're storing the object in src.contents and looping over that to get it. That's terribly slow, especially repeatedly. You want to store the object reference as a variable outright and hold on to it.


With that in mind, it should be easy for us to create a special list that we use for SFocus objects only.
// Something as simple as this can make your life tremendously easier.
// Just make sure to add your SFocus objects to this list at some point.
mob/var/list/SFocusList = list()

for (var/obj/SFocus/focus in SFocusList)
if (istype(focus, /obj/SFocus/Battle_Power)
// Battle_Power specific
var/obj/SFocus/Strength/SFBP = focus
SFBP.suffix="(Multiplier)"
if (FBMAchieved == 1)
SFBP.suffix="(Multiplier) (FBM Achieved)"
else if (FBMAchieved == 2)
SFBP.suffix="(Multiplier) (Ascension Achieved)"
if (Super_Tsufu_Learned)
SFBP.suffix="[SFBP.suffix.] (Super Tuffle)"
stat(SFBP)
else if (istype(focus, /obj/SFocus/Energy)
// Energy-specific
var/obj/SFocus/Strength/SFStr = focus
if (BaseMaxKi > (EnergyHardCap * KiMod))
SFEn.suffix="[round(Ki)] \[[KiMod]x\] [BaseMaxKi>EnergySoftCap*KiMod? "(Hard Capped)":""]"
else if (BaseMaxKi > (EnergySoftCap * KiMod))
SFEn.suffix="[round(Ki)] \[[KiMod]x\] [BaseMaxKi>EnergySoftCap*KiMod? "(Soft Capped)":""]"
else
SFEn.suffix="[round(Ki)] \[[KiMod]x\]"
stat(SFEn)
else
stat(focus)


I'm not sure if you want to keep SFocus objects in your contents, by the way. If you could remove them entirely from there, that'd be nice for you (as the programmer, it's easier to know where this are), and for your players (the game runs smoother).

With that out of the way... if I were in your position, and was constantly adding type-specific code, I would consider doing something like this instead.

// Assuming that /obj/SFocus types have some sort of reference to their player:

obj/SFocus/proc/DisplayStat()
throw EXCEPTION("Not implemented yet.")

obj/SFocus/Battle_Power/DisplayStat()
var/obj/statData = new()
statData.suffix = "(Multiplier)"

if (player.FBMAchieved == 1)
statData.suffix="(Multiplier) (FBM Achieved)"
else if (player.FBMAchieved == 2)
statData.suffix="(Multiplier) (Ascension Achieved)"
if (Splayer.uper_Tsufu_Learned)
statData.suffix="[src.suffix.] (Super Tuffle)" // [sic]?

return statData

obj/SFocus/Energy/DisplayStat()
var/obj/statData = new()
statData.suffix = "(Multiplier)"

if (player.BaseMaxKi > (player.EnergyHardCap * player.KiMod))
statData.suffix="[round(player.Ki)] \[[player.KiMod]x\] [player.BaseMaxKi > player.EnergySoftCap*KiMod? "(Hard Capped)":""]"
else if (player.BaseMaxKi > (player.EnergySoftCap * player.KiMod))
statData.suffix="[round(player.Ki)] \[[player.KiMod]x\] [player.BaseMaxKi > player.EnergySoftCap*KiMod? "(Soft Capped)":""]"
else
statData.suffix="[round(player.Ki)] \[[Kplayer.iMod]x\]"

return statData

obj/SFocus/Strength/DisplayStat()
return src

obj/SFocus/Endurance/DisplayStat()
return src

obj/SFocus/Force/DisplayStat()
return src

// etc...


// Then when you're doing the stat() calls, all you'd need to do is:
mob/var/list/SFocusList = list()

for (var/obj/SFocus/focus in SFocusList)
stat(focus.DisplayStat())


This way, all the logic and all the code related to your SFocus objects are right next to where your SFocus objects are defined.

Last, but not least, LummoxJR has correctly pointed out that we'd still be doing a bunch of redundant calculation each time.


One other thing that's slow here is that you're setting the suffix for these items each time during the loop instead of doing it when the actual var controlling it is turned on or off. It's pointless calculation and assignment that's resulting in a lot of appearance lookups that aren't needed.


So, instead of creating a "statData" object every time through a DisplayStat() proc, we could instead create such a "statData" object just once, and we set that object's suffix only when it's needed:

// Assuming that /obj/SFocus types have some sort of reference to their player:

obj/SFocus/proc/UpdateStat()
throw EXCEPTION("Not implemented yet.")

obj/SFocus/Battle_Power/UpdateStat()
statData.suffix = "(Multiplier)"

if (player.FBMAchieved == 1)
statData.suffix="(Multiplier) (FBM Achieved)"
else if (player.FBMAchieved == 2)
statData.suffix="(Multiplier) (Ascension Achieved)"
if (Splayer.uper_Tsufu_Learned)
statData.suffix="[src.suffix.] (Super Tuffle)" // [sic]?

obj/SFocus/Energy/UpdateStat()
statData.suffix = "(Multiplier)"

if (player.BaseMaxKi > (player.EnergyHardCap * player.KiMod))
statData.suffix="[round(Ki)] \[[player.KiMod]x\] [player.BaseMaxKi > player.EnergySoftCap*KiMod? "(Hard Capped)":""]"
else if (player.BaseMaxKi > (player.EnergySoftCap * player.KiMod))
statData.suffix="[round(Ki)] \[[player.KiMod]x\] [player.BaseMaxKi > player.EnergySoftCap*KiMod? "(Soft Capped)":""]"
else
statData.suffix="[round(Ki)] \[[player.KiMod]x\]"

obj/SFocus/Strength/UpdateStat()
return src

obj/SFocus/Endurance/UpdateStat()
return src

obj/SFocus/Force/UpdateStat()
return src

// etc...


mob/var/list/SFocusList = list()

obj/SFocus/var/atom/statData = new()

// And when you're doing the Stat() calls,

for (var/obj/SFocus/focus in SFocusList)
stat(focus.statData)


Just remember to call UpdateStatData() only when there is a significant change in your SFocus object.

Let us know what you think and if you have any questions.