ID:1496370
 
(See the best response by Ter13.)
I was told to refer to the profiler to see what is causing my game to lag so horribly even with a small amount of players. What should I be looking for in the profiler? Should I be paying more attention to the # of calls, self-CPU, total-CPU, or real time?

Moderator Note: This thread is being recovered manually.
Self CPU refers to how long the function took to run while processing.

Total CPU refers to how long the function and all functions called from that function took to run while processing.

Realtime shows how long a function has taken so far, from start to finish or now. If you sleep() during a function call, the time during that sleep will not contribute to self or total CPU, but will contribute to realtime.


In order to effectively use the profiler, you want to sort by Total and Self CPU in order to determine what's eating the most processing power. This will give you an idea of what is the most intense part of your game's code, and give you a starting point to finding improvements.

Take a look at the averaged times as well. If you are seeing a very large number of calls to something that is eating up a ton of CPU, you might try to optimize it by calling it less often. Or, if the average time is high, you might think about trying to make it faster by improving the methodology of how you solve the same problem via a new approach.

Now, as for realtime... If the realtime is high, but the self CPU and total CPU are low, it doesn't really matter, because you probably have some kind of a long-running delay going on in that function. As long as the self CPU and total CPU are fairly low, you should be alright.

For a proc that is an infinite loop, though, it can be really hard for people to understand what constitutes "low" and "high" CPU usage.

You need to think about how often, and when that code is running. Sure, the self CPU might be 5.147 seconds, but if the function is running one time during startup, before the players even connect, it's not going to be as high of a priority to optimize as something that takes 0.522 seconds and runs once every 10 seconds while players are trying to play your game.


Now, realtime and infinite loops can throw it off too.

In order to understand how much of the CPU time a function is using that is an infinite loop, you need to divide the Total or Self CPU (averaged) by the Realtime. So if the averaged Real Time of a function is 647.255, and the average total CPU of a function is 30.174, this may look like it's eating up a lot of CPU time, but we divide the two values: and we get approximately 4.66% of the CPU time being used by this function. That's not low, but it's not really all that high either.

Any further questions?
Each time I refresh the profiler, it increases everything. Should I just be using the information when I first open the profiler without refreshing it?
Refreshing it will update the profiler with more data on what's been running since the last refresh. This allows you to get better averages and more information. You just need to be aware how long your world has been running (using the timer on the network profiler is a good way to get that information) in order to interpret the results correctly.

Averages can be helpful for getting a more full picture of what's going on with a function, but it can also mask problems if you have weird corner cases where a function usually only takes a few milliseconds to finish, but has a very infrequent case where it takes a really long time to finish.

Overall, my best piece of advice is to use the profiler as a tool to help you discover more about programming, and not just rely on the profiler to tell you what's good and what's not. Use the profiler to help you focus on specific pieces of code so that you can learn what approaches are faster than others, and how to eliminate redundant code and wasteful methodologies.

Doing it right the first time is a lot easier, but doing it wrong helps teach you where you need to improve.
If I post a screen shot of the profiler data, would you be able to spot some problems? Or is this more of a self-done job? I'm not really understanding how to reduce lag and it's getting quite frustrating.
Sure, go right ahead.

Show me one screenshot of the profiler, sorted highest-to-lowest Self CPU, not averaged, and one screenshot of the profiler sorted highest-to-lowest Self CPU averaged. Also be sure to tell me how long the world has been running at the time of the last refresh so I can have all the needed data available to help you identify where things are slow.

Note: Both images should be taken from the same refresh, and the lowest time from the two screenshots will be used for gathering our data.

Please ensure that your first screenshot is taken within seconds after your last refresh. This will allow us to get better, more accurate results.



Of course, I won't be able to tell you a lot without further input from you, and a better understanding of what certain functions do, so I'll try to help walk you through the thought process of finding parts of your code to optimize. Sound fair?
54 seconds with nothing going on isn't going to tell me anything.

Based on your prior posts, /mob/player/Move is the biggest offender. Looking at your prior averages, /mob/player/Move() was taking up over 61.5% of your available CPU time.

Each call to Move() takes approximately 0.0201 seconds to process, and thus, if you have 50 people moving in one frame, 100% of your CPU budget is going to be used completing those movements, leaving no room for anything else.

Your NPC AI is also taking up a lot more CPU time than it probably should, with your AI eating 2.6 seconds of CPU time over 3.4 seconds of activation. This means that your AI is eating 76.4% of your CPU time.

If your AI is active while players are moving, those two procs will eat 100% of your CPU time with just under 5 players online moving at once.

These two things are the primary source of your CPU problems. Address them first.
How would I go about fixing movement? I don't know what's wrong with it?

client/North:

client
North()
if(mob.meditating)
var/T=0
for(var/obj/chakraarrow/M in oview(mob,1))if(M.dir==NORTH)T=1
if(T==0)
mob.cha-=mob.maxcha/10
if(mob.cha<0)mob.cha=0
mob.breakmed()
else
if(prob(18))mob.gainexp()
for(var/obj/chakraarrow/M in oview(mob,1))M.dir=pick(SOUTH,EAST,WEST)
mob.cha-=pick(0,0,0,0,0,1,1,2)
if(mob.cha<=0)
mob.cha=0
mob.breakmed()
mob.following=0
return
mob.following=0
if(mob.spectating)
mob.spectating=0
mob.client.perspective=MOB_PERSPECTIVE
mob.client.eye=mob.client.mob
if(mob.busy=="coffin")for(var/obj/coffinmove/M in oview(12))if(M.owner==mob)if(M.dir!=SOUTH)M.dir=NORTH
if(mob.isviewing)for(var/obj/blacklightninghound/M in world)if(M.owner==mob)
M.dir=NORTH
walk(M,M.dir)
if(mob.freeze&&mob.shadow)for(var/obj/imitation/M in oview(8))if(M.owner==mob)if(M.dir!=SOUTH)if(M.icon_state!="struggle")if(M.turns<2)
M.dir=NORTH
M.turns++
if(mob.freeze&&mob.shadow)for(var/obj/paralysis/M in oview(8))if(M.owner==mob)if(M.dir!=SOUTH)if(M.icon_state!="struggle")if(M.turns<2)
M.dir=NORTH
M.turns++
if(mob.freeze&&mob.sewing)for(var/obj/shadowsewing/M in oview(14))if(M.owner==mob)if(M.dir!=SOUTH)if(M.icon_state!="struggle")if(M.turns<2)
M.dir=NORTH
M.turns++
if(mob.caught||mob.buttonjam||mob.addled||mob.freeze||mob.imitated||mob.paralysed)return
if(mob.imitation)for(var/mob/M in world)if(mob.imitation==M)
if(mob.moving||mob.resting||mob.freeze||M.freeze)return
M.moving=0
for(var/obj/imitationt/T in world)if(T.owner==mob)step(T,NORTH)
step(mob,NORTH)
// for(var/obj/explosivebubbles/J in world)if(J.owner==mob)step(J,NORTH)
step(M,NORTH)
mob.dir=mob.shadowdir
M.dir=M.shadowdir
else
if(mob.confused)
mob.dir=SOUTH
if(mob.cloned)for(var/mob/Clone/clone/M in oview(4))if(M.cloneowner==mob)step(M,SOUTH)
// for(var/obj/explosivebubbles/J in world)if(J.owner==mob)step(J,SOUTH)
step(mob,SOUTH)
else
if(mob.spidering)
var/T=mob.dir
..()
mob.dir=T
return
mob.dir=NORTH
if(mob.cloned)for(var/mob/Clone/clone/M in oview(4))if(M.cloneowner==mob)step(M,NORTH)
// for(var/obj/explosivebubbles/J in world)if(J.owner==mob)step(J,NORTH)
..()


mob/player/Move():

mob/player/Move()
if(paralysed||dead||resting||shield||sandshield||freeze||ingenin==2||name=="Tree"||!fast&&moving)return
// winset(usr, "default.label44", "text=\"Current Coordinates: ([usr.x],[usr.y],[usr.z])\"")
var/fasting=0
if(fast)fasting=1
if(!fasting)moving=1
if(cloaking)
if(fullbody!=/obj/stealthsuit&&skillhidemast==0)
var/T=new/obj/cloakshadow(loc)
T:dir=dir
..()
if(clan=="Hyuuga"&&busy=="webbed2")weboff()
if(barrierhold)
icon_state=""
barrierhold=0
for(var/obj/collision/C in oview(2))if(C.owner==src)C.loc=loc
for(var/obj/detonatingkunaicollision/C in oview(1))C.loc=loc
for(var/obj/bars/C in oview(1))if(C.barowner==src)C.loc=loc
for(var/obj/spiderbow/M in world)if(M.owner==src)
M.dir=dir
M.loc=loc
for(var/obj/explosivebubbles/J in world)if(J.owner==src)step(J,dir)
var/D=0
for(var/obj/sandwave/C in loc)if(C.owner!=src&&C.owner:village!=src.village)
sandwave(C.owner)
D=1
if(D==0)
if(coffined==2)
pixel_y=0
overlays-=/obj/burybegin
overlay()
layer=3
sandwave=0
overlays-=/obj/burybegin
var/A=0
for(var/obj/slimefield/C in loc)if(C.owner!=src&&C.owner:village!=src.village&&!src.slipped)
slimedd(C.owner)
A=1
if(A==0&&slimed)slimed=0
delay()
runstep++
if(runstep>=4)if(!running&&!runningt&&client)
if(onwater&&fullbody!=/obj/mistdemonsuit)goto norun
if(z==2&&x>=102&&x<=202&&y<=274&&y>=2&&village!="Sand"&&headgear!=/obj/clothmask&&y>=2)goto norun
if(slowdownpassive == 1)goto norun
if(itachisusanoo == 1)goto norun
if(sasukesusanoo == 1)goto norun
if(madarasusanoo == 1)goto norun
if(firingbow == 1)goto norun
if(!finalized)goto norun
runstep=5
running=1
icon_state="run"
if(fullbody==/obj/stealthsuit)invisibility+=2
norun
if(paperwings)runstep=6
if(runstep>6)runstep=6
if(running)
if(fullbody==/obj/stealthsuit)
var/T=new/obj/runshadow(loc)
T:dir=dir
var/T=60-vit
if(T<1)T=1
if(energy>=35)
energy-=T/rand(9,16)+rand(0,2)
if(energy<0)energy=0
var/tank=0
for(var/obj/items/Full_Water_Container/M in src)tank=1
if(bubble)bubble:loc=loc
if(energy<=10||tank)
runstep=0
if(running)
running=0
icon_state=""
if(fullbody==/obj/stealthsuit)invisibility-=2
if(icon_state=="swim")
var/c=(100-vit)/10
if(c<1)c=1
energy-=rand(1,c)
if(energy<0)deathdrown()
if(firingbow)icon_state="spear"
if(paperwings||slipped)icon_state="paperwings"
if(!fasting)spawn(delay)moving=0
if(onwater)
var/obj/haku_ice/ice = locate(/obj/haku_ice) in loc
if(!ice)
if(clan == "Yuki")
new /obj/haku_ice(loc)
First things first, almost none of that code belongs in client/North.

Second, you need to start separating logic out that acts before the player moves, and when the player moves successfully.

Third, if you are searching through the world for objects that belong to the player, you are probably doing it wrong.

Fourth, if you are using Move() to trigger events on objects that are in the same tile as the player? Probably wrong.

Most of what I see here, is a LOT of logic that doesn't need to be embedded in the player's movement procedure, and is a consequence of working on stolen source code. This is going to take a long, long time for you to fix, and a LOT of learning.

Moderator note: Thread has been restored. Please carry on as though this thread was never deleted.
Okay, like I said before this thread got deleted: This is going to take a long, long time to fix, but here are some pointers. There's another post or two I'm going to make that will also help, but it's going to take some time.

Okay, let's spend a few minutes talking about Move().

Move() returns 1 if successful, and 0 if not successful.

Some of your logic needs to be performed before the Move(), and some needs to be performed after the Move() completes.

Let's implement a really simple way to do that.

atom/movable
Move(atom/NewLoc,Dir=0,step_x=0,step_y=0)
var/OldLoc = src.loc
var/odir = src.dir
var/osx = src.step_x
var/osy = src.step_y
. = ..()
if(.)
Moved(OldLoc,odir,osx,osy)
proc
Moved(atom/OldLoc,oDir=0,ostep_x=0,ostep_y=0)


What the above does, is define a stub method called Moved(), which is called after any successful move.

We are going to ONLY include things in your Move() proc that will actually prevent the movement from happening, or occur regardless of whether the move was successful.

We are going to use Move() ONLY for logic that should trigger only when a movement was successful. This should help us encapsulate logic in the right places.


First, let's look at a few trends I see in your code.

Just about everything in client/North/East/West/South needs to be moved out of there, and into mob.Move().

I notice a lot of stuff like this:

if(paralysed||dead||resting||shield||sandshield||freeze||ingenin==2||name=="Tree"||!fast&&moving)return


as well as:

if(mob.meditating)


I seriously think it would be much better/easier to implement a mob variable called nomove. If nomove is greater than zero, the mob won't be able to move. That way, everything, like being dead, being paralyzed, etc. will add 1 to this variable, and subtract one when it no longer applies.

mob
var
nomove = 0
Move()
if(nomove)
return 0
. = ..()


Alright, so now, let's talk about stuff like this:

for(var/obj/spiderbow/M in world)if(M.owner==src)
M.dir=dir
M.loc=loc


for(var/obj/collision/C in oview(2))if(C.owner==src)C.loc=loc


for(var/obj/slimefield/C in loc)if(C.owner!=src&&C.owner:village!=src.village&&!src.slipped)
slimedd(C.owner)


if(mob.isviewing)for(var/obj/blacklightninghound/M in world)if(M.owner==mob)
M.dir=NORTH
walk(M,M.dir)


Let's talk about how we're going to make this a little bit more efficient, and teach you a bit about datums and polymorphism in the process.

Instead of searching through the world, we're going to tell the player a bit about these objects ahead of time, allowing the player to ONLY do what it actually needs to, and not spend any time at all looking for the right objects.

In order to do this, we're going to implement what's called a Listener datum, which we will use as a proxy between the player and objects that have behavior that triggers when the player moves.

Let's take a look at this datum really fast:

#define TRIGGER_MOVE 1
#define TRIGGER_MOVED 2

move_listener
var/tmp
list/triggers = list()
move_mode = 0
//move_mode is either zero or 1.
//if the move is already canceled, the listener function will do nothing by default.
//if the event needs to happen regardless of whether the move has already been canceled
//set move_move to 1
proc
//this event will help us do things when the movable tries to move.
Move(atom/movable/source,atom/NewLoc,Dir,step_x,step_y,pending_move=1)
if(retval || move_mode)
return 1
else
return 0

//this event will help us do things after the movable has moved.
Moved(atom/movable/source,atom/OldLoc,oDir,ostep_x,ostep_y)

//this event will help us figure out when the movable has been deleted.
Deleted(atom/movable/trigger)
//unregister src from the movable
if(trigger.move_listeners)
trigger.move_listeners.Remove(src)

if(trigger.moved_listeners)
trigger.moved_listeners.Remove(src)

//remove trigger from triggers list
triggers.Remove(trigger)

//tell the trigger that it's being listened to for Move
Listen_Move(atom/movable/trigger)
if(!(trigger in triggers))
triggers.Add(trigger)

trigger.Listener_Added(TRIGGER_MOVE,src)

//tell the trigger that it's being listened to for Moved
Listen_Moved(atom/movable/trigger)
if(!(trigger in triggers))
triggers.Add(trigger)

trigger.ListenerAdded(TRIGGER_MOVED,src)

//tell the trigger that it's being listened to for Move & Moved
Listen(atom/movable/trigger)
Listen_Move(trigger)
Listen_Moved(trigger)


Okay, so this little magic bullet will allow us to attach behavior to things that we can dynamically keep an eye on, and keep us from doing all those horrible searches.

Let's set up movable objects to be able to use these things:

atom/movable
var/tmp
list/move_listeners
list/moved_listeners
Del()
//tell the listener that this mob has been deleted.
if(move_listeners)
for(var/move_listener/listener in src.move_listeners)
listener.Deleted(src)
if(moved_listeners)
for(var/move_listener/listener in src.moved_listeners)
listener.Deleted(src)
..()

//this is used to register the listeners
ListenerAdded(listener_type,listener)
switch(listener_type)
if(TRIGGER_MOVE)
//ensure the list exists, if not, create it.
if(!move_listeners)
move_listeners = list()

move_listeners.Add(listener)

if(TRIGGER_MOVED)
//ensure the list exists, if not, create it.
if(!moved_listeners)
moved_listeners = list()

moved_listeners.Add(listener)



Now that we have all of that out of the way, we need to make the listeners actually work.

atom/movable
Move(atom/NewLoc,Dir=0,step_x=0,step_y=0,override=0)
//we're changing the nomove concept a bit from above.
if(nomove)
. = 0

//we need to ensure that we have things listening for movements
//override keeps infinite loops from happening when you call Move/step from a move_listener
if(move_listeners && !override)
//run through each listener, and tell it we tried to move.
for(var/move_listener/listener in move_listeners)
. = listener.Move(src,NewLoc,Dir,step_x,step_y,.)

//if the pending return value is not zero, do the default action.
if(.)
//we've relocated the old location stuff down here now
var/OldLoc = src.loc
var/oDir = src.dir
var/osx = src.step_x
var/osy = src.step_y
. = ..()
if(.)
Moved(OldLoc,oDir,osx,osy)

//return the value of . by default

proc
Moved(atom/OldLoc,oDir,ostep_x,ostep_y)
//ensure that we have things listening for successful movements
if(moved_listeners)
//run through each listener and tell it we moved successfully
for(var/move_listener/listener in moved_listeners)
listener.Moved(src,OldLoc,oDir,ostep_x,ostep_y)


Alright, all of this is leading toward an actual answer to your problem. We now have the ability to stop searching the world every single time something moves, and start tying behavior directly together through event detection.

In the next post, I'm going to help you to use the move_listener to actually replace a few of the world searches you are doing, so keep an eye out for a bit. Setting things up right takes a while, and writing good code takes a lot longer than writing ten times as much bad code, so please be patient.
atom/movable
Move(atom/NewLoc,Dir=0,step_x=0,step_y=0)
var/OldLoc = src.loc
var/odir = src.dir
var/osx = src.step_x
var/osy = src.step_y
. = ..()
if(.)
Moved(OldLoc,odir,osx,osy)
proc
Moved(atom/OldLoc,oDir=0,ostep_x=0,ostep_y=0)

Can this be directly added to the Move()? Or is this a brand new parent
Best response
Okay, so we left off talking about fixing these:

for(var/obj/spiderbow/M in world)if(M.owner==src)
M.dir=dir
M.loc=loc


Now, you mentioned in PM to me that a spiderbow is a skill that you toggle that gives you an overlay, and when you press F, it fires the spiderbow. There's no reason to have this as a distinct object following the player. Just add it as an overlay.

We also determined via PM that your explosivebubbles skill was probably best handled as a series of overlays, as were your health bars and mana bars, rather than as distinct objects. The collision objects as well, you explained as being knives/shurikens stuck in the mob, which will also make them eligible to be overlays.

So, let's look at getting some of this logic out of Move().

You explained /obj/sandwave like this: The attack fires a wave of sand, and if an enemy steps in it 3 or more times, they are frozen.

It would be more useful to do something like this, however, so that we don't have to look for sandwave objects every time something moves:

obj/sandwave
var/tmp
mob/owner

Crossed(atom/movable/O)
if(ismob(O)) //ensure that the object stepping on this object is a mob
var/mob/m = O
m.sandwave_step(owner) //tell the mob they just stepped on a sandwave

Uncrossed(atom/movable/O)
if(ismob(O)) //ensure that the object stepping off this object is a mob
var/mob/m = O
if(!locate(/obj/sandwave) in m.loc)
m.sandwave_clear() //tell the mob that they just stepped on a square that had no sandwave on it.


The above code will make sure that the sandwave behavior is tied entirely to the sandwave objects, thus making your move function just a little bit faster.

Now, using this same approach, you might notice we can also move slimefield logic out of the Move proc the same way:

obj/slimefield
var/tmp
mob/owner

Crossed(atom/movable/O)
if(ismob(O))
var/mob/m = O
m.slime_step(owner)

Uncrossed(atom/movable/O)
if(ismob(O))
var/mob/m = O
if(!locate(/obj/slimefield) in m.loc)
m.slime_clear()


Alright, now, the last example I have the energy for tonight:

Those cloaking shadows and ice trails left in the water are great candidates for things that should be done with those move_listeners I set up.

Let's learn how to use those real fast:

move_listener
haku_ice
Moved(mob/trigger,atom/OldLoc,oDir,ostep_x,ostep_y)
if(trigger.onwater)
if(!locate(/obj/haku_ice) in trigger.loc)
new/obj/haku_ice(trigger.loc)


Now we need to set up the haku_ice listener to be able to be added to the mob's listeners.

var/move_listener/haku_ice/haku_ice = new()


We're just going to set up a global object, since we really only need one of these listeners. It can function for all members of clan yuki.

Now, all you need to do, is whenever the player logs in, loads from a savefile, or joins clan yuki, just:

haku_ice.Listen_Moved(mob)


Of course "mob" should be whatever the player or NPC should use the haku_ice behavior.


I don't have the energy to correct much else wrong with your approach, but this should move quite a lot of stuff out of your Move() proc, alongside the polymorphism stuff I taught you in PM, moving things over to overlays rather than being discrete objects, as well as learning how to use typecasting to your advantage, and embedding behavior in the right places.

You should have more than enough work ahead of you to keep you busy for a few days.

If you need more assistance, feel free to pop back in.