How Could this code be more efficient ? in Developer Help

ID:141036
 
Jul 4 2009, 1:15 pm (Edited on Jul 4 2009, 1:21 pm)
Code:
mob/Move()
    if(!src.move)
        ..()
    else if(src.INKYUU)return 0//state where can not move 
    else if(src.INSUNA)return 0//state where can not move 
    else if(src.INHIJUTSU)return 0//state where can not move 
    else if(src.move) return 0//state where can not move 
    else if(src.stunned) return 0//state where can not move 
mob/Move()
    ..()
    var/image/I
    . = ..()
    if(.)
        if(I)
            switch(dir)
                if(NORTH)pixel_y = -16
                else if(SOUTH)pixel_y = -16
                else if(EAST||WEST) src.icon_state=""
mob/Move()
    ..()
    if(src.health<=0)
        src.SpawnV()
        src.DieParty()

Problem description:
When people move the server efficiency may drop a little bit, is there any way i can reduce, or make it better and still keeping the same style movement?(I`m not to familiar with movement)
#1 Jul 4 2009, 1:22 pm
I'm not sure how much less processor-heavy it would make it, but you could use boolean operators for the else-if chain.
if(moving || thing1 || thing2 || thing3 || thing4) //if any are true
  return
//if all are false, no if() or else needed since it'd stop the proc before here already
return ..()

Not quite sure what you're trying to do with the rest of it, but it looks messy.
#2 Jul 4 2009, 1:23 pm
In response to Kaiochao (#1)
well its movement for a client, i dont know if what you posted would help.
#3 Jul 4 2009, 1:32 pm
In response to Louisthe10 (#2)
mostly within this code i think , i only posted the others to see if they would intertwine with things that would bring efficacy down.

mob/Move()
    ..()
    var/image/I
    . = ..()
    if(.)
        if(I)
            switch(dir)
                if(NORTH)pixel_y = -16
                else if(SOUTH)pixel_y = -16
                else if(EAST||WEST) src.icon_state=""
#4 Jul 4 2009, 2:06 pm
In response to Louisthe10 (#3)
You can use bits.

mob/Move()
    ..()
    var/image/I
    . = ..()
    if(. && I)
         if(dir & NORTH|SOUTH) pixel_y -= 16 //you might have made a typo on the north if, 
                                                //but I'll assume you didn't
         else src.icon_state="" //I'll also assume that there are only cardinal directions


EDIT: Hopefully that works, my bitwise math is rusty =/
#5 Jul 4 2009, 2:14 pm
In response to Louisthe10 (#3)
That code is badly written so that it will never do anything anyway.
Also, for organization and cleanliness purposes you shouldn't even have multiple overrides of the same proc (at the same object type) like that, unless some are actually from external sources (e.g. libraries). It's also a little more efficient to have just one. You should condense them all into one override.
#6 Jul 4 2009, 2:26 pm
In response to Louisthe10 (#2)
Unfortunately, you seem to lack the logic to apply his advice to your code, or are so lazy you didn't even make the effort to try.
|| is the logical OR operator. It takes two values, and returns true if any of them are true, false otherwise*.
*: This true or false value it returns is actually the last argument processed (due to 'short-circuiting'). So 2 || AnyValue will always return 2. The AND operator works in a similar way.
Let me try to make his example a little clearer:
Instead of this: 
mob/Move()
   if(Busy)
      return 0
   else if(Frozen)
      return 0
   ...

You can just use OR: 
mob/Move()
   if(Busy || Frozen) //if we are Busy OR Frozen...
      return 0
   ...

This is also more efficient, but using multiple if()s still wouldn't make the game lag badly or anything. Your problem here, if any, could well be in the procs called, SpawnV() and/or DieParty(). It would be nice if you had actually described the problem well.
Note your original code there didn't even make sense whatsoever and wouldn't correctly prevent moving (there are also other quirks, such as checking for death on moving). Also, when moving IS allowed, you should keep the return value of ..() and pass it (by returning it as well, one way or another), for example with return ..().
Lastly, there are far better ways to manage "currently busy / can't do stuff" states than this, but let's not overwhelm you.
#7 Jul 4 2009, 2:41 pm
In response to Kaioken (#6)
very true on that Kaioken, i know my programing is a little sloppy and needs some organization. Also thanks for the tip
#8 Jul 4 2009, 4:51 pm
Why is there five variables to make it where you can't move? >_> I doubt that really matters, but still.