search

joshcampbell191 / openfpga-cores-inventory

openFPGA Cores Inventory
https://joshcampbell191.github.io/openfpga-cores-inventory/
MIT License
55 stars 8 forks source link

API should not serve up ROM and BIOS links #9

Closed goronfreeman closed 2 years ago

goronfreeman commented 2 years ago

As discussed in #6, it's probably a good idea to remove the direct links to ROM and BIOS files, as this is a legal gray area. However, I do think that it is valuable for the API to let developers know when a core requires a BIOS file to be added before it's functional.

With this in mind, I propose this:

This would make the JSON structure look something like this:

"assets":  {
  "location": "Assets/gba/common/",
  "files": [
    { "file_name": "gba_bios.bin" }
  ]
}

I can see this being useful in an updater app. If a file_name is provided, then they can prompt the user to provide the file, or the updater can provide it.

goronfreeman commented 2 years ago

Created this issue for discussion around the issue. Pinging @joshcampbell191 @mattpannella @RetroDriven

mattpannella commented 2 years ago

yeah I think that's the best idea

mattpannella commented 2 years ago

could also include checksum for each file as an additional optional

neil-morrison44 commented 2 years ago

As “prior art” the MiSTer script which does similar has been up for 2 years: https://github.com/theypsilon/Update_All_MiSTer With, seemingly, no legal issues (though there is an issue where someone suggests presenting a warning on first run imploring the user to check the legal status in their region).

Your mileage for risk is your own of course - but I wouldn’t see many people using this if it requires manual steps for each core

esmith13 commented 2 years ago

I have experienced the same as well with MiSTer... That said, since this is a windows based updater, could a happy medium be the end user providing the mame roms on their own in a specific directory and the updater at least handling the conversion process and copying to pocket?

mattpannella commented 2 years ago

well, the updater I'm maintaining is multi platform and to be honest I am too lazy to deal with converting mras to make roms lol. I think the ideal would be the cores api just contains a list of required (bios) and maybe optional (roms) files with checksums and then the updater apps can do whatever they want. in my case I'd probably just take my existing ROM downloading system and just moving the definition to a new json file that's proprietary to my app and the API only contains the cores and list of required files

goronfreeman commented 2 years ago

well, the updater I'm maintaining is multi platform and to be honest I am too lazy to deal with converting mras to make roms lol. I think the ideal would be the cores api just contains a list of required (bios) and maybe optional (roms) files with checksums and then the updater apps can do whatever they want. in my case I'd probably just take my existing ROM downloading system and just moving the definition to a new json file that's proprietary to my app and the API only contains the cores and list of required files

I think the checksum idea is good. What I'm thinking now, is that we maybe separate out BIOS files from ROM files in the files array. For some cores, like Asteroids, a particular ROM file is needed for the core to be functional at all, so it may also be worth listing these files with their checksums are well. Spitballing, maybe something like this:

{
  "assets": {
    "location": "Assets/foobar/common/",
    "files": [
      {
        "bios": [
          {
            "file_name": "foobar.bin",
            "checksum": "abc123"
          }
        ],
        "roms": [
          {
            "file_name": "game.rom",
            "checksum": "efg456"
          }
        ]
      }
    ]
  }
}

Or maybe instead of bios and roms, it's required and optional?

joshcampbell191 commented 2 years ago

well, the updater I'm maintaining is multi platform and to be honest I am too lazy to deal with converting mras to make roms lol. I think the ideal would be the cores api just contains a list of required (bios) and maybe optional (roms) files with checksums and then the updater apps can do whatever they want. in my case I'd probably just take my existing ROM downloading system and just moving the definition to a new json file that's proprietary to my app and the API only contains the cores and list of required files

I think the checksum idea is good. What I'm thinking now, is that we maybe separate out BIOS files from ROM files in the files array. For some cores, like Asteroids, a particular ROM file is needed for the core to be functional at all, so it may also be worth listing these files with their checksums are well. Spitballing, maybe something like this:

{
  "assets": {
    "location": "Assets/foobar/common/",
    "files": [
      {
        "bios": [
          {
            "file_name": "foobar.bin",
            "checksum": "abc123"
          }
        ],
        "roms": [
          {
            "file_name": "game.rom",
            "checksum": "efg456"
          }
        ]
      }
    ]
  }
}

Or maybe instead of bios and roms, it's required and optional?

I like the idea of adding the checksum field. This can be useful for tools to validate the files once installed.

I'm still unsure about tracking ROMs (as it'll get pretty busy) even though some cores only support a single one (e.g. lunarlander). Also, in the case of NeoGeo, multiple files are required (bios, lo bios and sfix) in which case we might want to use a different name instead of bios perhaps dependencies.

mattpannella commented 2 years ago

that's why I switched to just assets. dependencies works too. splitting them into bios and roms kinda doesn't really accomplish much

neil-morrison44 commented 2 years ago

I agree with just leaving it as assets but as it is if there was a core that needed to install to 2 different paths it’d be unable to - maybe have assets as an object array instead so multiple locations can be defined? Or move “files” up a level and specify that it must be fully qualified paths

joshcampbell191 commented 2 years ago

I agree with just leaving it as assets but as it is if there was a core that needed to install to 2 different paths it’d be unable to - maybe have assets as an object array instead so multiple locations can be defined? Or move “files” up a level and specify that it must be fully qualified paths

The Gameboy / Gameboy Color core does this. I'm pretty sure there's an override location entry for this in @mattpannella updater.

mattpannella commented 2 years ago

yeah I added a location override as a bandaid when I ran into the issue with gb/gbc lol. got the job done

goronfreeman commented 2 years ago

Alright, I took the feedback from the comments above and changed things around again.

Here is how the GB/GBC core would look, where it has two separate BIOS files that need to be installed to two different locations:

{
  "assets": [
    {
      "location": "Assets/gb/common/",
      "files": [
        {
          "file_name": "dmg_bios.bin",
          "checksum": "32fbbd84168d3482956eb3c5051637f5"
        }
      ]
    },
    {
      "location": "Assets/gbc/common/",
      "files": [
        {
          "file_name": "gbc_bios.bin",
          "checksum": "dbfce9db9deaa2567f6a84fde55f9680"
        }
      ]
    }
  ]
}

Here is the NeoGeo response, where multiple files need to be placed in the same directory:

{
  "assets": [
    {
      "location": "Assets/ng/common/",
      "files": [
        {
          "file_name": "uni-bios_1_0.rom",
          "checksum": "6293999bbc32e594aa0ae1da2113dc4d"
        },
        {
          "file_name": "000-lo.lo",
          "checksum": "fc7599f3f871578fe9a0453662d1c966"
        },
        {
          "file_name": "sfix.sfix",
          "checksum": "aa2b5d0eae4158ffc0d7d63481c7830b"
        }
      ]
    }
  ]
}

This does away with the override_location that we currently have. In some ways, I like this better. It's clearer exactly where things need to go, IMO. However, one thing that is up in the air is listing the directory where other optional assets files like ROMs should be placed. Here is how the response looks for a core with no required asset files:

{
  "assets": [
    {
      "location": "Assets/snes/common/"
    }
  ]
}

Is it confusing with this now being an array of objects, there is a single object with only the location?

The only real reason that I separated the BIOS out from the ROMs in the proposed structure in my previous comment is that it gave us the ability to specify critical files vs. optional. We could add required: true to those files maybe?

RetroDriven commented 2 years ago

What about adding MRA files to the API? If Devs put these in Matt and I could Automate the Mame Zip to ROM Conversions on the fly. It could be automated based on the MRA files instead of relying on us for the manual conversion, upload, and additions to a Jason file.

goronfreeman commented 2 years ago

What about adding MRA files to the API? If Devs put these in Matt and I could Automate the Mame Zip to ROM Conversions on the fly. It could be automated based on the MRA files instead of relying on us for the manual conversion, upload, and additions to a Jason file.

I'm not familiar with MRA files. How do you envision this looking?

RetroDriven commented 2 years ago

They're basically Xml files that Mister uses for the Arcade Cores. For the Pocket Arcade Cores these are used with Mame Zips to convert/create Rom files needed.

So I'm thinking if we have the MRAs we can see what Mame Zips are needed. Download them and Convert everything on the fly and automate the Rom conversion needed for the Pocket Cores.

goronfreeman commented 2 years ago

Oh, nice! I just took a look at one of them, and it makes listing the file name and checksum no longer needed. We could even host the MRA files within this repo, since there's no potential legal issues with just providing that. Then we can just provide a list of URLs that point to the MRA files within the repo in the API. Is that kind of what you're thinking? Trying to figure out how we'd want to structure this in the API, but if I"m understanding you correctly, this seems like a good route to go.

joshcampbell191 commented 2 years ago

This might be an unpopular opinion but what about removing the location from the API? I understand this would mean that updaters would have to repeat this logic to place the files in the correct locations however it doesn't seem to "fit".

Another solution could be to add separate properties for asset folders as per the documentation. The main idea here is to abstract the paths.

This could look something like

{
  "platform": "name"
  "assets": [
    {
      "name": "common",
      "files": [
        {
          "name": "file"
          "checksum": "md5"
        }
      ]
    }
  ]
}

The path for an asset can then be constructed as follows: Assets/{platform}/{assets.name}/{files.name}.

goronfreeman commented 2 years ago

I feel fairly strongly that we should have something in the API indicating where assets should be placed. I think something like what you proposed above would work, if we didn't want to list the location directory for whatever reason. One change from what we have currently is that platform would have to be changed from a friendly name like we have now (i.e. NeoGeo) to what is used internally by the core (i.e. ng). That would mean that updaters would have to have an internal map from ng -> NeoGeo for displaying it nicely, or we could add another field with that name in the API. Would this pose any problems for the GB/GBC cores? It's really the odd man out, since it's the only repo that two different cores, so maybe we just have to resign to the fact that it's going to have to be handled differently than the others.

mattpannella commented 2 years ago

i feel like having to maintain a mapping from platform -> friendly platform name in the individual apps removes some portion of the purpose of centralizing this (we'd need to update the apps when new platforms are added). so i definitely say if the above method is what we go with, we include both platform and human readable platform(or whatever you wanna call it)

joshcampbell191 commented 2 years ago

I think we're getting pretty close to a schema we can all agree on. After getting the chance to sit down and revise things a little closer, I came up with the following:

{
  "identifier": "Spiritualized.GBC",
  "platform": "Game Boy Color",
  "repository": {
    "platform": "github",
    "owner": "spiritualized1997",
    "name": "openFPGA-GB-GBC"
  },
  "assets": [{
    "platform": "gbc",
    "common": [{
      "name": "gbc_bios.bin",
      "checksum": "md5"
    }]
  }]
}

I added a platform property to assets entry as the documentation indicates that a core can support multiple platforms. I also renamed repo to repository, user to owner and project to name. The last two were done to align with the GitHub API.

neil-morrison44 commented 2 years ago

Could there be something in having the Jekyll process download & parse the core’s json files? Would serve as a validation on the core’s address & could populate the output JSON with things you don’t want to risk being wrong about (like platform / where the files go / etc)

goronfreeman commented 2 years ago

Nice, I think we're getting close too. My question with this is are we able to convey where to place ROM files for cores that don't have any required assets, like a BIOS? The structure you showed above uses a key common to specify where the files are placed, but what if there are no files? we could have common: null I suppose, but not sure if that's weird or not.

goronfreeman commented 2 years ago

Could there be something in having the Jekyll process download & parse the core’s json files? Would serve as a validation on the core’s address & could populate the output JSON with things you don’t want to risk being wrong about (like platform / where the files go / etc)

I think the closest we could get with Jekyll is using GitHub actions to do something like this. Not 100% sure if that's possible, so we'd have to do some research, but that'd be a good enhancement for the future. I think it's most important we nail down a good structure for the API response so hopefully this can start being utilized. You could open an issue for this, so we can keep it in mind as we develop this more.

joshcampbell191 commented 2 years ago

Nice, I think we're getting close too. My question with this is are we able to convey where to place ROM files for cores that don't have any required assets, like a BIOS? The structure you showed above uses a key common to specify where the files are placed, but what if there are no files? we could have common: null I suppose, but not sure if that's weird or not.

That's a good point. For the most part, cores place them in under Assets/<platform>/common. If a core supports multiple platforms (like the Game Boy / Game Boy Color did initially) then there would be two directories (Assets/gb/common and Assets/gbc/common). It sounds like we should be adding the platform_ids field from the cores.json to the core entry JSON.

@mattpannella @RetroDriven @goronfreeman What are your thoughts?

RetroDriven commented 2 years ago

I think that makes the most sense here. As far as required Rom files go for the Arcade Cores. I believe we can make this work without an entry in the API but it can't hurt to have that in there still if it's not too much trouble and work.

mattpannella commented 2 years ago

is the thought behind using common key that maybe there will be other directories under Assets that we want to move files into? so we could have an array of files for each? i think this all looks great to me and then for me and retrodriven's purposes, I can just drop our existing bios/rom stuff into a new file and use this api to replace our existing cores json file with some tweaks to the models and be good to go

mattpannella commented 2 years ago

Nice, I think we're getting close too. My question with this is are we able to convey where to place ROM files for cores that don't have any required assets, like a BIOS? The structure you showed above uses a key common to specify where the files are placed, but what if there are no files? we could have common: null I suppose, but not sure if that's weird or not.

That's a good point. For the most part, cores place them in under Assets/<platform>/common. If a core supports multiple platforms (like the Game Boy / Game Boy Color did initially) then there would be two directories (Assets/gb/common and Assets/gbc/common). It sounds like we should be adding the platform_ids field from the cores.json to the core entry JSON.

@mattpannella @RetroDriven @goronfreeman What are your thoughts?

i think the example you gave above could cover this problem right(supporting multiple platforms)?


  "assets":[
    {
      "platform":"gbc",
      "common":[
        {
          "name":"gbc_bios.bin",
          "checksum":"md5"
        }
      ]
    },
    {
      "platform":"gb",
      "common":[
        {
          "name":"gb_bios.bin",
          "checksum":"md5"
        }
      ]
    }
  ]
goronfreeman commented 2 years ago

is the thought behind using common key that maybe there will be other directories under Assets that we want to move files into? so we could have an array of files for each? i think this all looks great to me and then for me and retrodriven's purposes, I can just drop our existing bios/rom stuff into a new file and use this api to replace our existing cores json file with some tweaks to the models and be good to go

Yeah, the common key isn't always common. It's the directory where the files underneath it are located. For example, the Asteroids, the file asteroid.rom needs to be placed at /Assets/asteroids/ericlewis.Asteroids/asteroid.rom, so the JSON looks like this:

{
  "assets": [
    {
      "platform": "asteroids",
      "ericlewis.Asteroids": [
        {
          "name": "asteroid.rom",
          "checksum": "1bcd2899e3f92d2824a2ac9def2d3286"
        }
      ]
    }
  ]
}

The key is ericlewis.Asteroids in this case.

There has been a ton of discussion on a host of other things in this issue, but I think we may have gotten to where we need with the most recent JSON structure proposed by @joshcampbell191. There are a couple things I think we should figure out before wrapping this up:

  1. Are we okay with leaving the MRA files discussion for another issue? I think this is a good idea that we should at least explore.
  2. Should the API have a field that indicates whether the core 1) doesn't require any additional files 2) requires a specific file or multiple files 3) can play a host of ROMs. My thinking here is that an updater could prompt a user for providing these files, or in the case of a core like SNES, could prompt to specify a directory where the user's ROMs are kept and handle the transferring to the correct directory.

I have this new structure all ready in #10, I just need to finish up updating the documentation.

joshcampbell191 commented 2 years ago

is the thought behind using common key that maybe there will be other directories under Assets that we want to move files into? so we could have an array of files for each? i think this all looks great to me and then for me and retrodriven's purposes, I can just drop our existing bios/rom stuff into a new file and use this api to replace our existing cores json file with some tweaks to the models and be good to go

Yeah, the common key isn't always common. It's the directory where the files underneath it are located. For example, the Asteroids, the file asteroid.rom needs to be placed at /Assets/asteroids/ericlewis.Asteroids/asteroid.rom, so the JSON looks like this:

{
  "assets": [
    {
      "platform": "asteroids",
      "ericlewis.Asteroids": [
        {
          "name": "asteroid.rom",
          "checksum": "1bcd2899e3f92d2824a2ac9def2d3286"
        }
      ]
    }
  ]
}

The key is ericlewis.Asteroids in this case.

There has been a ton of discussion on a host of other things in this issue, but I think we may have gotten to where we need with the most recent JSON structure proposed by @joshcampbell191. There are a couple things I think we should figure out before wrapping this up:

  1. Are we okay with leaving the MRA files discussion for another issue? I think this is a good idea that we should at least explore.
  2. Should the API have a field that indicates whether the core 1) doesn't require any additional files 2) requires a specific file or multiple files 3) can play a host of ROMs. My thinking here is that an updater could prompt a user for providing these files, or in the case of a core like SNES, could prompt to specify a directory where the user's ROMs are kept and handle the transferring to the correct directory.

I have this new structure all ready in #10, I just need to finish up updating the documentation.

There's only one problem I can see now with this approach and that's the scenario you just described.

When the core requires files that are specific to it (as opposed to files that are common to a platform), the documentation recommends placing them into a core specific folder within the platform folder. This however doesn't work too well when trying to deserialize the JSON into an object with a fixed set of properties.

@goronfreeman @mattpannella @RetroDriven What do you think about introducing a core property to the assets item for files that match this scenario (ex. asteroids)? It would then be up to the updaters / tools to move them into a folder matching the identifier (ex. Assets/{platform}/{identifier}/file.ext)? We would then have a property for common and core specific files.

I think we should be good once we agree on something for this scenario.

@goronfreeman As for the questions:

  1. I think we can create a separate discussion on how to deal with MRA files and see what we can do with them.
  2. What's the difference between 2 / 3? Is it when a core supports multiple platforms? I'm just trying to wrap my head around this one. It sounds like it could be useful to help direct the updaters.
goronfreeman commented 2 years ago

Here's a better explanation of those scenarios:

Requires no additional files

This one is straightforward. This is a core like Pong that is completely self contained. It's designed to play a single game and requires no extra files to be downloaded.

Requires additional file(s)

Now that I think about this one, there are actually a couple different scenarios that would fall into this category.

  1. An arcade core that requires a specific ROM file (i.e. Asteroids)
  2. A core that requires a BIOS file (i.e. GB/GBC)

Scenario 2 here is in interesting, because it's a core that both requires additional files to be operational, but it also can be provided a multitude of optional ROM files.

Requires only ROM files

This one is for cores that don't need any specific additional files for the core to be operational. ROM files still need to be provided to make the core useful, but it doesn't have to be any specific ROM. This is a core like SNES.

My thinking here is that the updaters can utilize this information to present the user with a nice UI to provide the needed files. For scenario 1, the updater wouldn't have to do anything. For scenario 2, the updater could prompt for specific files and utilize the MD5 checksum to validate they are correct. For scenario 3, the updater could prompt for a directory on the user's machine where they store their ROM files, and could handle copying them to the correct directory on the SD card.

goronfreeman commented 2 years ago

@goronfreeman @mattpannella @RetroDriven What do you think about introducing a core property to the assets item for files that match this scenario (ex. asteroids)? It would then be up to the updaters / tools to move them into a folder matching the identifier (ex. Assets/{platform}/{identifier}/file.ext)? We would then have a property for common and core specific files.

You're saying:

{
  "assets": [
    {
      "platform": "asteroids",
      "core": [
        {
          "name": "asteroid.rom",
          "checksum": "1bcd2899e3f92d2824a2ac9def2d3286"
        }
      ],
      "common": [
        {
          "name": "foo.bar",
          "checksum": "asdf123456"
        }
      ]
    }
  ]
}

I think that will solve all the issues we've discussed. When I was trying to add documentation around the previous structure (ericlewis.Asteroids as a key name) I realized it was odd to have a variable key in the JSON response, so I'm happy with this suggestion. Should we move forward with that? I'll update my PR, so we can hopefully get this to completion soon.

joshcampbell191 commented 2 years ago

Thanks @goronfreeman for the detailed explanation and for providing an example of the JSON (which I should've included in my response). It sounds like we might need one more property to track whether a core requires ROM / ROMs on top of the required BIOS files. I'm leaning towards a string at the moment that will represent the assets folder. Most likely the same as we did for the asset items (common / core).

What does everyone think?

mattpannella commented 2 years ago

@goronfreeman @mattpannella @RetroDriven What do you think about introducing a core property to the assets item for files that match this scenario (ex. asteroids)? It would then be up to the updaters / tools to move them into a folder matching the identifier (ex. Assets/{platform}/{identifier}/file.ext)? We would then have a property for common and core specific files.

You're saying:

{
  "assets": [
    {
      "platform": "asteroids",
      "core": [
        {
          "name": "asteroid.rom",
          "checksum": "1bcd2899e3f92d2824a2ac9def2d3286"
        }
      ],
      "common": [
        {
          "name": "foo.bar",
          "checksum": "asdf123456"
        }
      ]
    }
  ]
}

I think that will solve all the issues we've discussed. When I was trying to add documentation around the previous structure (ericlewis.Asteroids as a key name) I realized it was odd to have a variable key in the JSON response, so I'm happy with this suggestion. Should we move forward with that? I'll update my PR, so we can hopefully get this to completion soon.

hey sorry guys, fell asleep early last night and never replied lol. i like this a lot. this seems to solve any issues i can think of

mattpannella commented 2 years ago

Thanks @goronfreeman for the detailed explanation and for providing an example of the JSON (which I should've included in my response). It sounds like we might need one more property to track whether a core requires ROM / ROMs on top of the required BIOS files. I'm leaning towards a string at the moment that will represent the assets folder. Most likely the same as we did for the asset items (common / core).

What does everyone think?

so just an extra property that is a string containing the path to where roms should be put?

goronfreeman commented 2 years ago

As far as I can tell, in order to wrap up this iteration there are current two questions up in the air:

  1. How do we want to represent the directory where ROM files for the core are stored. Most cores use Assets/<platform>/common, but there are a few arcade cores that use Assets/<platform>/<identifier>.
  2. How do we want to address the this. Maybe we can punt on it?

Would appreciate feedback from @joshcampbell191, @mattpannella, and @RetroDriven so we can hopefully move towards actually using this soon!

joshcampbell191 commented 2 years ago

After going through the documentation and checking more in depth individual core configurations, I think I have the answers to these questions however it'll take me a bit of time to put this together. Will try and have something tonight.

mattpannella commented 2 years ago

As far as I can tell, in order to wrap up this iteration there are current two questions up in the air:

  1. How do we want to represent the directory where ROM files for the core are stored. Most cores use Assets/<platform>/common, but there are a few arcade cores that use Assets/<platform>/<identifier>.
  2. How do we want to address the this. Maybe we can punt on it?

Would appreciate feedback from @joshcampbell191, @mattpannella, and @RetroDriven so we can hopefully move towards actually using this soon!

i think #1 is satisfied by the suggestion a few posts up, no?

{
  "assets": [
    {
      "platform": "asteroids",
      "core": [
        {
          "name": "asteroid.rom",
          "checksum": "1bcd2899e3f92d2824a2ac9def2d3286"
        }
      ],
      "common": [
        {
          "name": "foo.bar",
          "checksum": "asdf123456"
        }
      ]
    }
  ]
}
goronfreeman commented 2 years ago

We decided not to track ROM files in the API, as it makes it difficult to maintain, so all the core entries in the above JSON are gone.

joshcampbell191 commented 2 years ago

After spending some time reviewing the documentation and verifying the core definition files, I came up with a method that can be used for providing ROMs / BIOS files.

Inside each core definition exists a file called data.json.

The documentation provides the following description for this file:

Describes up to 32 possible data slots. Each slot can be loaded with an asset, loaded and saved to a nonvolatile save file, and contains a number of options.

Requires additional file(s)

Take for example Lunar Lander's data.json:

{
    "data": {
        "magic": "APF_VER_1",
        "data_slots": [
            {
                "name": "ROM",
                "id": 101,
                "required": true,
                "parameters": 2,
                "filename": "llander.rom",
                "size_exact": 0,
                "size_maximum": 131072,
                "address": "0x00000000"
            }
        ]
    }
}

As you can see, it lists the llander.rom file as required for this core to run.

The parameters field contains a bitmap / int representing the slot's configuration. This includes whether the file is core specific or not. More about the parameters bitmap can be found in the documentation.

In the case of llander.rom, it can be determined by applying a mask (0x2) on the field and checking if the Core-specific file bit is set. When applying the mask on the parameters field, it returns 0x2 meaning that the file should be served from the core's assets folder.

Requires only ROM files

Here's another example, this time for the Spiritualized GB core (some entries have been removed to keep this brief):

{
    "name": "Cartridge",
    "id": "0x100",
    "required": true,
    "parameters": 129,
    "extensions": [
        "gb"
    ],
    "size_exact": 0,
    "size_maximum": 8388608,
    "address": "0x00000000"
},
{
    "name": "BIOS",
    "filename": "dmg_bios.bin",
    "id": "0x103",
    "required": true,
    "parameters": 137,
    "nonvolatile": false,
    "extensions": [
        "bin"
    ],
    "size_exact": 256,
    "size_maximum": 256,
    "address": "0x60000f00"
}

In this example, you'll notice that the BIOS entry follows this same logic.

However, since the core supports multiple ROM files, you'll notice that the Cartridge entry uses the extensions array to provide a filter that will be applied on files when browsing. When applying the mask (0x2) on the parameters field (129) this time, it returns 0x0 meaning that these files aren't core specific and should be loaded from the common folder.

Requires no additional files

Finally, if a core doesn't require any additional files, I don't believe the data.json will contain any references to entries with filename or extensions set.

Since it's possible to determine the location based on the contents of the data.json file using this approach, I don't believe it's necessary to include any more properties to the JSON from the API. As far as I'm concerned, even BIOS files can be removed from the API as they can also be determined using the same approach.

goronfreeman commented 2 years ago

I appreciate the digging in the docs for further information on this! However, I think that by the same logic, every other piece of data in the API, aside from the repo URL, could be constructed with the data found in the various JSON files. For example, here is the Lunar Lander core.json file (truncated for clarity):

{
    "core": {
        "magic": "APF_VER_1",
        "metadata": {
            "platform_ids": ["lunarlander"],
            "shortname": "LunarLander",
            "description": "Atari's 1979 Lunar Lander.",
            "author": "ericlewis",
            "url": "https://github.com/ericlewis/openfpga-lunarlander",
            "version": "0.9.1",
            "date_release": "2022-09-12"
        },
        ...
    }
}

The author and shortname can be used to construct what we've called the identifier in the API currently. I guess it's more of a question of philosophy in what to present in the API. My thinking was that we could provide an easy way for an updater to know the all the core pieces of data they would need to function on the level that the current updaters operate at and hopefully a bit more.

Given the information you were able to find, my mind went the other way, in that we could build some tooling to extract this information from a core's JSON file and spit it out for us to include in the API. It would be great if we could automate a lot of this, but given the limitations of working within Jekyll, it limits our options. I can imagine a world where we could receive notifications of a new core release, download and parse the JSON files, and update the API without us doing anything, but we'd need to stand up an app that could receive incoming requests.

In the meantime though, I personally see the removing of this information as making the API less useful than the JSON files that are already being maintained in each project. I'm not sure it's reasonable that every application would have to build its own way to parse out this information, and I see the API as a more convenient way to get at this information.

goronfreeman commented 2 years ago

Thinking about this a little more and seeing this comment, I have another course of action to propose. I think that we should keep required files in the API. These are only the specific files that can be detected from the data.json files, like you described above. This would not include files for cores like Dig Dug or Xevious, like are in the currently deployed version of the API, because their data.json files don't include any information on which files can be run. This would also mean removing the checksum, as that's not something we can determine ourselves from the JSON files.

For the first iteration, we do this manually. The next step would be to start working on a GitHub Actions workflow to automate this all for us. I think it's feasible to have most of the YAML structure filled out for us once we add a new core URL. The only thing we might still manually have to provide is the display_name. Since I don't think it'd be possible for us to receive a notification of a new core release with Jekyll, it is possible to run a workflow on a schedule. We could just run it every hour or whatever seems reasonable. This would also allow us to add new information stored in these files, like version and release_date, which would also have the side benefit of having the datatable on the website be sortable on those columns.

What do you think? Does that sound like a path forward? I'm happy to explore other options as well, but I want to make this API as useful to the community as possible

joshcampbell191 commented 2 years ago

You're right. It would definitely be easier to keep track of this (required files and the core specific state) in the API manually for the time being. We can then look into expanding into something more dynamic as proposed in the comment from #5. How about when a core supports multiple ROMs? Are we going to keep track of the extensions?

I'm assuming the JSON would look something like:

{
  "assets": [{
    "platform": "gb",
    "filename": "dmg_bios.bin",
    "extensions": [ "bin" ]
  }, {
    "platform": "gb",
    "extensions": [ "gb" ]
  }]
}

Here's another sample for Lunar Lander:

{
  "assets": [{
    "platform": "lunarlander",
    "filename": "llander.rom",
    "core_specific": true
  }]
}

Here are the new additions:

Note that the filename or extensions (or both) properties should always be provided for assets.

I think that about settles all the different scenarios that we'll encounter for now.

joshcampbell191 commented 2 years ago

@goronfreeman @mattpannella @RetroDriven I hope this proposed change works for everyone.

goronfreeman commented 2 years ago

@goronfreeman @mattpannella @RetroDriven I hope this proposed change works for everyone.

This sounds good to me! Thanks for taking the time to sift through all this info. I'm going to apply this to my PR tonight, then I'll tag you for a review. Looking forward to getting this finished up!

mattpannella commented 2 years ago

got the thumbs up from me!