ID:2068205
 
(See the best response by Ter13.)
Basically I want to damage a mob within view of 1 from an object, and continue to damage them until they leave.
I created a loop that is spawned when the object is created but it is eating up way too much cpu. Is there a more efficient way to do this?
It's also likely that your view() check is an issue. You should post your code.
Can you show the actual code?

Without understanding your current approach, it'd be difficult to give you pointers.
obj/Steam
icon='icons/steam_cloud.dmi'
icon_state="steam"
density=0
layer=MOB_LAYER+1


New()
..()
spawn()Melt()

proc/Melt()

while(src)
for(var/mob/M in view(src,1))
spawn()
if(M && src && src.owner && M!=src.owner)
var/damage = rand(400,1000)
if(M&&src&&src.owner)M.Damage(rand(conmult/2,conmult),0,src.owner)
if(M&&src&&src.owner&&prob(50))M:Wound(rand(0,1),0,src.owner)


sleep(35)
A few things here:

- Using src.thevar instead of just thevar is just a waste of typing, unless you have a local or global var with the same name. Clarify your code by nixing the src.xxxx cases.

- Checking if src is valid is pointless in a proc. If the src object is deleted, the proc will end.

- The value of the damage var that you're calculating isn't being used anywhere.

- The call to Wound() should not be using the : operator. If you've defined that proc for all mobs, then the . operator is fine. And if you haven't defined it for all mobs, that's a runtime error waiting to happen.
Best response
Question: Does this thing ever move?

A cleaner approach would be to create a hitbox that keeps track of the mobs that are currently standing inside of it and iterates over that list every tick.

obj/Steam
bound_width = 3*TILE_WIDTH
bound_height = 3*TILE_HEIGHT
bound_x = -TILE_WIDTH
bound_y = -TILE_HEIGHT

var
list/hurting
melt_time

Crossed(mob/M)
if(istype(M)&&M!=owner)
if(!hurting)
hurting = list(M)
Melt()
else
hurting += M

Uncrossed(mob/M)
if(istype(M)&&hurting)
hurting -= M
if(!hurting.len)
hurting = null

proc
Melt()
if(melt_time!=null) return //this is a loop capture pattern that prevents repeat iterations.
var/time = world.time //world.time starts at zero. Remember that null is not zero. That's why we're not simply checking !melt_time above.
melt_time = time
while(hurting&&melt_time==time)
for(var/mob/M in hurting)
M.Damage() //do your damage business in here
sleep(35)
melt_time = null


Now there are a few gotchas in here.

1) If you are ever setting the loc explicitly of any mobs, this can bug out. You should *always* ensure that Crossed() and Uncrossed() are reliable by never manually setting the location of anything that ever interacts with a Crossed()/Uncrossed() tracker.

2) If you ever manually delete your mobs, this will bug out too. You should never manually delete any mobs that are tracked by Crossed()/Uncrossed() trackers without first finding all objects that are beneath them and calling Uncrossed()/Crossed().

3) BYOND by default has no reciprocal to Crossed()/Uncrossed(). So if you ever move this steam object into position, it will not be informed of any potential targets already standing there. I strongly advise implementing Crossed()/Uncrossed() reciprocals under /atom/movable because it allows you to write cleaner, smarter, better to contain code.

Reference tracking is important, and ensuring that built-in functions are reliable by avoiding bad practices like manually setting locations and manually deleting objects while they are still on the map allows you to use BYOND's built-in function hooks to your advantage in much more elegant ways than simply checking oview() every few frames.