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

Save in-game language setting #4026

Conversation

xoascf
Copy link
Contributor

@xoascf xoascf commented Mar 29, 2024

Copy link
Contributor

@briaguya-ai briaguya-ai left a comment

Choose a reason for hiding this comment

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

it seems a bit odd to just thow a CVarSave() call in here

we already have a call to execute hooks when setting the language

// Persist the new language so it is not overridden on the next frame
if (languageChanged) {
CVarSetInteger("gLanguages", gSaveContext.language);
GameInteractor_ExecuteOnSetGameLanguage();
}

that is also called when changing it from the menu bar
if (ImGui::BeginMenu("Languages")) {
UIWidgets::PaddedEnhancementCheckbox("Translate Title Screen", "gTitleScreenTranslation");
if (UIWidgets::EnhancementRadioButton("English", "gLanguages", LANGUAGE_ENG)) {
GameInteractor::Instance->ExecuteHooks<GameInteractor::OnSetGameLanguage>();
}
if (UIWidgets::EnhancementRadioButton("German", "gLanguages", LANGUAGE_GER)) {
GameInteractor::Instance->ExecuteHooks<GameInteractor::OnSetGameLanguage>();
}
if (UIWidgets::EnhancementRadioButton("French", "gLanguages", LANGUAGE_FRA)) {
GameInteractor::Instance->ExecuteHooks<GameInteractor::OnSetGameLanguage>();
}
ImGui::EndMenu();
}

it seems the actual issue is the strange mix of using the cvar and global save setting and setting the in game language based on that in a few places https://github.com/search?q=repo%3AHarbourMasters%2FShipwright%20gLanguages&type=code

for a short term fix i think moving the CVarSave() call down inside the if (languageChanged) block should work, but it'd be nice to clean this up and not use 2 things to track the language selection

@xoascf
Copy link
Contributor Author

xoascf commented Apr 7, 2024

it seems a bit odd to just thow a CVarSave() call in here

we already have a call to execute hooks when setting the language

// Persist the new language so it is not overridden on the next frame
if (languageChanged) {
CVarSetInteger("gLanguages", gSaveContext.language);
GameInteractor_ExecuteOnSetGameLanguage();
}

that is also called when changing it from the menu bar

if (ImGui::BeginMenu("Languages")) {
UIWidgets::PaddedEnhancementCheckbox("Translate Title Screen", "gTitleScreenTranslation");
if (UIWidgets::EnhancementRadioButton("English", "gLanguages", LANGUAGE_ENG)) {
GameInteractor::Instance->ExecuteHooks<GameInteractor::OnSetGameLanguage>();
}
if (UIWidgets::EnhancementRadioButton("German", "gLanguages", LANGUAGE_GER)) {
GameInteractor::Instance->ExecuteHooks<GameInteractor::OnSetGameLanguage>();
}
if (UIWidgets::EnhancementRadioButton("French", "gLanguages", LANGUAGE_FRA)) {
GameInteractor::Instance->ExecuteHooks<GameInteractor::OnSetGameLanguage>();
}
ImGui::EndMenu();
}

it seems the actual issue is the strange mix of using the cvar and global save setting and setting the in game language based on that in a few places https://github.com/search?q=repo%3AHarbourMasters%2FShipwright%20gLanguages&type=code

for a short term fix i think moving the CVarSave() call down inside the if (languageChanged) block should work, but it'd be nice to clean this up and not use 2 things to track the language selection

That was the first iteration of my changes before I created this PR, but CVarSave() should only be called if the player confirms and exits the options menu screen with the A button, otherwise the other settings won't be saved.

@briaguya-ai
Copy link
Contributor

that definitely makes things odd, because it seems with the current logic you could update the language in the in-game options menu, then change something unrelated in the menu bar and have the language saved without choosing to save it in-game

not something this PR is introducing, just messy language logic that's been around for a while

Copy link
Contributor

@briaguya-ai briaguya-ai left a comment

Choose a reason for hiding this comment

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

it seems actually cleaning this up will be decently big, so i made an issue for that #4044

for now let's just :shipit:

@briaguya-ai briaguya-ai merged commit 32288be into HarbourMasters:develop-macready Apr 19, 2024
8 checks passed
@xoascf xoascf deleted the save-language-cvar-macready branch April 19, 2024 18:58
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