Skip to content

Fix IModRepositoryBridge request methods using the wrong game#2381

Open
JonathanFeenstra wants to merge 1 commit intoModOrganizer2:dev/oauth-graphqlfrom
JonathanFeenstra:fix-imodrepositorybridge
Open

Fix IModRepositoryBridge request methods using the wrong game#2381
JonathanFeenstra wants to merge 1 commit intoModOrganizer2:dev/oauth-graphqlfrom
JonathanFeenstra:fix-imodrepositorybridge

Conversation

@JonathanFeenstra
Copy link
Copy Markdown
Member

All request-methods of the IModRepositoryBridge accept a gameName argument, but this argument only works correctly if there is a game plugin with a gameShortName that matches the gameName. Otherwise, it just falls back to the currently managed game, which would probably confuse plugin developers using this API for games on Nexus that don't have a MO2 plugin (e.g., the Modding Tools section).

To fix this, I made some changes to the Nexus interface classes:

  • I removed m_NexusGameID from the NXMRequestInfo struct, which seems to be unused.
    • Now that this is no longer needed, the constructors only need the Nexus game name instead of the whole game plugin.
    • As a result, any request method overloads with a IPluginGame parameter can be (and have been) removed.
  • I changed getGame to return a nullptr instead of the currently managed game if no matching game plugin is found.
    • All call sites already check for nullptr anyway (in case the managed game is also null).
  • I added a getGameNexusName method that falls back to the supplied gameName if getGame returns nullptr. I changed all implementations of the request-methods of IModRepositoryBridge to use this method instead of getGame(gameName)->gameNexusName().

In other words, if there is no game plugin matching the specified gameName, it's assumed that gameName is already a Nexus game name, allowing plugin developers to use this API even without a matching game plugin. If it turns out that it wasn't a Nexus game name, the request will just fail, which I'd still prefer to silently falling back to the currently managed game.

Based on #2374 to avoid merge conflicts.

@Silarn
Copy link
Copy Markdown
Member

Silarn commented Apr 27, 2026

I think this fundamentally makes sense, but I'd like to run some tests with alternate game sources just to make sure there are no unintended consequences.

@Silarn
Copy link
Copy Markdown
Member

Silarn commented Apr 27, 2026

@Al12rs Since you merged this into the WIP downloads PR do you want to close this and use that instead?

@Al12rs
Copy link
Copy Markdown
Member

Al12rs commented Apr 27, 2026

@Al12rs Since you merged this into the WIP downloads PR do you want to close this and use that instead?

I didn't merge this in the downloads refactor pr. That was a different change that was merged. I have to re-review the OAuth stuff after the latest changes.

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