Use lootGameName() and displayGameName() in initial places
#2030
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Motivations - ModOrganizer2/modorganizer-game_ttw#36 (comment) Surfaced desires to change the Fallout TTW plugin's gameName to something more obvious, from TTW to Tale of Two Wastelands, but that would cause existing profiles to break, `displayGameName()` added in ModOrganizer2/modorganizer-uibase#141 gives us the option to change the way the game plugin is displayed, but leaves the existing integral `gameName()` alone - Similarly ModOrganizer2/modorganizer-game_ttw#36 attempted to fix the sort button LOOT cli argument by changing the `gameShortName()`, as 'TTW' isn't a valid option for LOOT, but that comes with some pretty concerning possible issues # Modifications - Use `displayGameName()` in create instance dialogs & the main window, this doesn't cover all places `gameName()` was being used for purely display reasons, but it covers the bulk - Use `lootGameName()` instead of `gameShortName()` for LOOT cli initiation
|
Note, it's expected the build will fail until ModOrganizer2/modorganizer-uibase#141 is merged |
Holt59
reviewed
May 23, 2024
| @@ -570,7 +571,8 @@ void GamePage::fillList() | |||
| continue; | |||
| } | |||
|
|
|||
| if (!m_filter.matches(g->game->gameName())) { | |||
| if (!m_filter.matches(g->game->gameName()) && | |||
There was a problem hiding this comment.
I'm not sure this should be kept - The gameName() is now internal only, so using it to filter for users could lead to unexpected behavior.
There was a problem hiding this comment.
This area is just the filter search when creating a profile, I don't think it'd hurt anything to continue using the gameName alongside the displayName
For tale of two wastelands for example I can search "TTW" or "Tale of Two wastelands"
Holt59
added a commit
that referenced
this pull request
May 26, 2024
* Use `displayGameName()` in create instance dialogs & the main window, this doesn't cover all places `gameName()` was being used for purely display reasons, but it covers the bulk. * Use `lootGameName()` instead of `gameShortName()` for LOOT cli initiation. * Use game display name in status bar.
Holt59
added a commit
that referenced
this pull request
May 26, 2024
* Use `displayGameName()` in create instance dialogs & the main window, this doesn't cover all places `gameName()` was being used for purely display reasons, but it covers the bulk. * Use `lootGameName()` instead of `gameShortName()` for LOOT cli initiation. * Use game display name in status bar. --------- Co-authored-by: Twinki <[email protected]>
|
Merged manually due to conflict on translation file (+ added change to status bar), 1871f32 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivations
displayGameName()added in AdddisplayGameName()andlootGameName()to plugin-base modorganizer-uibase#141 gives us the option to customize the way the game plugin is displayed without affecting the integralgameName()gameShortName()as 'TTW' isn't a valid option for LOOT, but that also might break existing profiles in some way,lootGameName()will let us fix this without the possibility of breaking existing profilesModifications
displayGameName()in create instance dialogs & the main window, this doesn't cover all placesgameName()was being used for purely display reasons, but it covers the bulklootGameName()instead ofgameShortName()for LOOT cli initiation