rustg_read_toml_file is broken

[SuperSayu]

As far as I can tell, #98 broke rustg_read_toml_file. We are using it in a tgstation branch, and when I happened to update rust_g, everything that depended on toml files broke.

the toml_file_to_json function in the library passes a json string containing "success" and "content" to byond, and the byond wrapper proc returns the value of "content" if it succeeds. From what I can tell, the value of content is escaped so that it is not actually parsed by json_decode() during this process, and returned as a json-formatted string (thankfully, with escape characters removed) instead of an associative array.

My current workaround is to json_decode the content again before you return it, and that appears to be working.

Test code (in an otherwise empty test environment, with rust_g.dm and a data.toml file to test):

world.New()
        world.log << "Starting"
        var/list/result = rustg_read_toml_file("data.toml")
        world.log << "Read done"
        if(istext(result))
                world.log << "It's just text"
                world.log << result
                var/list/result2 = json_decode(result)
                if(istext(result2))
                        world.log << "still just text"
                else
                        world.log << "converted?!?"

For me, this code returns the incredulous "converted?!?" text. The workaround is also functioning on a live server, providing the correct associative list for the data loaded from file.

[optimumtact]

way to go @tralezab

[ZeWaka]

Worked on the test data

[SuperSayu]

Yeah about that test...
#98 rewrote it.

var/test_json = @{"
{"database":{"data":[["delta","phi"]],"enabled":true,"ports":[8000,25565],"temp_targets":{"case":72,"cpu":79}}}
"}

/test/proc/check_toml_file2json()
    rustg_file_write(test_toml, "test.toml")

    var/toml_output = rustg_read_toml_file("test.toml")

    if (toml_output != test_json) //only works if the output of the function is text
        CRASH("test:\n[test_toml]\n \nexpected:\n[test_json]\n \nrustg:\n[toml_output]")

Previously it was:

/test/proc/check_toml_file2json()
    rustg_file_write(test_toml, "test.toml")

    var/toml_output = json_encode(rustg_read_toml_file("test.toml")) // turn a list into text
    var/test_output = json_encode(json_decode(test_json)) // convert text into a list and back again

    // ~= checks for structural equality
    if (toml_output != test_output)
        CRASH("test:\n[test_toml]\n \nexpected:\n[test_output]\n \nrustg:\n[toml_output]")

[san7890]

I wrote about this a bit in tgstation/tgstation#68690 but I think that what ZeWaka once termed a "dumb tramstation random error" (via #107 (comment), example found here: https://github.com/tgstation/tgstation/runs/7527162114?check_suite_focus=true) may also be related to this bug, since the runtimes seem to be specifically happening on just the Modular Map Loaders.

[19:01:34] Runtime in modular_map_loader.dm,34: bad index
  proc name: load map (/obj/modular_map_root/proc/load_map)
  src: the tramstation (/obj/modular_map_root/tramstation)
  src.loc: the floor (108,137,3) (/turf/open/floor/iron/smooth)
  call stack:
  the tramstation (/obj/modular_map_root/tramstation): load map()
  world: ImmediateInvokeAsync(the tramstation (/obj/modular_map_root/tramstation), /obj/modular_map_root/proc/loa... (/obj/modular_map_root/proc/load_map))
  the tramstation (/obj/modular_map_root/tramstation): Initialize(1)
  Atoms (/datum/controller/subsystem/atoms): InitAtom(the tramstation (/obj/modular_map_root/tramstation), 0, /list (/list))
  the tramstation (/obj/modular_map_root/tramstation): New(1)
  /datum/parsed_map (/datum/parsed_map): create atom(/obj/modular_map_root/tramstat... (/obj/modular_map_root/tramstation), the floor (108,137,3) (/turf/open/floor/iron/smooth))
  /datum/parsed_map (/datum/parsed_map): instance atom(/obj/modular_map_root/tramstat... (/obj/modular_map_root/tramstation), /list (/list), the floor (108,137,3) (/turf/open/floor/iron/smooth), 1, 0)
  /datum/parsed_map (/datum/parsed_map): build coordinate(/list (/list), /list (/list), the floor (108,137,3) (/turf/open/floor/iron/smooth), 1, 0)
  /datum/parsed_map (/datum/parsed_map):  load impl(1, 1, 2, 0, 1, -1e+31, 1e+31, -1e+31, 1e+31, 0)
  /datum/parsed_map (/datum/parsed_map): load(1, 1, 2, null, 1, null, null, null, null, null)
  Mapping (/datum/controller/subsystem/mapping): LoadGroup(/list (/list), "Station", "map_files/tramstation", /list (/list), /list (/list), /list (/list), 0)
  Mapping (/datum/controller/subsystem/mapping): loadWorld()
  Mapping (/datum/controller/subsystem/mapping): Initialize(684916)
  Master (/datum/controller/master): Initialize(10, 0, 1)

/tg/station configs these modular map loaders for Tramstation via this TOML file here: https://github.com/tgstation/tgstation/blob/master/strings/modular_maps/Tramstation.toml . I hope that the associated PR can also rectify that issue as well. It's a good thing we have CI for all maps there otherwise I'm like 95% certain it would have gotten to production and we would probably just get majorly large fuckups.

[SuperSayu]

@san7890 The "bad index" runtimes on the map loader were exactly what we were seeing (leading to a Tramstation that was partly non-functional), and it was a royal pain to track down. I had noticed that it was also not loading the word filter toml, but I wasn't sure exactly why it was breaking until I created the testbed in the OP.