Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix tinfoil issue with improper rom DL urls and support for rom folders #1316

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

KaitoKid
Copy link

Addresses the concerns in #1208

Tinfoil Parsing Issues

Root cause is the FS handler for get_roms assumes that directories with a single file inside as multi

.
├── Homebrew
│   ├── my_homebrew.nsp

which results in the following FSRom being created after the directory is scanned

{
  'multi': True, 
  'file_name': 'Homebrew', 
  'files': [{'filename': 'my_homebrew.nsp', 'size': 0, 'last_modified': 1718254179.6487029}]
}

This fix addresses two things:

  1. Single files in directories being mistakenly addressed as multi
  2. Changing the file path to be the actual path of the file, not the file_name. Instead, it renames the file in Tinfoil to the file_name instead.

Known Bugs

Since this takes just the first obj of each rom file, it cannot handle cases where multiple files are stored in one directory. I'll defer to the community on the best method of handling this, but a couple ideas come to mind:

  1. Creating separate directories at the root level for base/patch/update
  2. Standardizing file names so that titledb/tinfoil's api/something can be matched up

@KaitoKid
Copy link
Author

Added a few more updates to address the following:

  • Handling empty folders
  • Addressing rom.full_path using the file_name, which is stored as the rom directory name (causes an inaccurate file redirect upon download
  • Rolled back changes to the filename. I was hoping to rename it as "Rom/rom_file.nsp" within Tinfoil, but I can see that it would not pass routing rules. Leaving it without an extension causes Tinfoil to open it as a file, not as an installer

@KaitoKid KaitoKid changed the title Fix tinfoil issue with single rom in directory Fix tinfoil issue with improper rom DL urls and support for rom folders Nov 25, 2024
backend/handler/filesystem/roms_handler.py Outdated Show resolved Hide resolved
)
),
size=rom.file_size_bytes,
size=rom.files[0].get('size'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rom.files[0] here now needs to be file, after the latest changes.

)
for rom in roms
for rom in roms if rom.files and len(rom.files) > 0
for file in rom.files
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want to expose all files to Tinfoil. Ideally, we could only include the ones with a valid extension that Tinfoil understands (in case users maintain other kind of resources within the rom folder).

@zurdi15 zurdi15 changed the title Fix tinfoil issue with improper rom DL urls and support for rom folders fix: Fix tinfoil issue with improper rom DL urls and support for rom folders Dec 2, 2024
@adamantike
Copy link
Collaborator

@KaitoKid, please let us know if you plan to keep updating this PR.

@gantoine gantoine marked this pull request as draft January 4, 2025 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants