ribrdb / desynced-tools

Tools for working with behaviors and blueprints from Desynced.
MIT License
4 stars 3 forks source link

Upgrade to latest DesyncedJavaScriptUtils #47

Closed swazrgb closed 6 months ago

swazrgb commented 6 months ago

Remove tods.ts and switch to serialization provided by DesyncedJavaScriptUtils

Fixes #45

Vendors https://github.com/StageGames/DesyncedJavaScriptUtils/commit/1a92339440d181fed0f43eebf78ef6266f57c2be

swazrgb commented 6 months ago

This encoder appears to have another issue. Reported to Bernhard from stage.

The problem I've encountered:

image

Game behavior string:

DSCV000YOh02HLhj3A9N5u1or2Op29Kcbj25sxo5052Uza2Ok1KK3YGj4M28CW9O2yTS1Z3W19Zv30qSQ9274vtm3YGsaq25rsYk289lZp00VkF30

JSON:

{
    "0": {
        "0": false,
        "2": 1,
        "3": false,
        "op": "check_number"
    },
    "1": {
        "0": false,
        "op": "notify"
    },
    "vars": [],
    "parameters": [
        false
    ]
}

Encoded behavior string:

DSCV0000Id0IAKUX0vU30M0wceKO1uWXPE1kL6HN1nkr8g22kZID02qoTY33L07c1vjXiw22kZl830qSOm274vtm3YGsam25rsYk289lZp00VkF3A

This decodes to the same JSON as before, but when loaded in the game the Value of compareNumber is empty, instead of referencing P1.

image

ribrdb commented 6 months ago

i think that after a gap in numeric keys all keys are treated as strings. You can't see this in json, but basically the game is getting:

{
 [0]: false,
  "2": 1,
  "3": false,
  "op": "check_number"
}

instead of

{
 [0]: false,
 [2]: 1,
 [3]: false,
 op: "check_number"
}
swazrgb commented 6 months ago

Good catch, thank you!

swazrgb commented 6 months ago

@ribrdb I dropped your patch in favour of @b-stage's

ribrdb commented 6 months ago

@ribrdb I dropped your patch in favour of @b-stage's

Does that actually work? It looks like it's still using js 0-based indexes instead of lua 1-based indexes. Might only show up if there's a gap of 2 or more params

swazrgb commented 6 months ago

I believe you're right. It seems gaps of 2 are rather rare, I haven't managed to get the game to generate any (it prefers emitting false, or maybe the decoder does that?) but if I manually construct the JSON I can trigger the bug.

It seems there is a similar -1 missing from the decoder. I posted this to discord:

Based on my testing this +1 does appear to be necessary. The following behavior JSON:

{
  "0": {
      "0": false,
      "4": 2,
      "op": "select_nearest"
  },
  "1": {
      "0": false,
      "op": "notify"
  },
  "vars": [],
  "parameters": [
      false,
      false,
      true
  ],
  "pnames": [
      "state",
      "out"
  ],
  "name": "foo"
}

This should set Closest to P2. But when loading into the game instead Unit B is set to P2.

However I think there is also a bug in the decoder that makes things confusing.

If I change v|0 to (v|0)+1 and generate a behavior string I get:

DSCV0000If0000AN001NQY1z4bpm1kNZif25rroM2yS5e12Ojve33YHp8E28CW9O2yTS1Z2dV4PP25rsxQ00eQDb37t3xK1mdoX21rBwyb2flreM28Dfxw2zWMUr32zfrT20BtoG2z4aUX30hVAs1rA1Ms00Q

Loading this behavior string gives the correct result of Closest=P2, however decoding this behavior string results in:

    "0": {
        "0": false,
        "5": 2,
        "op": "select_nearest"
    },

Where it shows the +1'd value. So I believe that the encoder is missing a +1, and the decoder is missing a -1.

However I haven't actually managed to get the game to generate a behavior with a gap of 2 which might be why this bug has gone un-noticed?

Just to be sure, this is the definition of the select_nearest instruction:

{ "exec", "A", "A is nearer (or equal)" }, // 0
{ "exec", "B", "B is nearer" }, // 1
{ "in", "Unit A", nil, "entity" }, // 2
{ "in", "Unit B", nil, "entity" }, // 3
{ "out", "Closest", "Closest unit", nil, true }, // 4
swazrgb commented 6 months ago

Cherry picked a fix for both the encoder & decoder

ribrdb commented 6 months ago

However I haven't actually managed to get the game to generate a behavior with a gap of 2 which might be why this bug has gone un-noticed?

I'm seeing it if a call a 4 parameter subroutine and only pass the first and last parameter.