DeathCheck not executing?

Page: 1 2
ID:141143
 
Jun 12 2009, 3:56 am (Edited on Jun 12 2009, 4:02 am)
Code:
obj
    radio
        icon = 'bomb.dmi'
        icon_state = "radio"
        verb
            Pick_up()
                set category = "Actions"
                set src in oview(1)
                src.loc = usr

obj/radio/verb/drop_bomb(varX as num, varY as num, varZ as num)
    set name = "radio"
    set category = "Actions"
    varX = min(world.maxx, max(1, varX))
    varY = min(world.maxy, max(1, varY))
    varZ = min(world.maxz, max(1, varZ))
    var/obj/bigassbomb/B = new
    B.overlays += new/obj/Overlays/big_bomb
    for(var/i=3 to 1 step -1)
        if(!B) return // in case of bomb defenses
        B.Move(locate(varX, varY+i, varZ), SOUTH)
        sleep(5)
    if(!B) return
    B.Move(locate(varX, varY, varZ), SOUTH)
    Explode(new /Effect/BasicBoom(B.loc,1,4))
    for(var/mob/M in view(B, 4))
        M.hp -= (100 + M.defence) // minus to two values combined
        spawn() usr.deathcheck2(M)
    del(B)
    del(src)



 mob
    proc
        deathcheck2(mob/M)
            if(M.hp<=0)//If the health is less than 0.
                world<<"[M] has been bombed by [src]"
                src.kills+=1
                Ranking(src)
                M.loc = locate(0,0,0)
                M<<"wait 10 seconds.. you will be restored"
                sleep(100)
                if(M.team == "blue")
                    M.loc = locate(5,5,1)
                    M.hp = M.mhp
                    scorered += 2
                    endgame()
                    showscores()
                else
                    M.loc = locate(143,143,1)
                    M.hp = M.mhp
                    scoreblue += 2
                    endgame()
                    showscores()


Problem description:

Now i dont see why, but after the bomb is dropped and exploded the mobs... dont get affected 0.o

Any ideas why this is happening?
Jun 12 2009, 10:47 am
First of all, your deathcheck() is back-asswards. It's basically walking up to a guy on the street and saying, "Hey, I'm checking for dead people. Have you killed anybody?"

This is how it should be written:

mob/proc/deathcheck(var/mob/killer)
  if(hp <= 0)
    killer << "You killed [src]"
    loc = null
    return 1
  return 0


Then, different types of mobs can have different deathchecks:

mob/dragon/proc/deathcheck(var/mob/killer)
  //check if we died according to the normal deathcheck
  .=..()
  if(.)
    killer << "You be a dragon slayer!"


You should then be calling M.deathcheck(usr) rather than usr.deathcheck(M).


As for the problem you're having: it most likely is an issue with you Explode() proc, which you haven't shown.
Jun 12 2009, 11:02 am
In response to Garthor
Garthor wrote:
First of all, your deathcheck() is back-asswards. It's basically walking up to a guy on the street and saying, "Hey, I'm checking for dead people. Have you killed anybody?"

This is how it should be written:

mob/proc/deathcheck(var/mob/killer)
>   if(hp <= 0)
>     killer << "You killed [src]"
>     loc = null
>     return 1
>   return 0

Then, different types of mobs can have different deathchecks:

mob/dragon/proc/deathcheck(var/mob/killer)
>   //check if we died according to the normal deathcheck
>   .=..()
>   if(.)
>     killer << "You be a dragon slayer!"

You should then be calling M.deathcheck(usr) rather than usr.deathcheck(M).


As for the problem you're having: it most likely is an issue with you Explode() proc, which you haven't shown.

Hello mister.
The explode is from a library i found before,

proc
{
    Explode(Effect/theeffect)
    {
        if(theeffect.orig)
            new /ExplodeNode/CenterAll(theeffect,theeffect.orig.x,theeffect.orig.y,theeffect.basepower)
    }

    ExplodeBM(Effect/theeffect)
    {
        if(theeffect.orig)
            new /ExplodeNode/CenterBM(theeffect,theeffect.orig.x,theeffect.orig.y,theeffect.basepower)
    }
}

ExplodeNode
{

    CenterBM
    {
        New(Effect/theeffect,x as num,y as num, power as num)
        {
            if(round(power) > 0)
            {
                power = theeffect.Apply(x,y,power,NORTH)

                new /ExplodeNode/Straight(theeffect,x,y - 1,power,NORTH)
                new /ExplodeNode/Straight(theeffect,x,y + 1,power,SOUTH)
                new /ExplodeNode/Straight(theeffect,x - 1,y,power,WEST)
                new /ExplodeNode/Straight(theeffect,x + 1,y,power,EAST)
            }
            del(src)

        }

    }

    CenterAll
    {
        New(Effect/theeffect,x as num,y as num, power as num)
        {
            if(round(power) > 0)
            {
                power = theeffect.Apply(x,y,power,NORTH)

                new /ExplodeNode/Straight(theeffect,x,y - 1,power,NORTH)
                new /ExplodeNode/Straight(theeffect,x,y + 1,power,SOUTH)
                new /ExplodeNode/Straight(theeffect,x - 1,y,power,WEST)
                new /ExplodeNode/Straight(theeffect,x + 1,y,power,EAST)
                new /ExplodeNode/Corner(theeffect,x - 1,y - 1,power,NORTHWEST)
                new /ExplodeNode/Corner(theeffect,x + 1,y - 1,power,NORTHEAST)
                new /ExplodeNode/Corner(theeffect,x - 1,y + 1,power,SOUTHWEST)
                new /ExplodeNode/Corner(theeffect,x + 1,y + 1,power,SOUTHEAST)

            }

            del(src)

        }

    }

    Straight
    {
        New(Effect/theeffect,x as num,y as num, power as num, dir as num)
        {
            var
            {
                tx
                ty
            }

            if(round(power) > 0)
            {
                if(x >= 1 && x <= world.maxx && y >= 1 && y <= world.maxy)
                    power = theeffect.Apply(x,y,power,dir)

                    switch(dir)
                    {
                        if(NORTH)
                            ty = -1
                        if(SOUTH)
                            ty = 1
                        if(WEST)
                            tx = -1
                        if(EAST)
                            tx = 1
                    }
                    if(theeffect.speed)
                        spawn(theeffect.speed)
                            src.New(theeffect,x+tx,y+ty,power,dir)
                    else
                        src.New(theeffect,x+tx,y+ty,power,dir)
            }
            else
                del(src)
        }
    }

    Corner
    {
        New(Effect/theeffect,x as num,y as num, power as num, dir as num)
        {
            var
            {
                ty
                tx
            }

            if(round(power) > 0)
            {
                if(x >= 1 && x <= world.maxx && y >= 1 && y <= world.maxy)
                    power = theeffect.Apply(x,y,power,dir)

                    switch(dir)
                    {
                        if(NORTHEAST)
                            ty = -1
                            tx = 1
                            if(theeffect.speed)
                                spawn(theeffect.speed)
                                    new /ExplodeNode/Straight(theeffect,x,y - 1,power,NORTH)
                                    new /ExplodeNode/Straight(theeffect,x+1,y,power,EAST)
                            else
                                new /ExplodeNode/Straight(theeffect,x,y - 1,power,NORTH)
                                new /ExplodeNode/Straight(theeffect,x+1,y,power,EAST)
                        if(SOUTHEAST)
                            ty = 1
                            tx = 1
                            if(theeffect.speed)
                                spawn(theeffect.speed)
                                    new /ExplodeNode/Straight(theeffect,x,y + 1,power,SOUTH)
                                    new /ExplodeNode/Straight(theeffect,x +1,y,power,EAST)
                            else
                                new /ExplodeNode/Straight(theeffect,x,y + 1,power,SOUTH)
                                new /ExplodeNode/Straight(theeffect,x +1,y,power,EAST)
                        if(NORTHWEST)
                            ty = -1
                            tx = -1
                            if(theeffect.speed)
                                spawn(theeffect.speed)
                                    new /ExplodeNode/Straight(theeffect,x,y - 1,power,NORTH)
                                    new /ExplodeNode/Straight(theeffect,x - 1,y,power,WEST)
                            else
                                new /ExplodeNode/Straight(theeffect,x,y - 1,power,NORTH)
                                new /ExplodeNode/Straight(theeffect,x - 1,y,power,WEST)
                        if(SOUTHWEST)
                            ty = 1
                            tx = -1
                            if(theeffect.speed)
                                spawn(theeffect.speed)
                                    new /ExplodeNode/Straight(theeffect,x,y + 1,power,SOUTH)
                                    new /ExplodeNode/Straight(theeffect,x - 1,y,power,WEST)
                            else
                                new /ExplodeNode/Straight(theeffect,x,y + 1,power,SOUTH)
                                new /ExplodeNode/Straight(theeffect,x - 1,y,power,WEST)
                    }

                    if(theeffect.speed)
                        spawn(theeffect.speed)
                            src.New(theeffect,x + tx,y + ty,power,dir)
                    else
                        src.New(theeffect,x + tx,y + ty,power,dir)
            }
            else
                del(src)
        }
    }
}


I dont think it should interrupt the deathcheck, should it?
Jun 12 2009, 11:48 am
In response to Gogeta126
I have no idea because what you have there is a complete and utter mess. I'm thinking that you calling New() directly may be causing some issues, but I don't see anything off-hand that I'm sure would cause a lockup. However, I'd say you DEFINITELY need to rework what you're doing, there. Here's what I'd do:

obj/explosion
  New(loc, power, speed)
    ..()
    explode(loc) //or whatever it is you want to do to turfs that get exploded on
    var/list/turfs = list(loc, loc, loc, loc)
    for(var/i = 1 to power)
      //step each arm of the explosion forward
      for(var/v = 1 to 4)
        //will catch if we step off the map
        if(turfs[v])
          turfs[v] = get_step(turfs[v], 1<<(v-1))
          explode(turfs[v])
      sleep(speed)

//for creating a fat cross
obj/bigexplosion
  New(loc, size, power, speed)
    ..()
    //initial explosion
    for(var/turf/T in orange(src, size))
      explode(T)
    
    var/list/turfs[4][1+size*2]
    //we have to populate the array with the edges of a "fat cross"
    for(var/v = 1 to 4)
      var/d = 1<<(v-1)
      var/turf/start = loc
      //move to the edge of the initial explosion
      for(var/p = 1 to size)
        start = get_step(start, d)
      //get a list of tiles to the left and right
      turfs[v][1] = start
      var/l = turn(d, 90)
      var/r = turn(d, -90)
      var/turf/left = start
      var/turf/right = start
      for(var/s = 1 to size)
        left = get_step(left, l)
        right = get_step(right, r)
        turfs[v][s*2] = left
        turfs[v][s*2+1] = right
    
    for(var/i = 1 to power)
      //step each arm of the explosion forward
      for(var/v = 1 to 4)
        for(var/p = 1 to 2*size+1)
          //will catch if we step off the map
          if(turfs[v][p])
            turfs[v][p] = get_step(turfs[v][p], 1<<(v-1))
            explode(turfs[v][p])
      sleep(speed)


I guess that didn't end up quite as neat as I was hoping it would be, but oh well.

I think your issue is that you're deleting the bomb before you hurt the mobs. At the risk of making myself fix more code, what's that Apply() proc doing?

Also, could you just put world << "B: [B]" right before the del(B) line to see whether it's getting deleted before that or not?
Jun 12 2009, 12:15 pm (Edited on Jun 12 2009, 12:45 pm)
I may be rusty, but I'm going to say the problem is here:

    for(var/mob/M in view(B, 4))
        M.hp -= (100 + M.defence) // minus to two values combined
        spawn() usr.deathcheck2(M)
    del(B)
    del(src)


spawn() says "create a process under me to come back and do this after I'm done."

del(src) says "kill me, and any processes I have under me."

See the problem? If you're going to spawn that death check, you're going to need to do that spawning in something that isn't going to delete itself. Otherwise, by the time the deathcheck process is run, that process won't exist because the garbage collection threw it out with everything else associated with that object.

Quickest, dirtiest fix: sleep() right after the spawn() so that it doesn't get to the del(src) before it gets to the process spawn()ed under src. I say quickest, dirtiest because it really is an inefficient and potentially hazardous way to go about it - for example, if something else deletes this object, you're back where you started.

Better yet, don't spawn the deathcheck all, just immediately kill the sucker if they're dead. Sleeping() after spawning essentially does this, except it goes through all the suspended processes in the game before getting here (again, with potentially dangerous results).

Best yet, don't have a M.hp-=hitpointsToDeduct, create an actual proc under mob like:

mob
  proc/Damage(var/hitpointsToDeduct)
    if (hitpointsToDeduct != null) hp = hp - hitpointsToDeduct
    // Code to check if I'm dead goes here.


After you do that, only use that Damage() proc for adjusting hitpoints from things that can kill them. That way, you know that any time they're hurt they can't avoid their death check. It's also a whole lot easier than having to manually type a death check every time a mob takes damage.
Jun 12 2009, 12:29 pm
In response to Geldonyetich
Geldonyetich wrote:
I may be rusty, but I'm going to say the problem is here:

Yeah, you're right. Spawned threads still have the same src and are attached to it, so they are gone as well when src is deleted.
Jun 12 2009, 12:47 pm
In response to Geldonyetich
I don't feel that your explanation of spawn() is quite right. Mainly, the phrase "after I'm done" implies that the current proc finishes before the code in spawn() executes, as opposed to the next time a sleep() is reached.

Anyway, the quickest fix is to actually change spawn() to spawn(-1) which will send execution to the code in spawn() rather than waiting for later. It will return to the current procedure whenever a sleep() is hit, and once in deathcheck2() it's safe.

But I missed that spawn() when I was looking over this the first time. Whoops.
Jun 12 2009, 12:48 pm
In response to Garthor
hmm.. something i have tested:

Used dropped the bomb on myself. My hp went down to 0 yet i did not die.

lolwut
Jun 12 2009, 1:03 pm (Edited on Jun 12 2009, 1:15 pm)
In response to Garthor
Yeah, I saw that ambiguity in what I wrote too, and failed to bend even my native grasp to the English language to explain it.

What I meant was that spawn() was putting its contents on hold while the rest of the procedure played out, and because the rest of the procedure involved deleting the source, it never got around to performing what was under spawn().

The weird thing about spawn(-1) is I'm not sure why I'd ever need it. If I want to send the execution to the contents of spawn to happen right now, I'd just take out spawn() entirely. DM is single threaded at heart, there's real no room for it to spawn() negatively. I can only put things off, not go back into the past. If I'm trying to sequence my new spawns() before pre-existing spawns, my code's got issues about what can and cannot be suspended.
Jun 12 2009, 1:16 pm
In response to Garthor
Garthor wrote:
Anyway, the quickest fix is to actually change spawn() to spawn(-1) which will send execution to the code in spawn() rather than waiting for later.

Sure, but the correct fix is not to use spawn() there but in his Deathcheck() that causes all of its callers to be delayed, instead of the sleep().
Jun 12 2009, 1:18 pm
In response to Geldonyetich
spawn(-1) makes a change in that you can even have a sleeping function like input() or even an infinite loop in it, but once it hits any sleeping function the original called proc will still be able to continue and return and execution will return to its callers. Otherwise, the callers would be bogged down, often unnecessarily needing to wait for the full completion of the code, even if it's sleeping.
This is of course useful and can only be done by using spawn().
Jun 12 2009, 1:24 pm
In response to Geldonyetich
Oh, in which case you don't understand what spawn() does. As you said, DM is single-threaded at heart, but that's a dumb thing to say because so are processors. Parallelism is produced by rapidly cycling through "simultaneous" processes, not by actually performing multiple commands at the same time.

The difference is that, with spawn(0), control remains in the current procedure until it sleeps, at which point it goes to the spawned thread. That's simplifying it a lot, but essentially how it works. With spawn(-1), the process is the exact same, only control goes to the spawned thread first. The difference is illustrated like so:

mob/verb/test()
    world << "ONE"
    spawn() test2()
    world << "TWO"
    sleep(10)
    world << "FOUR"

proc/test2()
    world << "THREE"
    sleep(10)
    world << "FIVE"


This outputs in order. However, if spawn(-1) is produced, then it would output "ONE THREE TWO (pause one second) FIVE FOUR". If a spawn() is not used at all it would output "ONE THREE (pause) FIVE TWO (pause) FOUR".
Jun 12 2009, 1:28 pm
In response to Kaioken
While that is valid in this circumstance it would not be valid in every circumstance. For example, if in one proc we wanted to call deathcheck() and wait for it to finish, but in another we wanted to call it and not wait for it, spawning off a new thread within deathcheck() would not allow the first proc to work at all.

But I don't see a circumstance in which you would want to be able to do that, here, so it's just a general statement that "always use spawn() instead of sleep()" is not good advice.
Jun 12 2009, 1:33 pm (Edited on Jun 12 2009, 1:48 pm)
In response to Garthor
Sure, I knew all that, I was just saying that it goes against my code philosophy to have a processes spawned and then force a spawn of another process to precede it.

Something like dealing with input as Kaioken pointed out is one of the few exceptions to the rule that I'd allow.

Generally speaking, I just don't like having enough suspended processes floating around that I reach a point where I have to guess that there might be a process floating around that's going to do some damage unless I spawn a process to cut it off at the pass. It's precarious business! Nail everything down that needs to be nailed down, and it's safer when the player starts poking buttons at random intervals.
Jun 12 2009, 1:39 pm
In response to Garthor
Garthor wrote:
While that is valid in this circumstance it would not be valid in every circumstance.

Naturally. Depends on the situation and context.

"always use spawn() instead of sleep()" is not good advice.

Of course. I didn't say that. =P Obviously sometimes sleep() is called for, and often it can't even be replaced by spawn() anyway.
Jun 12 2009, 1:58 pm (Edited on Jun 12 2009, 2:14 pm)
In response to Geldonyetich
Geldonyetich wrote:
Sure, I knew all that, I was just saying that it goes against my code philosophy to have a processes spawned and then force a spawn of another process to precede it.

That's not what's occurring here in the slightest. Nothing is being "forced", we're simply changing the order of processing. Threads in BYOND never (as far as I know) lose control until they hit a sleep() statement of some sort ("some sort" including things like background procedures and input prompts), so there are no unexpected race conditions. Code between any two sleep() statements will be executed without interruption.

With your initial solution:

for()
  spawn()
    X
sleep()


there was the potential for a race condition, where an earlier spawn() would have been processed first, and could have done something to muck up X. However, that is the caveat of using sleep(), and the programmer should be full aware that no assumptions should be made after a sleep().

With a spawn(-1), there is no such issue as immediately upon hitting spawn(-1), the current proc is put on hold while X is processed until it reaches a sleep().


This is all the sort of stuff you appreciate better after you've had to deal with threads in another language. Granted, it's hardly unique, but it's certainly simplified.


EDIT: Actually, there is the condition where your own mob gets killed and its contents deleted - along with the radio - which would stop the explosion right there. However, that's a design issue and would be fixed in the process of fixing another glitch (spamming bombs before the radio is deleted)

Really, another solution is just to be moving this radio to null at the start of the verb. The garbage collector will pick it up after the proc's finished, assuming there are no other references to it.
Jun 12 2009, 2:17 pm (Edited on Jun 12 2009, 2:34 pm)
In response to Garthor
Garthor wrote:
Geldonyetich wrote:
Sure, I knew all that, I was just saying that it goes against my code philosophy to have a processes spawned and then force a spawn of another process to precede it.

That's not what's occurring here in the slightest. Nothing is being "forced", we're simply changing the order of processing.

Oh, I'm sorry, I thought we were still talking about why spawn() would contain a negative argument.

Threads in BYOND never (as far as I know) lose control until they hit a sleep() statement of some sort ("some sort" including things like background procedures and input prompts), so there are no unexpected race conditions. Code between any two sleep() statements will be executed without interruption.

Sure but unless you've wired your code to be hammering the cpu at 100% constantly (which would probably result in something completely unplayable because no time is made to even update the screen) you're definitely going to hit a sleep even if you never typed sleep(). By this I mean you'll naturally run out of things that need to be done right away.

Thus, it's a fair assumption that those spawns() may hit unexpectedly, especially when dealing with an event-driven model game, which is the kind of game I'm most interested in.

So that's why I look at your previous example (with the ONE, TWO, THREE, FOUR, FIVE) and don't think, "oh, look, one could creatively manipulate spawn() to determine the order in which these screen prints execute." Instead, I look at it and say, "why is that man juggling eggs? Balls would make less of a mess when I unexpectedly toss this running chainsaw at him."

With your initial solution:

for()
>   spawn()
>     X
> sleep()


Actually, I was saying this "quick and dirty" solution would be:

  for()
    spawn()
      // spawned code goes here
    sleep() // That's right, in every for loop.


Which is a mess (not only dirty but not particularly quick if we're thinking in terms of CPU load) but would indeed cause that spawn() to be done before it even reached the next iteration of the for loop and well before it reaches the del before the for() that kills everything spawned inside of this proc. But on to your point:

there was the potential for a race condition, where an earlier spawn() would have been processed first, and could have done something to muck up X. However, that is the caveat of using sleep(), and the programmer should be full aware that no assumptions should be made after a sleep().

(Hence why I said it was so very dirty of a solution.)

With a spawn(-1), there is no such issue as immediately upon hitting spawn(-1), the current proc is put on hold while X is processed until it reaches a sleep().

The point I was making is this:

Don't sleep, don't spawn (-1), if it needs to be done now, just do it now. I said this for the very same reason you're telling me here: if the code's already running, you're not going to lose control of it until you hit a sleep (or you just run out of code to be run now).

Funny enough, according to the reference, input() actually contains a sleep.
Jun 12 2009, 2:34 pm
In response to Geldonyetich
spawn() is used when we need something else to happen WITHOUT having to wait for it to finish. In his deathcheck() proc, he does this:

mob/proc/deathcheck()
  DIE
  sleep(100)
  LIVE AGAIN


If we were to simply call deathcheck() without a spawn(), then we would have people in the explosion queuing up to die, waiting for the previous person to live again before they take their turn. Hence, spawn() is necessary.

And you have this really weird paranoia that something elsewhere in the program is going to suddenly explode your procedure if you ever give up control. Maybe you just further don't understand how things work? If, in the middle of executing one procedure, the server receives input from a player, the current procedure will NOT be interrupted, and that input will NOT be processed until the thread sleeps. There is only one active thread of processing at any time, so the idea of another thread randomly interfering with it is silly.
Jun 12 2009, 2:35 pm
In response to Geldonyetich
Geldonyetich wrote:
Oh, I'm sorry, I thought we were still talking about why spawn() would contain a negative argument.

The negative argument is just a flag. sleep() has the same. No, sleep(-10) doesn't mean "go back in time a second". :P

Funny enough, according to the reference, input() actually contains a sleep.

What's funny there? Obviously input() has to wait until the player actually answers with his choice so it can return the choice he made. Or did you think input() immediately returned, then after the player made his choice went back in time and returned it? Haha. =P
Various built-in procs are the same way - they need to wait for something to finish in order to function properly. alert(),input(),Export()... Whenever you call procs like this, there is the potential for conditions to have changed, so you should revalidate any required ones.
Jun 12 2009, 2:45 pm (Edited on Jun 12 2009, 3:11 pm)
In response to Garthor
Garthor wrote:
spawn() is used when we need something else to happen WITHOUT having to wait for it to finish.

I still don't get what I'm supposed to not be knowing. If I knew that killing a procedure before it had a chance to execute the contents of its spawn() would cause those contents to never run, why wouldn't I have known that?

And you have this really weird paranoia that something elsewhere in the program is going to suddenly explode your procedure if you ever give up control.

I am a paranoid coder. I think it's a good habit. However, in this case, it's justified.

  spawn (10)
    IveDamagedThePlayer()
    PerformDeathCheck()
  // Go do something else.


Half a second later, the soon-to-be-dead player taps a key (which manages to get through the internet before the full second has passed). After going through a complicated mishmash of similarly unslept code, it eventually reaches code something along the lines of:

  // This happens immediately, it's not spawned, not sure why I put spawn(10) at first
  if (alive) BlowUpTheEntireUniverse()
  else
    usr << "Hah, you could have done that, good thing you're dead, eh?"


Because I decided to spawn my death check, which is happening in the topic at hand, now I've allowed players character who should be dead to BlowUpTheEntireUniverse().

Obvious solution? Don't spawn things which need to be done immediately! In an event-driven model, something could happen even in half a second! Sure, it's unlikely, but unlikely is a very poor shield when thousands of players are in your game hammering keys.

Paranoid? Definitely. But at least I didn't grant corpses the ability to blow up my universe.

Basically, you're telling me all about this wonderful tool known as spawning and how it works. I'm nodding and saying, "sure, I know that, but only certain things should be spawned - if you're spawning it at -1, unless it's a fairly unusual situation, you might as well just do it now." You keep hearing this and thinking I misunderstand what spawning is.
Page: 1 2