thpatch / thcrap

Touhou Community Reliant Automatic Patcher
https://www.thpatch.net
The Unlicense
558 stars 41 forks source link

make options actually settable in user config #133

Closed ExpHP closed 3 years ago

ExpHP commented 3 years ago

Currently,

{
  "patches": [
    {"archive": "repos/nmlgc/base_tsa/"},
    {"archive": "lol/test/"},
  ], "options": {
    "fun": { "val": 1000 }
  }
}

in config/*.js does not actually override the value of option:fun. This is because a JSON with the correctly-merged "options" object is constructed in run_cfg.json (and thus printed in the logfile), but this is not the JSON object that is actually used during binhack and codecave compilation!

This PR fixes this by copying the merged "options" object back to file.


An alternative fix would be to change these lines to use run_cfg.json:

https://github.com/thpatch/thcrap/blob/df99529271d0bdf08833df3688cfd38ee00a013e/thcrap/src/runconfig.cpp#L226-L235

This alternative would provide the questionable capability of letting config/*.js control codecaves and binhacks, but with the benefit that the object printed to the logfile would more closely match what's used at runtime. (since setting these properties in this file already affects the JSON that is logged)

ExpHP commented 3 years ago

poke

32th-System commented 3 years ago

Where do you put the options in your run config?

ExpHP commented 3 years ago

I showed the config JSON in the OP. Or are you asking for something else?

To be clear, the JSON provided above assumes that either lol/test/th08.js or lol/test/global.js contains the following:

{
    "options": {
        "fun": {
            "type": "i32",
            "val": 24576
        }
    }
}
ExpHP commented 3 years ago

Ah, one potiential issue just occurred to me in that this commit introduces the side-effect of modifying file, whereas it appears that the original intent of this function was to treat file as essentially const modulo refcounts. (this is reasonably easy to fix by shallow-copying file into a new object near the beginning)

32th-System commented 3 years ago

I put my options here:

{
  "patches": [
    {"archive": "repos/nmlgc/base_tsa/",
     "config": {
        "options": {
            "fun": { "val": 1000 }
         }
     }

    },
    {"archive": "lol/test/"}
  ]
}

and it works. This is how I intended it to work

ExpHP commented 3 years ago

...oh. Okay, sorry, I think I got thrown off by the conversation on discord, and never noticed/completely forgot about 85594b59177b8af7979bdf0358f11e6774eb2a77.

(namely I recall seeing this and assumed that's how you intended options to be used...)