Very messy long code.

ID:171470
 
Sep 19 2004, 12:57 pm
 
obj/Medikit
    icon='stuff.dmi'
    icon_state="mk"
    verb
        Get()
            set src in oview(1)
            usr.contents += src
        Drop()
            if(suffix == null)
                src.loc = usr.loc
        Heal(mob/M)
            if(M.hp==8)
                usr<<"You have full health"
                return
            if(M.hp==7)  
                usr<<"You have healed 1 hp"
                for(var/obj/healthbar/H in M.client.screen)
                    var/is = 0
                    is = text2num(H.icon_state) //make the var "is" the number equivilant to H's icon state
                    is += 1 //add one to "is"
                    H.icon_state = num2text(is) //make H's icon state the text equivilant of is
                    M.hp += 1
                return
            if(M.hp==6)  
                usr<<"You have healed 2 hp"
                for(var/obj/healthbar/H in M.client.screen)
                    var/is = 0
                    is = text2num(H.icon_state) //make the var "is" the number equivilant to H's icon state
                    is += 2 //add one to "is"
                    H.icon_state = num2text(is) //make H's icon state the text equivilant of is
                    M.hp += 2
                return
            if(M.hp==5)  
                usr<<"You have healed 3 hp"
                for(var/obj/healthbar/H in M.client.screen)
                    var/is = 0
                    is = text2num(H.icon_state) //make the var "is" the number equivilant to H's icon state
                    is += 3 //add one to "is"
                    H.icon_state = num2text(is) //make H's icon state the text equivilant of is
                    M.hp += 3
                return
            if(M.hp==4)  
                usr<<"You have healed 4 hp"
                for(var/obj/healthbar/H in M.client.screen)
                    var/is = 0
                    is = text2num(H.icon_state) //make the var "is" the number equivilant to H's icon state
                    is += 4 //add one to "is"
                    H.icon_state = num2text(is) //make H's icon state the text equivilant of is
                    M.hp += 4
                return
            if(M.hp==3)  
                usr<<"You have healed 4 hp"
                for(var/obj/healthbar/H in M.client.screen)
                    var/is = 0
                    is = text2num(H.icon_state) //make the var "is" the number equivilant to H's icon state
                    is += 5 //add one to "is"
                    H.icon_state = num2text(is) //make H's icon state the text equivilant of is
                    M.hp += 5
                return
            if(M.hp==2)  
                usr<<"You have healed 4 hp"
                for(var/obj/healthbar/H in M.client.screen)
                    var/is = 0
                    is = text2num(H.icon_state) //make the var "is" the number equivilant to H's icon state
                    is += 6 //add one to "is"
                    H.icon_state = num2text(is) //make H's icon state the text equivilant of is
                    M.hp += 6
                return
            if(M.hp==1)  
                usr<<"You have healed 4 hp"
                for(var/obj/healthbar/H in M.client.screen)
                    var/is = 0
                    is = text2num(H.icon_state) //make the var "is" the number equivilant to H's icon state
                    is += 7 //add one to "is"
                    H.icon_state = num2text(is) //make H's icon state the text equivilant of is
                    M.hp += 7
                return

Basically this is a medikit...DUH...But when i heal myself the bar dissapears....Is there a resaon why?
Sep 19 2004, 12:59 pm
I don't know, because I didn't read past the part where you had 8 if() statements, one for each possible value of health.
Sep 19 2004, 1:03 pm (Edited on Sep 19 2004, 1:09 pm)
I'm pretty sure all you have to do is make if(hp==1) before if(hp==8), unless your problem is related to the actual changing of icon_states. And use else if after if(hp==1).

I would also recommend you making a better proc than if blah else if blah else if blah else if blah...

Something like:

mob
    verb
         Heal(mob/M)
             usr<<"[M] has healed 1 hp"
             for(var/obj/healthbar/H in M.client.screen)
                 M.hp += 1                    
                 var/num = H.icon_state
                 num += 1 //add one to "is"
                 H.icon_state = num


No idea if that'll work though...


By all your messages say You instead of [M].
Sep 19 2004, 2:43 pm
In response to DeathAwaitsU
Still wrong.
Sep 19 2004, 3:52 pm
In response to DeathAwaitsU
You would probably want to tell M that they've been healed. I'd want to know.
Sep 20 2004, 1:12 am
In response to Garthor
Could you point out what's wrong?

If you mean the example i showed, then is this what i should be doing?

mob
    verb
         Heal(mob/M)
             usr<<"[M] has healed 1 hp"
             M << "You have gained 1 hp"
             for(var/obj/healthbar/H in M.client.screen)
                 M.hp += 1                    
                 var/num = text2num(H.icon_state)
                 num += 1 //add one to "is"
                 H.icon_state = num2text(num)


Sep 20 2004, 2:05 am
In response to DeathAwaitsU
That's very slightly closer. There are still several problems though.

Firstly, CodingSkillz2's* original code looked like it was intended to heal up to full health. Your code doesn't do that; it only heals one health. (Actually, even that's not guaranteed. See the next paragraph.) A far better way is to just set their current health to their maximum health, and then update the health bar once.

Secondly, even though you're supposedly only adding one HP each time, you could easily end up adding more than that. Think about what happens if they have more than one health bar in their HUD... See the problem? You're adding 1 HP each time the loop loops. You may now smack yourself upside the head and say "DOH!". ;-)

Thirdly, why increment the health bar as well as the health? It's much more robust to increment the health, and then SET the health bar. That way they can't get out of sync; and if they do, the next Heal() proc will put them back in sync.

Fourthly, while we're talking about being in sync, a by far superior method of keeping a health bar updated is to make a general mob/ChangeHP() proc that changes src's HP; and, if there's a client, also updates the health bar. Then you can call that from all over the place; whenever you change a mob's HP, instead of setting the var you must instead call the proc. Then you don't have to update your health bar in 50 different places.

Fifthly, you didn't check to make sure that M.client isn't null. If you try to heal an NPC, that verb will crash.

There may be other problems I missed, but those are the most immediately obvious ones.

* I just have to point out the irony in the name "CodingSkillz2"... Sorry. Couldn't restrain myself. =P
Sep 20 2004, 4:21 am
In response to Garthor
Garthor wrote:
I don't know, because I didn't read past the part where you had 8 if() statements, one for each possible value of health.

So please explain to the guy why that is a very bad thing to do, or simply ignore it. Comments about your personal attitude to reading code is not a bit interesting to hear about in this forum.


/Gazoot
Sep 20 2004, 2:45 pm
In response to Gazoot
It's not really bad. Just longer than it could be.
Sep 20 2004, 2:51 pm
In response to Dession
Thats a fact i can't hide.
Sep 20 2004, 2:54 pm
In response to CodingSkillz2
What the hell? Did you reply to the wrong post again?
Sep 20 2004, 3:02 pm
In response to Dession
Actually, it IS bad.
Sep 20 2004, 3:05 pm
In response to Crispy
Crispy wrote:
* Couldn't restrain myself. =P

Oh perfectly acceptable, its not like you're a forum moderator or anything. Just don't get bent out of shape when mortals don't restrain themselves.

If there's only one of these objects in the client's screen, you don't need a loop you can just use locate(). Also, instead of having a separate case for each HP value, you can just calculate their HP gain based on their current and maximum HP values (and you could also include a maximum value that the medkit can heal).
Sep 23 2004, 7:11 am
In response to Garthor
Because?

Seriously, just saying something sucks without saying why isn't helping anyone. If you're not helping with your posts, then why post? Is it so hard to state the reason, and *gasp* possible solutions to the problem? Ok, so his programming skill isn't that great. You're not helping him improve by saying his code sucks.

Please, if you're not going to be constructively critisizing, then don't bother critisizing in the first place. We'd rather have poor coders than poor critics. At least there's no mighty ego to contend with in the former group.

~X
Sep 23 2004, 12:57 pm
In response to Xooxer
It's bad because it's inflexible, tedious to write, can include errors if you don't do your calculations write, can have errors if you make a typo, is harder to test, and because it obfuscates your code.