[Bug] Ordered keys are not preserved on `DeepCopy()`

[nickdnk]

Description

If you create a JSON object using:

JSON_Object json = json_read_from_file("file.json", JSON_DECODE_ORDERED_KEYS);

the keys are ordered as expected. If you then do:

JSON_Object jsonCopy = json.DeepCopy();

the keys are now out-of-order.

I also tried calling json.EnableOrderedKeys() before and after copying, but it didn't help.

Reproduction

test_ordered_keys.json:

{
  "players": {
    "76561198034202275": "s1mple",
    "76561198246607476": "b1t",
    "76561198121220486": "Perfecto",
    "76561198044045107": "electronic",
    "76561198040577200": "sdy"
  }
}

char output[4096];
JSON_Object json = json_read_from_file("test_ordered_keys.json", JSON_DECODE_ORDERED_KEYS);
json.EnableOrderedKeys(); // This makes no difference.
JSON_Object jsonCopy = json.DeepCopy();
jsonCopy.EnableOrderedKeys(); // This also makes no difference.

json.Encode(output, sizeof(output), JSON_ENCODE_PRETTY);
LogMessage("Original: %s", output);
jsonCopy.Encode(output, sizeof(output), JSON_ENCODE_PRETTY);
LogMessage("DeepCopy: %s", output);

delete json;
delete jsonCopy;

Original: {
    "players": {
        "76561198034202275": "s1mple",
        "76561198246607476": "b1t",
        "76561198121220486": "Perfecto",
        "76561198044045107": "electronic",
        "76561198040577200": "sdy"
    }
}
DeepCopy: {
    "players": {
        "76561198246607476": "b1t",
        "76561198044045107": "electronic",
        "76561198040577200": "sdy",
        "76561198121220486": "Perfecto",
        "76561198034202275": "s1mple"
    }
}

Versions

sm-json: v4.1.1 SourceMod Compiler: 1.10 SourceMod Build: Not sure. Building with Get5's Docker.

[clugg]

Hey mate, great catch and thanks for reporting this/providing a fix! I've closed your PR as I wanted to add a regression test for this but have credited you in the commit and the release notes. I've just released v4.1.2 with this fix.

[nickdnk]

Great. I thought about adding a test, but I didn't know how you wanted it to work.

Did you ensure in your regression test that it actually fails if the keys are not ordered? It's consistently applying the same wrong (random?) order of keys, so it should be easy to create a case (like mine above) that you know fails if they are not ordered.

[clugg]

To be honest, I was lazy and didn't check - I've just checked and confirmed that my test case wasn't suitable (i.e. the keys were ordered as expected even prior to this fix). I've updated the test now with a case that does fail when run against the previous version but passes on the current release.

Thanks again for your input, it's great to know that people are still actively using this!

[nickdnk]

To be honest, I was lazy and didn't check - I've just checked and confirmed that my test case wasn't suitable (i.e. the keys were ordered as expected even prior to this fix). I've updated the test now with a case that does fail when run against the previous version but passes on the current release.

Thanks again for your input, it's great to know that people are still actively using this!

Alright. Cool.

This project is very important to Get5, so it's much appreciated. I've migrated a lot of code to methodmaps to get a more class-like structure of objects (https://github.com/splewis/get5/blob/development/scripting/include/get5.inc#L169 and down). It helps a lot, and the format is much better than KeyValues. It is also at the foundation of the entire HTTP event-system: https://splewis.github.io/get5/latest/events_and_forwards/