ID:2120711
 
Code:
// returns the closest target within view
NearestTarget()
set background = 1
if(active_targets.len)
var/mob/target = active_targets[active_targets.len]
var/distance = get_dist(src, target)
for(var/mob/T in active_targets)
var/tmp_distance = get_dist(src, T)
if(!T.ko && tmp_distance < distance)
distance = tmp_distance
target = T
if((target in oview(src)) && !target.ko)
return target
return null


Problem description:
Basically eating up too much cpu. How do I make this more efficient?
I'm not sure how you derive your active_targets list, but you could start by removing your final if() check.

   NearestTarget()
// ...
var/list/potential_targets = active_targets & oview(src)
for(...)
// ...

If I'm not mistaken, the above should make potential_targets only contain the list of objects that exist both in the active_targets list and in the oview().

Looping through this list might largely reduce the number of list elements you need to iterate over, depending on how active_targets was originally defined.
In response to Hiead
mob/var/tmp/active_targets[0]
This sounds more like a problem of calling the proc unnecessarily often moreso than the actual cost of it. It would help a lot to keep the targets list as empty as possible as well.
I'm curious if Hieds suggestion help much? I would have to agree with MisterPerson, the code seems pretty efficient. The most work done would probably be the inner for loop where its calling get_dist though that shouldn't cause any abnormal cpu. The only thing I can suggest is possibly changing oview() to oviewers(). If the active_targets list is considerably large which I don't see why it would be, you can try using this type of for loop.
var/length=active_targets.len
for(var/i in 1 to length)
var/mob/T=active_targets[i]
In response to Fat Albert
Just going to add to this, try not to re-declare variables in loops.

var/mob/T=active_targets[1]
for(var/i in 2 to active_targets.len)
T=active_targets[i]

In response to Fat Albert
i in x to y is definitely faster in DM than x, x <= y, x++, but accessing an item from a list via [index] in DM is much slower than just doing a normal for each for(var/thing/T in your_list)

tl;dr the for loop part is faster, but the list element access is much worse, making it worse over all.
In response to CrimsonVision
I thought looping over the index would be faster because it avoids the implicit istype(), kinda like:
// for(var/thing/T in your_list) is like
for(var/_T in your_list)
if(istype(_T, /thing))
// ...
Modified it, this should work.
I don't know how you could get this any faster without building it into the proc and losing the overhead.

proc/nearview(range=0,mob/m)
var mob/M
loop
for(. = 0 to range)
for(M in hearers(range,m))
break loop
return M
I use this to find the closest target and never really noticed it using much cpu but then again im only checking within a 4 tile radius
                var/mob/tempclosest=null
var/mob/closest=null
var/list/f=list()
for(var/mob/m in obounds(128,usr))
if(!m.dead)
f+=m
for(var/mob/m in f)
tempclosest=m
if(get_dist(usr, tempclosest) < get_dist(usr, closest))
closest=tempclosest
return closest


if you are grabbing active targets by using all the mobs ingame then i suspect its using a lot of cpu when ur extremly far from the closest target? or if you have a bunch of targets inside your active targets list that will also eat up a lot.
other than that MisterPerson already mentioned that you may just be calling this proc too often
In response to Kaiochao
You are correct that it's good to avoid the implicit istype()! however, accessing via index is still worse.

if you want a for each that avoids the implicit istype() you can do (what I've taken to calling around /tg/ SS13) an "istypeless for loop":

for(var/t in list)
var/thing/T = t
...stuff with T...

Of course this is dangerous and you can only do it on lists which contain only the type you want to access or subtypes of it, it's a shame the index way isn't faster as then you'd be able to have this in every list loop, but alas as far as I recall the [index] is much slower than how a for each accesses it (so slow it's not worth using)

This is still quite useful for larger games like SS13, like where we have lots of x_list, y_list, z_list, etc_list to avoid world loops.
In response to Kozuma3
Kozuma3 wrote:
Modified it, this should work.
I don't know how you could get this any faster without building it into the proc and losing the overhead.

proc/nearview(range=0,mob/m)
> var mob/M
> loop
> for(. = 0 to range)
> for(M in hearers(range,m))
> break loop
> return M


Just out of curiosity, (I'm not home and cant test), would that get the 'nearest' target, the target at the southwest corner of the range area, or just any random target within the range? I'm not sure how or even if hearers orders its list of mobs.
In response to Flick
if hearers() is like most other Byond Procs, it is bottom left (southwest) biased.

Edit: tested it, it is indeed bottom left biased.
if this is not desirable behaviour one thing you can do is get ALL candidates for nearest (eg: everyone who was 1 turf away) in a list and just pick() them or use some other selection criteria, as opposed to just using the first one you find (biased)
In response to Flick
view() and similar procs do not guarantee an order. Therefore your best bet is to use get_dist() on each item found and compare to the previous best choice, if any.