openfpga-cores-inventory / analogue-pocket

https://openfpga-cores-inventory.github.io/analogue-pocket/
MIT License
72 stars 12 forks source link

standardized beta key support via updaters.json #800

Open mattpannella opened 1 week ago

mattpannella commented 1 week ago

now that coin-op collection is implementing a beta key like jotego's stuff, i think we could standardize this by adding a section to updaters.json to identify whether a core requires a license, what the file name is, and where to put it

"license": {
  "file_name": "jtbeta.bin",
  "asset_location": "jtpatreon/common"
}

or something along those lines then we can remove the conditions where we're just checking for "jtbeta" inside data.json, and base everything off the updaters file

mattpannella commented 1 week ago

was talking with @neil-morrison44 via discord, he pointed out we dont even need the asset location. would be better to leave it out to avoid it going out of the sync with the actual core's json files. we can look up the location as long as we have the filename

mattpannella commented 1 week ago

ok, the coinop guys agreed to this format for their cores theyre gonna release soon. so we can stick with this format (and now you can just add a check for this in the inventory parser and not hardcode another file name check)

{
  "license": {
    "filename": "coinop.key"
  }
}
joshcampbell191 commented 1 week ago

Thanks for the update. I'll keep that in mind when updating the code. I'm just about ready to release a major update to the updater script (see refactor branch).

mattpannella commented 1 week ago

as of now, jotego hasn't responded or agreed to implement the license in updaters.json, but i'm not sure it matters much for you, because if you implement the check, if/when he does make the change, it'll just work

mattpannella commented 1 week ago

jotego said his. next release will include the license in updaters.json, so assuming it's good, you can probably drop all the JtBETA logic and just check for the value in updaters.json, now

https://github.com/jotego/jtbin/issues/397#issuecomment-2348095622

mattpannella commented 1 week ago

gah, ran into a minor snag working on this stuff. if/when you implement the check on the inventory, would you be ok with including that license filename in the api response? this way if a core requires a license, we can check that filename and block installing the core. otherwise there's no way of knowing the license file name without pulling down the zip

neil-morrison44 commented 1 week ago

Might be an idea just to copy the entire updaters.json into the inventory API response? - that way anything we add in the future will be available to us for un-downloaded cores too

joshcampbell191 commented 1 week ago

I can provide the contents of the updaters.json in the API response however it should already be part of the data slots / instance.json as a required file. You could always check the contents of the folder to ensure all the required files are present. At least with the updaters.json it would be more clear as to which data file contains the license.

mattpannella commented 1 week ago

@neil-morrison44 good call. this way any new feature we decide to add in there will just be available via the API without needing the pull the zip down