ID:2075893
 
(See the best response by Popisfizzy.)
I'm using an AI turf with an AI proc and I'm receiving this error in game. Here is the error:

Infinite loop suspected--switching proc to background.
If it is not an infinite loop, either do 'set background=1' or set world.loop_checks=0.
proc name: AI (/mob/enemy/proc/AI)
usr: Guest-308764653 (/mob)
src: the skull (/mob/enemy/skull)
usr.loc: Grass (7,14,1) (/turf/Grass)
src.loc: AI (9,15,1) (/turf/AI)
call stack:
the skull (/mob/enemy/skull): AI()
AI (8,14,1) (/turf/AI): Enter(Guest-308764653 (/mob), Grass (7,14,1) (/turf/Grass))

Here is my current code for the turf as well as the proc and enemy. This code format was found in the Dante's tutorial here https://www.youtube.com/ watch?v=sqGp3LGBRuc&list=PLvvX2aT8X6e6Lnry3-kp5_aiD0vo57YDR& index=10

Code:
    mob/enemy
var/intelligence=0
skull
icon='skullenemy.dmi'
icon_state="skull"
baseIcon="skull"


New()
src.respawn=src.loc



proc/AI()
while(src.intelligence==1)
/* for(var/mob/m in view(3,src))
if(m.client&&src.target==null)
walk_towards(src,m,0,3)
src.target=m*/

if(src.target!=null)
var/mob/Target=src.target
if(Target.x<src.x-3||Target.x>src.x+3)
walk(src,0)
src.loc=src.respawn
src.target=null
src.intelligence=0
for(var/mob/m in get_step(src,src.dir))
if(m.client&&src.icon_state!="")
var/damage=src.str-(m.def/3)
m.hp-=damage
m.healthCheck()
src.deathCheck(m)
world<<"hey *8"
sleep(10)


turf/AI
icon='Grass.dmi'
icon_state="grass"
Enter(var/mob/m)
if(m.client)
for(var/mob/enemy/e in view(3,m))
if(e.target==null)
walk_towards(e,m,0,3)
e.target=m
e.intelligence=1
e.AI()
return 1

mob/proc/deathCheck(var/mob/killed)
if(killed.hp<=0)
if(killed.client)
killed.loc=locate(1,1,1)
killed.hp=killed.maxhp
killed.healthCheck()

else
var/mob/enemy/e=killed
killed.icon_state=""
walk(killed,0)
killed.loc=killed.respawn
killed.overlays-=killed.healthBar
var/obj/bar/healthOverlay/o=new();killed.overlays-=o
e.intelligence=0
sleep(50)
killed.icon_state=killed.baseIcon
killed.hp=killed.maxhp
killed.healthCheck()
killed.target=null


Problem description:

Here is all the code I have from the tutorial's AI turf video. Any help would be greatly appreciated.
Best response
Your first problem is using that tutorial. Just looking over it extremely briefly, that person is an extraordinarily-novice programmer and should not be giving advice to others. He's like a kid with a learner's permit giving driving tips to ten-year-olds. Nothing good comes of that.

Now, your main problem is right here:
        while(src.intelligence==1)
/* for(var/mob/m in view(3,src))
if(m.client&&src.target==null)
walk_towards(src,m,0,3)
src.target=m*/

At not point does src.intelligence change to a value that will allow the loop to terminate, and thus you have an infinite loop. There are many other problems with your code, though.

As for your other problems:

In this case, intelligence is what is known as a Boolean type. The specific value of it doesn't matter, only whether it's "true" or "false". DM does not have a specific boolean type as some other languages do (or data types at all), so it's more a semantic property rather than a syntactic property. Nevertheless, the values 0, null, and "" (an empty string) are all treated at "false", while everything else is treated as "true".

A good litmus test for whether something should be treated as boolean or not is: when this value is equal to 1, will it behave any differently at all compared to when it's equal to 2 or 3 or any other value? If the answer is "no", then you should treat is a boolean.

Thus, in this situation what you should have is,
while(intelligence)
// Code

src is not necessary or helpful here, so you can leave it off without issue.

So, what is going on in the above is that states in DM act only on boolean data; that is, whether a condition is "true" or "false". Because intelligence is set to a non-false value, it is true, and so the code will execute.

The second problem is that the above loop "should" be more like this:
while(intelligence)
if(src.target!=null)
var/mob/Target=src.target
if(Target.x<src.x-3||Target.x>src.x+3)
walk(src,0)
src.loc=src.respawn
src.target=null
src.intelligence=0
for(var/mob/m in get_step(src,src.dir))
if(m.client&&src.icon_state!="")
var/damage=src.str-(m.def/3)
m.hp-=damage
m.healthCheck()
src.deathCheck(m)
world<<"hey *8"
sleep(10)

Indentation is syntactically-important in DM, as it determines how the compiler interprets code order and scope. Because of how you had it above, there was nothing in the while loop and thus it just saw it as "so long as this value is true, do nothing over and over and over again". How I have it here, it interprets it as "so long as this value is true, do the stuff the code here tells me to".

There are numerous other issues that I'll get to in another reply, beyond the relevant stuff here, but I need to take care of some things first. Needless to say, though, stop using that tutorial. The person has barely an inkling of what they're doing.
Thanks for the help Popisfizzy. There are not many tutorial videos out there and if it were not for these videos, I would not know about the reference guide on Byond's website. It was not an entire waste of time on my part. I actually did not run into a single problem with his coding up until his last video, although the way he goes about coding probably included a lot of unnecessary steps that are not good habits to learn early on. Either way your help is appreciated. I made the corrections in my code but I am still receiving the same error when I walk on the AI turf, undefined loop and my enemy stacks on top of my player. Hopefully you cover this in the next post :)

-BIGBROYO
I was going to try and write a more specific critique to that code, but it's just wrong from nearly every angle. Instead, I'm gonna critique by example to give you an idea of where things can readily go wrong and why.

I actually did not run into a single problem with his coding up until his last video, although the way he goes about coding probably included a lot of unnecessary steps that are not good habits to learn early on.

Well, perhaps not in terms of the overt behavior of the code you didn't experience problems, but you are inexperienced with programming and thus do not have an eye for the the poor design decisions and bad programming practices that riddle this code. It's not simply unnecessary steps but a lot of bad steps and illogical choices.

The most obvious point here is the deathCheck() method. This is an antipattern that has been prolific on this website for years and years, and I believe it dates back to the Zeta source. The way deathCheck() methods like this work is akin to my door asking me whether it's unlocked before it lets me in; in other words, the logic is entirely backwards. It violates in a very essential way two important object-oriented design principles: separation of concerns and extensibility.

Separation of concerns says that an object should be concerned only with its own behavior. In this case, why should the attacker be concerned with how damage is dealt to the attackee? Why should it be concerned with the logic behind whether the attackee has died? It shouldn't. The attacker's only concerns should be determining how much damage it should, at the very least, try to do and perhaps whether it succeeded.

Next, extensibility. Now, this is not strictly an OO design principle but more a general software principle and one that should nevertheless be considered. Here, imagine we decided to add a mob that had unique damage behavior. Specifically, imagine if it took less-and-less damage the lower it's HP was (but with some sort of caveat that made it a non-threat at low HP). The current approach requires an rewrite of the AI() method in order to handle this, because it is completely inflexible regarding dealing damage. Likewise, what if there was some mob that—after being killed the first time—took a stronger form that could then be killed in full. Once again, implementing this would require a rewrite of the deathCheck() proc, making it completely inflexible.

Below is an example of how you could implement something like this while retaining flexibility, doing away entirely with the silliness of the deathCheck() method.

mob
var
health
death_icon_state

proc
Damage(dam = 0, mob/attacker)
// dam only refers to *attempted* damage, not necessarily the actual
// damage inflicted. In the default implementation, though, they are the
// same unless it would make health negative.

// If dam > health, then we will only do as many points of damage
// as there are points of health left.
dam = max(health, dam)
health -= dam

if(health <= 0)
// When health is less than or equal to 0 (less than here is
// just in case) then we call the Die() method to handle the actual
// stuff that happens on death.
Die(attacker)

return dam

Die(mob/attacker)
icon_state = death_icon_state

Dead()
// Returns true when dead, and false otherwise.
return (health <= 0)

And now you can do an attack loop as something like,
mob/troll
var
damage = 10
damage_variance = 1

proc
Target(mob/m)
while(!Dead() && !m.Dead())
// ! is logical not. For example, !true is false and !false is true.
// Basically, this says that so long as the troll is alive and so is
// its target, keep attacking.

if(get_dist(src, m) <= 1)
// If we're within one tile of the target, try and attack.
Attack(m)

walk(src, 0) // Terminate any movement if we're within range.

else
// Refer to the ref for what these arguments mean. One reason I don't
// like the various step and walk methods is because of
// how opaque their arguments are.
walk_towards(src, m, 1, 5, 0)

sleep(10) // Wait one second between each iteration.

Attack(mob/target)
// Does a random amount of damage between damage - damage_variance and
// damage + damage_variance.
return t.Damage(
damage + rand(-damage_variance, damage_variance),
src
)

This is a much, much cleaner approach.

As an aside, here's an example of the benefits of extensibility, using the two examples I gave above.
mob/troll/unkillable
// This troll is, strictly-speaking, not able to be killed, but when its health reaches a certain
// point it can no longer move and it does a pittance in terms of damage. At that point, it is
// basically just a very angry statue.

var
lock_movement = 0 // When true, it can no longer move.

max_health = 100
lock_health = 5

New()
health = max_health
return ..()

Move()
return !lock_movement && ..()

Damage(dam, mob/attacker)
if(health <= lock_health)
// No damage is done if it's below its threshold.
return 0

else
// If it's not below is threshold yet, then we have to compute some things.

dam *= 1/(max_health - health + 1) // Less-and-less damage is done as it has lower health.

if((health - dam) <= lock_health)
// If this would bring it below its threshold, just set it equal to that threshold.
health = lock_health

// And now it can no longer move.
lock_movement = 1

else
health -= dam

// Now we update the damage it does based on its health. It will always do at least one base damage, though
// this can mean between 0 and 2 damage due to variance.
damage = (health - damage_threshold + 1)**2

return dam // And output how much damage was done.

mob/troll/respawner
// This troll will, upon the first time it is killed, respawn into a bigger,
// badder, and more-dangerous version.

var
buff_icon_state

max_health = 50
buff_scale = 4 // Gets a buff of x4 on all stats when it respawns.

const
BASE_STATE = 0
BUFF_STATE = 1
DEAD_STATE = 2

// Its current state.
state = BASE_STATE

Attack(mob/target)
var/dam = damage + rand(-damage_variance, damage_variance)

if(state == BUFF_STATE)
// When in its buff_state, we apply the buff_scale to damage output.
dam *= buff_scale

return target.Damage(dam, src)

Damage(dam, mob/attacker)
if(state == BUFF_STATE)
// In its buff_state, it gets a damage reduction.
dam /= buff_scale

// This calls the parent proc with the given arguments. That is, it does what Damage() does in
// mob/Damage() above, with the given arguments.
return ..(dam, attacker)

Die(mob/attacker)
// Here, you could maybe exploit the attacker argument to make this variant of the troll only
// target its attacker, and ignore anything else. This is an example of why such an argument
// can be useful when defined ahead of time, even if you don't need it now. I'm not gonna quite
// do that here, though.

if(state == BASE_STATE)
// When dying in the base state, change to the buff_state.

state = BUFF_STATE
icon = buff_icon_state
health = max_health * buff_scale

else if(state == BUFF_STATE)
// When dying in the buff state, then it just dies as usual.

state = DEAD_STATE
..()

Dead()
return (state == DEAD_STATE)

Notice how easy both of those were using this approach? Now imagine trying to do this with the approach that they take in the tutorial. It would be nearly impossible without making everything as convoluted as hell. And this is a critique of just one part; the rest of it uses approaches that are just as wrong and use equally-bad patterns that make maintenance, extension, and interaction horrible. Hence why I suggest you no longer use the tutorial.
Thanks for your input Popisfizzy. I see how it would make things easier in the future to go your route.

I figured out the problem, you have to put 'set background=1' underneath proc/AI(). This resolves the loop and the stack error. Not entirely sure what it means, other than background=true/on.

-BIGBROYO
Setting background=1 isn't solving your problem, just masking it. The code is a real mess.

I strongly advise taking a closer look at Pop's code, in depth, and applying his techniques. In particular, while your AI code is the source of your troubles, your deathcheck() is backwards. The death check should belong to the mob that may be dying, not to the killer, and the argument sent should be the killer. Notice how many times you're using the "killed" var in that proc; it's because you made src the killer instead, but that proc has almost nothing to do with the killer and everything to do with the killee.

Fixing the structural problems in your code now will save you a hundred headaches down the line. It's easier to change now than it will be later, and design mistakes have a way of spreading into other code until they're difficult to drive out.