Snippet Sunday #6: "Why is my code slow?"

Page: 1 2
Mar 19 2015, 2:37 pm
That is so useful if I knew that earlier would've been nice.

DEBUG: sealingproc(usr = Seijin (/mob/player), src = Orochimaru(Elite NPC) (/mob/EliteMissonMob/Orochimaru)
DEBUG: sealed = 1
DEBUG: con = 0

The mob I sealed has no con(control) yet I might still want to seal those mobs.
Mar 19 2015, 2:46 pm
The mob I sealed has no con(control) yet I might still want to seal those mobs.

Your original proc had several if-then statements, which you left out of this post. They all did the same thing, though:

    sealingproc()
        for(var/mob/M in world)
            if(M.sealed==1&&M.con>80)
                sleep(200)
                new/obj/immortality(M.loc)
                sleep(10)
                new/obj/immortalityfire(M.loc)
                sleep(1)
                M.alpha=0
                sleep(1)
                new/obj/immortality(locate(M.sealer))
                sleep(10)
                new/obj/immortalityfire(locate(M.sealer))
                sleep(1)
                M.alpha=initial(alpha)
                M.sealed=0
                M.loc=(locate(M.sealer))
            else
                if(M.sealed==1&&M.con<80)
                    sleep(200)
                    new/obj/immortality(M.loc)
                    sleep(10)
                    new/obj/immortalityfire(M.loc)
                    sleep(1)
                    M.alpha=0
                    sleep(1)
                    new/obj/immortality(locate(M.sealer))
                    sleep(10)
                    new/obj/immortalityfire(locate(M.sealer))
                    sleep(1)
                    M.alpha=initial(alpha)
                    M.sealed=0
                    M.loc=(locate(M.sealer))
                else
                    if(M.sealed==1&&M.con<60)
                        sleep(200)
                        new/obj/immortality(M.loc)
                        sleep(10)
                        new/obj/immortalityfire(M.loc)
                        sleep(1)
                        M.alpha=0
                        sleep(1)
                        new/obj/immortality(locate(M.sealer))
                        sleep(10)
                        new/obj/immortalityfire(locate(M.sealer))
                        sleep(1)
                        M.alpha=initial(alpha)
                        M.sealed=0
                        M.loc=(locate(M.sealer))


Each one of those branches could be better organized:

if(con>80)
else if(con<80)
else if(con<60) <-- this one will never happen, because con<60 is also <80.


You would want to rearrange your branches:

if(con>80)
else if(con>=60)
else


Now, here's the trouble. All three of your branches do the exact same thing.

Just get rid of the con check, because there's no need for it in your code.
Mar 19 2015, 2:51 pm
I fixed it:

sealingproc()
        world << "DEBUG: sealingproc(usr = [usr] ([usr.type]), src = [src] ([src.type])"
        world << "DEBUG: sealed = [sealed]"
        if(!sealed) return
        world << "DEBUG: con = [con]"
        if(con<80)
            sleep(200)
            new/obj/immortality(loc)
            sleep(10)
            new/obj/immortalityfire(loc)
            sleep(1)
            alpha = 0
            sleep(1)
            new/obj/immortality(locate(x,y+2,z))
            sleep(10)
            new/obj/immortalityfire(locate(x,y+2,z))
            sleep(1)
            src.alpha = initial(alpha)
            src.sealed = 0
            loc=(locate(x,y-2,z))

Works as intended now, and the reason why I had if statements doing the same thing. Is because I wanted to scale the time based of their con level, so it substracts a minute of the sealing time, for each increment of 20 con :)

Thanks so much for the continued help.
Mar 19 2015, 2:58 pm (Edited on Mar 19 2015, 3:23 pm)
In response to Seijinz
Seijinz wrote:
Works as intended now, and the reason why I had if statements doing the same thing. Is because I wanted to scale the time based of their con level, so it substracts a minute of the sealing time, for each increment of 20 con :)

^ You could just use a var and math to determine that instead.

#define floor(x) round(x)

// duration = 5 minutes minus 1 minute times increments of 20
var/duration = 3000 - 600*floor(con/20)

// later in the code
sleep(duration)

Mar 19 2015, 3:26 pm
That's a good idea indeed, thanks for that suggestion and snippet.

I got a question:

When I use the sealing verb, it shows anything in view not just mobs but jutsus and banners as well.

How can I make it so it just shows mobs and players?

I tried mob/M in view(8) but it gives me errors :)
Mar 19 2015, 3:31 pm (Edited on Mar 19 2015, 3:37 pm)
In response to Seijinz
You have the input "as null|anything in view(...)". If you want it to only be mobs, you can either change "anything" to "mob", or you can filter out invalid objects from the view() list.

e.g.
chosen = input(...) as null|mob in view(...)


or
var mobs[] = view(...)
for(var/a in mobs)
    if(!ismob(a) || (some other filter condition))
        mobs -= a // exclude it from the choices
chosen = input(...) as null|anything in mobs


or, if you only want to target players, and you have a global list of players (which is kind of standard for games these days)
// a list of atoms in view that are also in global.players (players in view)
var mobs[] = view(...) & global.players
chosen = input(...) as null|anything in mobs
Mar 19 2015, 3:53 pm
I deliberately left that bug in.

That's because I wanted to demonstrate a common error that people make when using input prompts.

You can actually automatically filter them in several ways:

var/mob/m = input() as null|mob in oview(8)


using the built-in filters is one way to go.

You can also filter the list manually:

var/mob/m = input() as null|mob in (view(8)-usr)


^Don't use that exact example, because oview() does the same thing.

You can filter multiple items out at once too by subtracting another list from the provided one:

var/mob/m = input() as null|mob in (online_players-usr.guild)


Make sense?
Mar 19 2015, 3:54 pm
(Kaiochao beat me to it, because while I was writing this post my girlfriend put tin foil in the microwave and started a kitchen fire.)
Mar 19 2015, 4:15 pm
Jesus your GF doesn't know much about electronics does she :3

Makes total sense, even if you think I didn't learn from this I did.

You made me trial and error, and I saw the pieces coming together, I am so sorry for coming across as a disrespectful dick.

I fan'd you, and kaiochao, thanks both a great bunch :)
Mar 19 2015, 4:24 pm
Makes total sense, even if you think I didn't learn from this I did.

You made me trial and error, and I saw the pieces coming together, I am so sorry for coming across as a disrespectful dick.

What you need to hear isn't always what you want to hear.

This is always true. No matter how old you get.
Mar 19 2015, 4:27 pm
That's true, I myself am almost 23 in a few days, engaged with a wonderful woman who already had a kid, which I feel as if it's my own.

This still applies to me, every, single, day :)
Mar 19 2015, 6:29 pm
Filtering by subtracting lists from lists is one of those things that can work so elegantly when you find a use for it that it feels like cheating.
Mar 19 2015, 6:38 pm
^Another thing that people don't know:

if("Amy"<"Betty")
    world << "This works!"


less than and greater than operators work on strings.
Mar 19 2015, 7:25 pm
In response to Ter13
Ter13 wrote:
^Another thing that people don't know:

> if("Amy"<"Betty")
>     world << "This works!"
>

less than and greater than operators work on strings.

I didn't know this one. Is it a shortcut of doing

if(length("Amy")<length("Betty"))
    world<<"This works!"


??
Mar 19 2015, 7:41 pm
Nope, it's a case-sensitive string comparison.

if("stringA"<"stringB")
    world << "this works!"


It checks the ascii character value of each element in the strings until it finds a character that is greater or lesser than the other string, returning true or false, respectively.

Basically, it will tell you whether one string will become before or after another string if you were to try to alphabetize them.
Mar 19 2015, 10:22 pm
Oh, thats a nice thing to know. Thanks :)
Mar 20 2015, 2:05 pm
In response to Ter13
People really don't know that? I thought that was the basics.

Though I hardly need to do string arguments in byond, I don't even think I have any. At least not greater than or lesser than arguments with strings.
Mar 20 2015, 2:21 pm
In response to DanteVFenris
DanteVFenris wrote:
People really don't know that? I thought that was the basics.

Though I hardly need to do string arguments in byond, I don't even think I have any. At least not greater than or lesser than arguments with strings.

Exactly why I didn't know. Unles I did know at one point but forgot due to never using it o.o
Page: 1 2