Can't understand why this happens...

ID:2245359
 
May 9 2017, 7:24 pm (Edited on May 9 2017, 7:56 pm)
(See the best response by Lummox JR.)
The following code comes from this library, if it can help you:
http://www.byond.com/developer/Kidpaddle45/InterfaceCommands

Code:
/Option
    parent_type = /obj
    Click()
        usr.Answer = "[src.name]"
        usr.CloseWindow("Switch","0")
    MouseEntered()
        winset(usr,"Switch.grid1","style='body{background-color:#58FA58;color:#FFFFFF;}'") // Give it a green highlight color
        usr << output(src,"Switch.grid1:1,[usr.List.Find(src)]")
    MouseExited()
        winset(usr,"Switch.grid1","style=''")
        for(var/Z in usr.List)
            usr << output(Z,"Switch.grid1:1,[usr.List.Find(Z)]")


mob/var/list/List = list()
mob/var/Answer = ""

proc/Switch(mob/M, var/Text = "", var/Options = list(), var/Fade = 0, var/CloseOnFade = 0)
    if(winget(M, "Switch", "is-visible") == "true") return

    //Clean up the interface and write the text//
    M << output(null, "Switch.grid1");M.List.Cut() //Clear grid, and clear the list.
    winset(M, "Switch.grid1", "cells = 0x0") // Another way of clearing the grid...
    winset(M,"Switch.grid1","style='body{background-color:#FFFFFF;}'") //Make sure the cells are not highlighted.
    winset(M,"Switch.Text","text = '[Text]'") // Add the text
    winset(M, "Switch.grid1", "cells = 1x[length(Options)]") // Create the right number of cells based on the number of options.
    /////Lets start adding the options///////////

    for(var/a in Options)
        var/Option/O = new ; O.name = "[a]"
        M.List.Add(O)
    for(var/Z in M.List)
        M << output(Z,"Switch.grid1:1,[M.List.Find(Z)]")

    //////Fading Code//////////////////////////////////
    if(!Fade) winshow(M,"Switch",1);winset(M, "Switch", "alpha='255'") // If "Fade = 0" Show the window, no fade, set alpha to 255.
    if(Fade) M.Fade("Switch") //If "Fade = 1" Show the window using the fade in proc.
    //////WAIT FOR THE ANSWER/////////////////////////

    while(!M.Answer)
        sleep(1)
        if(M.Answer) goto END

    /////////RETURN THE ANSWER////////////////////////
    END
    var/Answer = M.Answer; M.Answer = null;M.List.Cut();
    return "[Answer]"


Problem description:
The following piece of code can be found in my library: Interface Commands. It's basically a custom switch proc to replace the built-in one.

Now, lets create 2 switch pop up one after the other like so:
    switch(Switch(src,"Question 1: What keyboard key is used to Interact with other game elements?",list("A","B","C","D")))
        if("A")
            src <<output("<b>CORRECT!</b>","Chat")
        else
            return
    switch(Switch(src,"Question 2: Test2",list("A","B","C","D")))
        if("B")
            src <<output("<b>CORRECT!</b>","Chat")
        else
            return
For some reasons it works perfectly in SINGLE PLAYER. The issue happens when the game is online. Only one switch will appear. Even if they chose the right answer, it will only output CORRECT and the 2nd pop up wont show. To make the case even weirder, sometimes it randomly shows the 2nd switch pop up after you get the right answer...

I'm going crazy o.o
May 9 2017, 7:47 pm
Just curious, is it somehow switching between clients/mobs so src is then a different player and not the original src?

As in, /Option is taking usr from Player A and then somewhere along the lines Player B becomes usr?

I've always cringed at walls of switch statements. I prefer tree-style where the "quiz" is all contained within 1 parent switch().
May 9 2017, 7:57 pm
Thought it was the issue but this happens when no one is on the server :/

Found the issue. Adding a sleep(10) in between each Switch() fix the issue. The reason is probably that on server the Switch Interface popup doesnt get the time to disapear and there's a check at the begining of the Switch() proc to see if there is currently a visible Switch pop up...if there is one, it wont show the new Switch pop up.

Now my question is, why does it work in single player and not online :/
May 9 2017, 8:00 pm
Sounds like you should be avoiding sleep() and do some fail-safe checks instead.
May 11 2017, 12:34 pm
y goto tho

    while(!M.Answer)
        sleep(1)
        if(M.Answer) break
May 11 2017, 1:21 pm
In response to Unwanted4Murder
That second conditional is unnecessary altogether regardless.
May 11 2017, 1:39 pm
In response to Unwanted4Murder
Unwanted4Murder wrote:
y goto tho

>     while(!M.Answer)
>         sleep(1)
>         if(M.Answer) break
>



for(){sleep(1);if(M.Answer){break}}
May 11 2017, 5:06 pm
In response to FKI
FKI wrote:
That second conditional is unnecessary altogether regardless.

The loop could actually run a single time after the answer state changed if it relied on user input. It's mostly a timing thing. I've had it come up enough to always do the second conditional myself.
May 12 2017, 10:41 am
Instead of sleeping for 1s, the better way to handle this would be a queue in Switch(). If it knows a switch box is still up, it can wait until the previous one has been canceled and then launch the next. That's actually what input() does anyway; it keeps a queue going.

The reason this works in single-player so well and not online is the delay. winget() is a great deal more responsive when it can be done locally. Even the tiny delay of online play means any assumptions that don't take latency into account are invalidated.

Since sleep() can do a lot of the work for you, this would be my approach for Switch():

client/var/switch_inuse = 0   // now-serving counter
client/var/switch_id = 0      // ID number for next call

proc/Switch(mob/M, var/Text = "", var/Options = list(), var/Fade = 0, var/CloseOnFade = 0)
    var/client/C
    if(istype(M, /client)) C = M
    else if(istype(M)) C = M.client
    if(!C) return

    // wait for my turn
    var/id = C.switch_id++
    while(C && C.switch_inuse != id) sleep(1)
    if(!C) return

    // from now on, use goto cleanup instead of return
    // and set . to the return value
    ...

    // end of proc
    cleanup:
    ...  // other misc cleanup
    // increment the now-serving counter (important!)
    if(C && C.switch_id == ++C.switch_inuse)
        // if no switches are waiting, we can hide the box
        winset(C, "Switch", "is-visible=false")

This method avoids the winget() entirely. When switch_id and switch_inuse are in sync, that means a new Switch() box is ready to be displayed to that client. The very first thing this proc does (aside from grabbing a convenient reference to the client) is take a number so it can wait if need be. Then for as long as the switch_inuse number doesn't match that ID, it waits because it knows there's still a Switch() in progress and maybe others waiting ahead of it. It is extremely important that every Switch() call increment client.switch_inuse at the end, so that other calls can then go forward. The bit of logic I added at the end always does that increment, and also checks to see if there are any calls still waiting; if there aren't, it knows to close the box.
May 12 2017, 10:49 am
In response to Lummox JR
Best response
Actually as soon as I posted this I realized a potential flaw: What if the proc that ultimately called this gets canceled via a deletion, but the client is still valid? In that case switch_inuse remains wrong forever because it never gets incremented.

Let's try this instead:

autocounter
    var/object
    var/name

    New(object,name)
        src.object = object
        src.name = name
    Del()
        if(object) object.vars[name]++
        ..()


And now in Switch():
client/var/switch_inuse = 0   // now-serving counter
client/var/switch_id = 0      // ID number for next call

proc/Switch(mob/M, var/Text = "", var/Options = list(), var/Fade = 0, var/CloseOnFade = 0)
    var/client/C
    if(istype(M, /client)) C = M
    else if(istype(M)) C = M.client
    if(!C) return

    // wait for my turn
    var/id = C.switch_id++
    while(C && C.switch_inuse != id) sleep(1)
    if(!C) return

    // whenever this proc ends, now we know for sure C.switch_inuse will be incremented
    var/autocounter/AC = new(C, "switch_inuse")

    // from now on, use goto cleanup instead of return
    // and set . to the return value
    ...

    // end of proc
    cleanup:
    ...  // other misc cleanup
    // don't increment the counter now because AC.Del() will do it
    if(C && C.switch_id == C.switch_inuse+1)
        // if no switches are waiting, we can hide the box
        winset(C, "Switch", "is-visible=false")