ID:1134329
 
(See the best response by Fushimi.)
Code:
mob/npc
proc
AI()
set background=1
var/allset=0
for(var/mob/D in world)
if(D.z==src.z)
if(D.client)
var/ax=src.x-25
var/dx=src.x+25
var/ay=src.y-25
var/dy=src.y+25
if(D.x>=ax&&D.x<=dx&&D.y>=ay&&D.y<=dy)
allset=1
if(D in oview(src))
allset=2
if(allset)
for(var/mob/m in oview(16,src))
if(m.missiontarget==src&&m.mission==src)
walk_away(src,m,8,m.rundelay+1)
if(m.missiontarget==src&&m.mission==4)
if(src.follow==m)
src.Approach(m)
if(!src.combat2)
src.combat2=pick("Rock","Paper","Scissors")
var/didsomething=0
if(allset==2)
if(!src.aggrod)
var/d=rand(1,100)
if(d==1)
var/t=rand(1,42)
src.Speech(t)
if(src.wander)
step_rand(src)
didsomething=1
sleep(rand(20,40))
if(src.hostile&&allset==2)
src.npcHostile()
didsomething=1
if(!didsomething)
sleep(rand(40,60))
if(src.health>0)
spawn()src.AI()
else
sleep(rand(140,160))
if(!src.dead)
spawn()src.AI()


Problem description:

This has been causing lag(I think). It takes up most CPU and has been using more realtime than possible. Within 13 minutes it has an 8 digit number as real time. BTW all enemies call AI proc on New(). Is there a more efficient way to code that.(would a world proc help?)
Well, of course you are having your mobs have an active AI() in unneeded cases.

You would want to only activate that AI() proc when a client is within view, otherwise you are just wasting resources without any logical reason.
If you had only a few of those mobs it wouldn't probably be such a big deal, while in the other hand, with quite a few mobs it will get more and more intense.
I have over 200 mobs and I like this AI system because they walk around, chat, run, and attack on sight. Is there a way to have all these while lowering lag?
Best response
Instead of calling AI() on mob/New(), call it via player movement.

mob/Move()
for(var/mob/NPC/N in oview(src))
if(!N.Activated)N.AI()
..()


This is an example. You would add a variable 'Activated' to your NPCs, and set it to 1 inside AI().
In your AI, you must also do a check so if there are no players, kill the AI() proc.
Perfect! Thanks alot
I'm telling this for future reference.

Recursive spawning of your AI proc is bad. It creates an infinite stack. Use a regular for/while loop.

Also, you're looping though every mob in world. That's not good. If you are only interested for all mobs who have clients, loop though clients instead. Then, check if the client's mob falls under your criteria. Though, you shouldn't be doing this loop in the first place for the reasons Fushimi stated earlier.

It is a good idea to put npcs on standby then activate them when you move near them. And when the npcs no longer detect a player nearby, put them again on standby.

You may optimize this further. Still, you can't have it all. If you want a super fast yet complicated ai that always run on background, you better get a super computer. If you can't, don't do unnecessary stuff.

Humankind cannot gain anything without first giving something in return. To obtain, something of equal value must be lost. That is alchemy's First Law of Equivalent Exchange.
I hate it when it comes down to it. I have to choose between gameplay or lag. I'll go with gameplay since I can reboot before lag gets that bad.
It is your choice but, let me advise that if you ignore this issue it will become worst with the time.
And it will get so worse that not even Dan would be able to fix it.

You will learn a lot of bad habits, that I would say you have already been taught about.
Throw away the unnecessary stuff and keep what you need.

Also, don't forget to think from a player's perspective. For example: Are npcs, miles away from me and randomly stepping, part of my gameplay? The answer is no. That is why the standby/wakeup system was suggested.
In response to Fushimi
Fushimi wrote:
> mob/Move()
> for(var/mob/NPC/N in oview(src))
> if(!N.Activated)N.AI()
> ..()
>


This seems like a bad way to activate your AI. Maybe it would be okay with one player. But I imagine that 20 players running around would cause a lot of lag since this code loops through every tile in view?

Then again, I'm having a hard time coming up with an alternative. So maybe that's what you will have to settle for :/
Lesson 19b of Forum_accounts AI tutorial ( http://www.byond.com/developer/Forum_account/EnemyAI ):

mob
var
team = 0
proc
ai()
hit_by(mob/m)
attack(mob/target)
show_hit(target)
target.hit_by(src)

player
Login()
. = ..()
team = TEAM_1
icon_state = "green"

Move()
. = ..()
// The player's view size is 10, so if we wake all mobs within
// 13 tiles we'll wake some mobs just outside our range of view.
// This way, when they do come into view they're already active.
for(var/mob/ai/m in range(13,src))
if(m.sleeping)
m.sleeping = 0
sleeping_mobs -= 1
spawn() m.ai()


There really isn't a faster way to do this, the alternative is to keep a list of mobs that need AI control, and then looping through each one in the list and doing a distance check on them, and that as an alternative is probably actually slower.
In response to Jemai1
Jemai1 wrote:
I'm telling this for future reference.

Recursive spawning of your AI proc is bad. It creates an infinite stack. Use a regular for/while loop.

About the call stack issue...

From Lesson 09b in Forum_account's ( http://www.byond.com/developer/Forum_account/EnemyAI ):

/*

The mob's ai proc is a loop. It doesn't use for or while, it creates
a loop by calling the function over again. Here is the ai proc from
the first example, code-01.dm:

ai()
step_rand(src)
spawn(10) ai()

What does spawn(10) do and why is it necessary?

the spawn proc delays execution of code but lets the current proc
continue its execution. Here's an example:

spawn(10)
world << "A"
world << "B"

The execution of world << "A" is delayed by one second, so this program
would output "B" then "A".

What would happen if we changed our ai proc to this:

ai()
step_rand(src)
sleep(10)
ai()

This appears to do the same thing. The ai() proc is called again one
second after the call to step_rand. This seems correct, but you'll run
into trouble. It'll take some explaining to understand why.

The call stack is a list of function calls. When you call a function,
the function is added to the front of the list. When that function ends
(when its end is reached or when you call return) that function is removed
from the list. The function that is now at the front of the list is what
we return to. For example:

turf
Click()
a()

proc
a()
world << "A"
b()

b()
world << "B"

If we run this program and click on a turf, here's how the call stack
will change over time:

Action Call Stack
user clicks on a turf /turf/proc/Click
Click calls a /turf/proc/Click, /proc/a
a calls b /turf/proc/Click, /proc/a, /proc/a
b returns /turf/proc/Click, /proc/a
a returns /turf/proc/Click
Click returns (empty)

Let's see how the call stack behaves in our last ai proc:

ai()
step_rand(src)
sleep(10)
ai()

ai is called from mob.New /mob/ai/proc/New, /mob/ai/proc/ai
ai calls ai /mob/ai/proc/New, /mob/ai/proc/ai, /mob/ai/proc/ai
ai calls ai /mob/ai/proc/New, /mob/ai/proc/ai, /mob/ai/proc/ai, /mob/ai/proc/ai
ai calls ai /mob/ai/proc/New, /mob/ai/proc/ai, /mob/ai/proc/ai, /mob/ai/proc/ai, /mob/ai/proc/ai
ai calls ai /mob/ai/proc/New, /mob/ai/proc/ai, /mob/ai/proc/ai, /mob/ai/proc/ai, /mob/ai/proc/ai, /mob/ai/proc/ai

The ai proc never returns! Because of this, the call stack grows and
grows. The call stack is stored in memory, and because you have a
limited amount of memory you'll eventually run out.

Ok, that way of creating the AI loop is out. Let's look at another popular
method:

ai()
while(1)
step_rand(src)
sleep(10)

This looks better. The proc doesn't call itself so we shouldn't run
into the call stack problem -- or will we?

Let's take a look at the state machine from code-09a.dm. In particular,
look at the indicated line.

ai()
if(dead) return

if(state == HAPPY)
get_target()

if(target)
if(health < 5)
// Decide if the mob is going to stay and fight or
// run away. Announce the decision to the world,
// even though it should be clear from its behavior.
if(prob(50))
world << "[name] decides to stay and fight."
state = FIGHT
else
world << "[name] decides to run away."
state = RUN_AWAY
Look at this line ----> return ai()
...
else
step_rand(src)
sleep(20)
else if(state == FIGHT) ...
else if(state == RUN_AWAY) ...
spawn() ai()

When the mob is in the HAPPY state and is below 5 health it decides
if it should transition to FIGHT or RUN_AWAY. Once it makes a transition
we call "return ai()". Calling return will exit this current instance of
the ai loop, but first it calls ai() again. Because the mob changed
states we want to execute another ai cycle immediately so that the mob
will perform an action that is appropriate for its new state.

This code will not have the call stack problem because every call to
the ai proc terminates. A call either reaches the end, spawns the next
call, then ends. Or, the call reaches a "return ai()" line, which returns
after the recursive call ends.

If we didn't want to use "return ai()", the alternative would be
something like this:

if(state == HAPPY)
get_target()

if(target)
if(health < 5)
if(prob(50))
world << "[name] decides to stay and fight."
state = FIGHT
// fight code
else
world << "[name] decides to run away."
state = RUN_AWAY
// run away code
...
else
step_rand(src)
sleep(20)
else if(state == FIGHT)
// fight code
else if(state == RUN_AWAY)
// run away code
spawn() ai()

We'd have to repeat the code for fighting or running away within the
block that handles the HAPPY state. We'll go into more detail in code-10.dm,
but repeating code like this is a bad idea.

If we had used a while loop for our ai proc, like this:

ai()
while(1)
if(dead) return
if(state == HAPPY)
...
return ai()
...
else if(state == FIGHT)
...
else if(state == RUN_AWAY)
...
sleep(10)

We could still run into the call stack problem. When the mob transitions
to a new state and calls "return ai()" it adds a function to the call
stack and this new function will never end (so it will never be removed
from the call stack). It may not happen often enough that we'll run out
of memory, but it will still take up an unnecessary amount of memory.

Instead we can use something you may have never heard of: loop labels.

ai()
ai_loop:
while(1)
if(dead) return
if(state == HAPPY)
...
continue ai_loop
...
else if(state == FIGHT)
...
else if(state == RUN_AWAY)
...
sleep(10)

The "ai_loop:" before the while loop is a loop label. To assign the
while loop to that label we indent the while loop. When inside a loop,
the continue command aborts the current iteration of the loop and
starts the next iteration. If you don't specify a label it will start
the next iteration of the innermost loop. We need to use a label
because our code might look like this someday:

ai()
ai_loop:
while(1)
if(dead) return
if(state == HAPPY)
...
for(var/i = 1 to 10)
...
continue ai_loop
...
else if(state == FIGHT)
...
else if(state == RUN_AWAY)
...
sleep(10)

The continue statement is inside of two loops (the while and the for). Just
saying "continue" would start the next iteration of the for loop. In this
case we have to use a label.

Loop labels are strange and require you to indent your proc by another tab,
With the number of if statements you'll end up having inside your ai proc
you don't need more things contributing to indentation.

*/
> mob/Move()
> for(var/mob/NPC/N in oview(src))
> if(!N.Activated)
> N.Activated=1
> spawn()N.AI()
> ..()

        AI()
set background=1
var/allset=0
//for(var/mob/P in oview(10))
// if(P)
for(var/mob/D in world)
if(D.z==src.z)
if(D.client)
var/ax=src.x-16
var/dx=src.x+16
var/ay=src.y-16
var/dy=src.y+16
if(D.x>=ax&&D.x<=dx&&D.y>=ay&&D.y<=dy)
allset=1
if(D in oview(src))
allset=2
if(allset)
for(var/mob/m in oview(16,src))
if(m.missiontarget==src&&m.mission==src)
walk_away(src,m,8,m.rundelay+1)
if(m.missiontarget==src&&m.mission==4)
if(src.follow==m)
src.Approach(m)
if(!src.combat2)
src.combat2=pick("Rock","Paper","Scissors")
var/didsomething=0
if(allset==2)
if(!src.aggrod)
var/d=rand(1,100)
if(d==1)
var/t=rand(1,42)
src.Speech(t)
if(src.hostile&&allset==2)
src.npcHostile()
didsomething=1
if(src.wander)
didsomething=1
step_rand(src)
sleep(30)
if(!didsomething)return
//sleep(rand(40,60))
if(!src.dead)
spawn()src.AI()
else
src.activated=0
return

With these codes AI() is activated once players are close by and automatically shut down when they aren't so I think CPU will be lower. Correct me if I'm wrong.
You are very wrong, because of this:

for(var/mob/D in world)
//...
for(var/mob/m in oview(16,src))


These types of for loops should be avoided as much as possible. It might be okay to do it when the player moves around - but even then with too many players it will lag out the game. However, having 200 mobs looping through every mob in the world (so, all 200 NPCs, all players) and also looping through all mobs in view...this is gonna grind your game to a halt I suspect
Also this is not a great way to do it:

var/ax=src.x-16
var/dx=src.x+16
var/ay=src.y-16
var/dy=src.y+16
if(D.x>=ax&&D.x<=dx&&D.y>=ay&&D.y<=dy)
allset=1
if(D in oview(src))
allset=2


Not that the number crunching will necessarily cause noticable lag, but it would be a lot more efficient to just calculate a distance:

var/dx = abs(x - D.x)
var/dy = abs(y - D.y)
if((dx+dy) < 32))
allset = 1

//or...

if(get_dist(src,D) < 16)
allset = 1