Can someone please help me with this combat thing I am trying to work out?

ID:2663253
 
Mar 11 2021, 4:24 am
(See the best response by Spevacus.)
Code:
mob/verb/Attack()

    if(src.doing == null)

        for(var/mob/Monster/M as mob in get_step(src,src.dir))
            if(src.doing == "Fighting")
                return
            else
                src.doing = "Fighting"
                src.Combat(M)
                return



mob/proc/Combat(mob/M)

    if(istype(M,/mob/Monster))
    
        Battle_Start

        var/src_agi = roll("1d[src.agi]")
        var/mob_agi = roll("1d[M.agi]")

        if(src_agi < mob_agi)
            src << 'Miss.wav'
        else
            var/damage = roll(src.dmg_dice) + src.str - M.def
            if(damage <= 0)
                damage = 0
                src << 'Miss.wav'
                F_damage(M,damage,"#EF9B84")
            else
                src << 'Punch.ogg'
                F_damage(M,damage,"#EF9B84")
                M.hp_min -= damage
                if(M.hp_min <= 0)
                    spawn() src.Death(M)
                    return
                else

        sleep(3)

        if(mob_agi < src_agi)
            src << 'Miss.wav'
        else
            var/damage = roll(M.dmg_dice) + M.str - src.def
            if(damage <= 0)
                damage = 0
                src << 'Miss.wav'
                F_damage(src,damage,"#EF9B84")
            else
                src << 'Punch.ogg'
                F_damage(src,damage,"#EF9B84")
                src.hp_min -= damage
                if(src.hp_min <= 0)
                    spawn() M.Death(src)
                    return
                else

                    sleep(7)
                    if(src)
                        if(M)
                            goto Battle_Start
                        
        src.doing = null
        M.doing = null



mob/proc/Death(mob/M)

    if(istype(M,/mob/Monster))

        if(M.hp_min < 0)
            src << ">> You've defeated [M.name]."
            src.exp += M.exp
            src.Update()

            if(M.gold)
                src << ">> You've found [M.gold] gold coin(s)."
                src.gold += M.gold
                src << 'Gold.wav'

            src.doing = null
            new/obj/Blood(M.loc)
            del(M)
            return


Problem description:
The intended way the code is supposed to work is, the player uses the Attack() command to attack things around them, right now it's just checking for mobs, but later on I intend to allow players to attack other things, like furniture.

Attack() makes sure you aren't already doing something, checks for a monster in front of the player and then calls Combat(), which is supposed to be like a turn based MUD combat system, in which the two sides trade blows until someone runs away or die.

With the code I am showing you right now, having 'return' after calling the Death() proc does not yeild any errors, but simply acts as if the monster never died, it also locks the players stats in fighting mode as if the fight never ended.

If I remove the 'return' after the Death() proc, Death() will be called properly and the monster will die, but then I get an error about the mob no longer being there and being unable to read the vars.

I am sure it has something to do with how I am trying to time all of this out, like something is in the wrong order or something. Can someone PLEASE help me figure this out, I've been trying to fix this for three days.

Mar 11 2021, 10:27 am (Edited on Mar 11 2021, 1:56 pm)
Change 
M.doing = null

To
if(M) M.doing = null


Youre getting the error because you're trying to set doing to null when the M has died and no longer exists. The if statement will check if they still exist and if not it won't do anything.

Edit:
?. Would be a tidier alternative to the above as suggested by Spevacus below:
M?.doing = null
Mar 11 2021, 10:42 am
Best response
It's worth mentioning that M?.doing = null does the same thing as what Kirix recommends and is a bit more readable. If M is null, it'll just gloss over the statement without throwing an error.
Mar 11 2021, 11:24 am
In response to Spevacus
Spevacus wrote:
It's worth mentioning that M?.doing = null does the same thing as what Kirix recommends and is a bit more readable. If M is null, it'll just gloss over the statement without throwing an error.


I didn't know that, very helpful thanks Spevacus!
Mar 11 2021, 4:12 pm
In response to Spevacus
Wow, that's awesome, I had no idea you could do that!
Mar 11 2021, 4:14 pm
In response to Spevacus
With that being said, it's still not working as intended. The current error message is:

runtime error: Cannot read null.dmg_dice
proc name: Combat (/mob/proc/Combat)
  usr: (src)
  src: HartWing (/mob)
  src.loc:  (331,98,1) (/turf/Turfs/T003)
  call stack:
HartWing (/mob): Combat(null)
HartWing (/mob): Attack()


Edit: I fixed it, ignore me, haha.
Mar 11 2021, 4:46 pm
In response to Spevacus
Well it looks like I have a new problem now, when you first encounter a monster and attack it, it works as intended, every monster after that, instead of going in rounds for combat like the first time, it just a single pass through and then stops, so you have to tap the verb to make it keep going. I'm not getting any errors, any ideas?
Mar 11 2021, 5:46 pm (Edited on Mar 11 2021, 5:53 pm)
In response to HartWing
Sounds like you're falling into a part of your loop that's not handled and will not always wrap back up to Battle_Start.

I would suggest changing up your battle loop a bit to something more easily controllable, like a while loop. Whenever you want to break out of the loop, you can change the controlling condition(s) to be false, and you will bust out of it. You can also call break to immediately terminate the innermost loop (in this case, being our while loop).

Here's an example of how I would format your battle loop based on the code you provided above. This could use boatloads of improvements I'm sure, but I feel like you'll better understand what's happening in your loop if it's structured in such a way, and you can build (or completely rework) this in the future so that it feels easier to manipulate for you:

var/battleIsActive=1
while(battleIsActive)//The top of the battle loop
    var/src_agi = roll("1d[src.agi]")
    var/mob_agi = roll("1d[M.agi]")
    if(src_agi < mob_agi)
        src<<'Miss.wav'
    else
        var/damage=roll(src.dmg_dice) + src.str - M.def
        if(damage<=0)
            damage = 0
            src<<'Miss.wav'
            F_damage(M,damage,"#EF9B84")
        else
            src<<'Punch.ogg'
            F_damage(M,damage,"#EF9B84")
            M.hp_min -= damage
            if(M.hp_min <= 0)
                src.Death(M)
                break//Break the loop immediately, as death has occured and the mob shouldn't
                //be able to attack our player if it's dead.
    sleep(3)
    if(mob_agi < src_agi)
        src<<'Miss.wav'
    else
        var/damage = roll(M.dmg_dice) + M.str - src.def
        if(damage <= 0)
            damage = 0
            src << 'Miss.wav'
            F_damage(src,damage,"#EF9B84")
        else
            src << 'Punch.ogg'
            F_damage(src,damage,"#EF9B84")
            src.hp_min -= damage
            if(src.hp_min <= 0)
                M.Death(src)
                break//Break the loop immediately, the player died
    sleep(7)//Following this, assuming you've not set battleIsActive to 0 anywhere
    //and haven't hit a "break" call, you should go back to the top of the loop.
src.doing = null
M?.doing = null


I would also consider extracting the dealing-damage portion out of this loop and making it its own proc, that way you can just call it for either the player or the mob. It could look something like:

//Returns true if the damage killed the target
mob/proc/dealDamage(mob/M)
    var/damage=roll(src.dmg_dice) + src.str - M.def
    if(damage<=0)
        damage = 0
        if(src.client) src<<'Miss.wav'//If our src is a player, play a sound.
        F_damage(M,damage,"#EF9B84")
        return 0
    else
        if(src.client) src<<'Punch.ogg'
        F_damage(M,damage,"#EF9B84")
        M.hp_min -= damage
        if(M.hp_min <= 0)
            src.Death(M)
            return 1
        return 0


Then, our loop that we made earlier, when revised, looks like...

var/battleIsActive=1
while(battleIsActive)//The top of the battle loop
    var/src_agi = roll("1d[src.agi]")
    var/mob_agi = roll("1d[M.agi]")
    if(src_agi < mob_agi)
        src<<'Miss.wav'
    else
        if(src.dealDamage(M))//If our damage kills the target...
            break//Break the while loop
    sleep(3)
    if(mob_agi < src_agi)
        src<<'Miss.wav'
    else
        if(M.dealDamage(src))//If the mob's damage kills the player
            break//Break the loop immediately, the player died
    sleep(7)
src.doing = null
M?.doing = null


Much cleaner to read, no?

Good luck as you continue on!
Mar 11 2021, 6:35 pm
Thanks a lot, I appreciate the help and the way you broke it down makes it very easy to read.