Running client side .winset via "command =" is broken

ID:2253345
 
Jun 1 2017, 12:26 pm
Not a bug
BYOND Version:511
Operating System:Windows 7 Ultimate 64-bit
Web Browser:Chrome 59.0.3071.82
Applies to:Dream Seeker
Status: Not a bug
This is not a bug. It may be an incorrect use of syntax or a limitation in the software. For further discussion on the matter, please consult the BYOND forums.
Descriptive Problem Summary:
If you run winset on the client via the command= parameter, it will always say, winset: Parameter ID_HERE.(null) not found.

Ie, the parameter is always seen as null. If you try to access a nested Parameter, it is always the last one that is seen as null.

example:

winset(MrStonedOne, null, "command = .winset "mapwindow.map.zoom-mode=blur"")
//winset: Parameter mapwindow.map.(null) not found.


Note: There are valid reasons to run winset this way, mainly generic anti-snowflake reasons. This is for a top menu (the one with file, icon, etc) framework that requires being able to simulate clicks to menu items when the client connects. It would be improper snowflakely code to specially run certain commands via winset() rather then .winset, since this framework should ideally not care about what command the menu item runs when clicked.

Numbered Steps to Reproduce Problem:
run verb .do_winset "stuff.here=blah" changing anything about the skin
...
profit
Code Snippet (if applicable) to Reproduce Problem:
/client/verb/do_winset(var/x as text)
    set name = ".do_winset"
    set hidden = 1
    winset(src, null, x)


Expected Results:
It works
Actual Results:
winset: Parameter mapwindow.map.(null) not found.
Jun 14 2017, 2:26 pm
Noting https://github.com/tgstation/tgstation/pull/28470 here so i know to revert this hacky workaround once the underlying byond bug is fixed.
Jun 14 2017, 3:08 pm
I ran a test on this, and I don't think it's actually a bug. The problem appears to be your encoding of the string, which didn't take param encoding into account. This works:

var/p = list2params(list("command" = ".winset \":map.text-color=#fff\""))
winset(src, null, p)

I did notice however that the parsing is screwing up at encountering the : character in front of "map" in my example, so the parser needs to be updated for that newer format. It won't impact your case, though, since you're using an exact name.
Jun 14 2017, 3:09 pm
Lummox JR resolved issue (Not a bug)
Jun 14 2017, 3:24 pm
Here is the actual code thats used:

/datum/verbs/menu/Icon/Load_checked(client/C) //So we can be lazy, we invoke the "checked" menu item on menu load.
    var/atom/verb/verbpath = Get_checked(C)
    if (!verbpath || !(verbpath in typesof("[type]/verb")))
        return
    if (copytext(verbpath.name,1,2) == "@") //verbs who's name starts with @ are treated as direct client side commands and aren't created
        winset(C, null, "command = [copytext(verbpath.name,2)]")
    else
        winset(C, null, "command = [replacetext(verbpath.name, " ", "-")]")



/datum/verbs/menu/Icon/Scaling
    checkbox = CHECKBOX_GROUP
    name = "Scaling Mode"
    default = /datum/verbs/menu/Icon/Scaling/verb/NN

/datum/verbs/menu/Icon/Scaling/verb/NN()
    set name = "@.winset \"mapwindow.map.zoom-mode=distort\""
    set desc = "Nearest Neighbor"

/datum/verbs/menu/Icon/Scaling/verb/PS()
    set name = "@.winset \"mapwindow.map.zoom-mode=normal\""
    set desc = "Point Sampling"

/datum/verbs/menu/Icon/Scaling/verb/BL()
    set name = "@.winset \"mapwindow.map.zoom-mode=blur\""
    set desc = "Bilinear"



    for (var/thing in verblist)
        var/atom/verb/verbpath = thing
        if (!verbpath)
            stack_trace("Bad VERB in [type] verblist: [english_list(verblist)]")
        var/list/entry = list()
        entry["parent"] = "[type]"
        entry["name"] = verbpath.desc
        if (copytext(verbpath.name,1,2) == "@") //verbs who's name starts with @ are treated as direct client side commands and aren't created
            entry["command"] = copytext(verbpath.name,2)
        else
            entry["command"] = replacetext(verbpath.name, " ", "-")

        .[verbpath] = HandleVerb(arglist(list(entry, verbpath) + args))


This is how our topmenu system works.


And it fails here.

edit. here are the full files if you were wondering

https://github.com/MrStonedOne/tgstation/blob/ d94d5ee97212f635d81e99d654935f3ce6d99f7b/code/datums/ verbs.dm datum verb loading system

https://github.com/MrStonedOne/tgstation/blob/ d94d5ee97212f635d81e99d654935f3ce6d99f7b/interface/menu.dm datum verb top menu system

Jun 14 2017, 3:24 pm (Edited on Jun 14 2017, 3:33 pm)
I think I just encoded it wrong in my example in the OP post, but the bug is definitely there.

Clicking the menu item works, but invoking it using command = does not work.
Jun 14 2017, 3:34 pm
Nvm, you're right....

I'm going to credit you in the change log: https://github.com/tgstation/tgstation/pull/28475 (see the cl section)