Enhance platform handling in RomM and add FsSlug property

[phscezario]

I hope this PR, along with the other one that was closed (#2771), will help by:

• Adding the platform fs_slug in the mapping configurations
• Enabling search using fs_slug in the API

[gemini-code-assist]

Summary of Changes

Hello @phscezario, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the platform handling within RomM by integrating a new FsSlug property. This change allows for more robust and flexible identification of platforms, particularly when interacting with the RomM API. It addresses the need to search and map platforms using a file system-friendly slug, improving the overall reliability of platform-related operations and resolving a previously identified issue.

Highlights

• New Platform Identifier: Introduced a new PlatformFsSlug property in EmulatorMapping to provide an alternative way to identify platforms, especially for API searches.
• Enhanced Platform Lookup: Modified the RomM logic to first attempt to find a platform using its IgdbId, and if PlatformFsSlug is present, it will also be used to find the corresponding API platform.
• API Search by FsSlug: Enabled the RomM API to search for ROMs using the platform_fs_slug parameter when the PlatformFsSlug is available in the mapping, allowing for more flexible platform identification in queries.
• Improved Mapping Validation: Updated the validation for emulator mappings to ensure that a platform is considered missing only if both Platform and PlatformFsSlug are null.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request enhances platform handling by introducing PlatformFsSlug as an alternative to PlatformId for identifying platforms when fetching games from the RomM API. The changes are in the right direction, but I've found a few critical issues and areas for improvement.

My review focuses on:

• Correctness: I've identified several potential NullReferenceExceptions caused by allowing mapping.Platform to be null without ensuring all its usages are null-safe. The logic for looking up the platform is also flawed and can lead to a crash.
• Maintainability: There's an opportunity to refactor the construction of API query parameters to remove significant code duplication.

I've provided specific comments with code suggestions to address these points. Please review them carefully, especially the critical ones, as they can cause the application to crash during game import.

[gemini-code-assist]

This change allows mapping.Platform to be null if mapping.PlatformFsSlug is provided. However, this introduces NullReferenceExceptions in several places later in the code that still assume mapping.Platform is not null.

For example:

• Line 256: mapping.Platform.Name
• Line 378: mapping.Platform.Name
• Line 409: mapping.Platform.Name

The potential NullReferenceException on line 247 is addressed in a separate comment, but these other instances also need to be fixed. You should either make all usages of mapping.Platform within this loop null-safe (e.g., using the null-conditional operator ?.) or reconsider this condition to avoid breaking existing logic.

[gemini-code-assist]

There are a few issues with the logic for finding the apiPlatform:

1. Typo: There's a redundant assignment apiPlatform = apiPlatform = ... on line 247.
2. Crash Risk: If mapping.Platform is null (which is now possible due to the change on line 240), accessing mapping.Platform.IgdbId on line 247 will cause a NullReferenceException.
3. Flawed Logic: The code first attempts to find the platform by IgdbId and then overwrites it if PlatformFsSlug is available. The logic should prioritize PlatformFsSlug and only fall back to IgdbId if the slug is not present.

Here is a suggested replacement that fixes these issues and is safer and clearer:

                RomMPlatform apiPlatform = null;
                if (mapping.PlatformFsSlug != null)
                {
                    apiPlatform = apiPlatforms.FirstOrDefault(p => p.FsSlug == mapping.PlatformFsSlug);
                }
                else if (mapping.Platform != null)
                {
                    apiPlatform = apiPlatforms.FirstOrDefault(p => p.IgdbId == mapping.Platform.IgdbId);
                }