Better way to do this?

ID:152680
 
Feb 6 2006, 6:37 am (Edited on Feb 6 2006, 8:57 am)
 
mob
    var
        aura
            myAura

aura
    parent_type=/obj
    var
        r
        g
        b
        on
    New(mob/M)
        var/icon/I=new('Auras.dmi')
        var/obj/I2=new
        I2.icon=icon('Auras.dmi',"2")
        I2.pixel_y = 32
        while(r == null)
            r=input(M,"How much red would you like in your aura?") as num
        while(g == null)
            g=input(M,"How much green would you like in your aura?") as num
        while(b == null)
            b=input(M,"How much blue would you like in your aura?") as num
        I.Blend(rgb(r,g,b))
        src.icon=I
        I.icon=I2.icon
        I.Blend(rgb(r,g,b))
        I2.icon=I
        src.overlays+=I2
        openMe(M)
    proc
        closeMe(mob/M)
            M.overlays -= src
            on=0
        openMe(mob/M)
            M.overlays += src
            on=1
mob
    verb
        Aura()
            if(!myAura)
                myAura = new(src)
                return
            myAura.on ? myAura.closeMe(src) : myAura.openMe(src)
        Change_Aura_Color()
            myAura ? myAura.closeMe(src) : 0 
            myAura = new(src)


Everything works the way I intended it, it was made for a friend so I wanted to make sure it was done correctly. Does anyone have any advice for me to make it better, if so?
Feb 6 2006, 8:14 am
You used a closing tag to open your dm tag. Just a heads up.

Cheetoz wrote:
mob
>   var
>       aura
>           myAura
> 
> aura
>   parent_type=/obj
>   var
>       r
>       g
>       b
>       on
>   New(var/mob/M = usr)

While this might work in this specific circumstance, usually you will want to pass the mob in question to the function. If you expand your system at all, this could easily break by letting it default to usr.
>       var/icon/I=new('Auras.dmi')
>       var/icon/I2=new('Auras2.dmi')
>       I.Shift(1,32)

Just curious, why are you shifting it there? Also, why have two auras and stack one over the other, why not just have them blended together right in auras.dmi in the first place?
>       while(r == null)
>           r=input("How much red would you like in your aura?") as num
>       while(g == null)
>           g=input("How much green would you like in your aura?") as num
>       while(b == null)
>           b=input("How much blue would you like in your aura?")   as num

Since you are going the extra step to do the right thing and use a parameter (M) instead of user throughout this function, you might as well use it instead of usr for input as well. That is, input(M, "How much blue...?")as num
>       I.Blend(rgb(r,g,b))
>       I2.Blend(rgb(r,g,b))
>       src.icon=I
>       src.overlays+=I2
>       openMe(M)
>   proc
>       closeMe(mob/M = usr)
>           M.overlays -= src
>           on=0
>       openMe(mob/M = usr)
>           M.overlays += src
>           on=1
> mob
>   verb
>       Aura()
>           if(!myAura || myAura == null)

If there is no myAura (!myAura) then being null will be caught by the first part, therefor the "|| myAura == null" is redundant. If it's null, it's false and won't get to that part in the first place.
>               myAura = new
>               return
>           if(myAura.on)myAura.closeMe()
>           else myAura.openMe()
>       Change_Aura_Color()
>           if(!myAura || myAura == null)
>               myAura = new
>               return
>           else
>               myAura.closeMe()
>               del(myAura)
>               myAura = new

</dm>
A bit redundant again. If you are going to create a new aura regardless (you do it in both if and else) you might as well just do it once. Doing so would make it instead look like the following.
...
        if(myAura)
            myAura.closeMe()
            del(myAura)
        myAura = new
Feb 6 2006, 8:32 am (Edited on Feb 6 2006, 8:55 am)
In response to Loduwijk
Well, I was shifting it because it is multi-tiled. Thanks for the advice, it is very much appreciated.



EDIT: I should've tested the shift, I thought it would push it over into a new tile ( such as pixel_y ). My mistake, I need to be using a seperate item for the top.


EDIT 2: I am about to update my post with the updated version of the code if anyone has any further advice.
Feb 6 2006, 11:43 am
In response to Cheetoz
Yeah, I tried shift, it doesn't go to the next tile at all. I even found out when I was trying to do something with my own test particle system and my own version of the Grass Generator.