Conversation
| #define loaded_mods game_manager.get_mod_manager().get_loaded_mods() | ||
| if (std::find(loaded_mods.begin(), loaded_mods.end(), &mod) != loaded_mods.end()) { | ||
| mod_info_dictionary[mod_info_loaded_key] = true; | ||
| } else { | ||
| mod_info_dictionary[mod_info_loaded_key] = false; | ||
| } | ||
| #undef loaded_mods |
There was a problem hiding this comment.
Please do not use macros for this.
There was a problem hiding this comment.
Just store it in a local variable.
When using methods to get a value, do not assume the method returns the exact same value each invocation.
Also the method might be costly.
| return FAILED; | ||
| } | ||
|
|
||
| ERR_FAIL_COND_V_MSG(!game_manager.set_roots(roots, replace), FAILED, "Failed to set dataloader roots!"); |
|
|
||
| if (!game_manager.load_mod_descriptors()) { | ||
| UtilityFunctions::push_error("Failed to load mod descriptors!"); | ||
| err = FAILED; |
There was a problem hiding this comment.
It's best to return failed here. No point continuing with a broken state.
Also this can be done using ERR_FAIL_COND_V_MSG(!game_manager.load_mod_descriptors(), FAILED, "Failed to load mod descriptors!");
There was a problem hiding this comment.
I think it could be better to load the base game if mods are broken or unavailable rather than crashing the whole game. Open to doing it either way, that was my thought process.
There was a problem hiding this comment.
Paradox games typically fail when the desired mod breaks the game.
It makes sense to me from a user perspective.
When I play a mod that is close to vanilla I might not notice that the mod failed to load if you just load vanilla instead.
If you do want to go down that road, we have to inform the user in the UI, not just the logs.
|
|
||
| if (!game_manager.load_mods(roots, replace_paths, converted_mods)) { | ||
| UtilityFunctions::push_error("Failed to load mods!"); | ||
| err = FAILED; |
There was a problem hiding this comment.
Same as comment L667, better to return here and use error macro.
| auto add_message = std::bind_front(&LoadLocalisation::add_message, LoadLocalisation::get_singleton()); | ||
| if (!game_manager.load_definitions(add_message)) { | ||
| UtilityFunctions::push_error("Failed to load defines!"); | ||
| err = FAILED; |
There was a problem hiding this comment.
If you update the above 2 cases, please also update this for consistency.
| func _ready(): | ||
| mod_info = GameSingleton.get_mod_info() | ||
| var mod_status_file := ConfigFile.new() | ||
| mod_status_file.load("user://mods.cfg") |
There was a problem hiding this comment.
I don't like ModMenu & MainMenu sharing this magic string.
Shouldn't this be part of mod manager or some class?
There was a problem hiding this comment.
This file generally feels like a lot of logic and very little UI.
Consider using a cpp class similar to the BudgetMenu to do the logic.
wvpm
left a comment
wvpm
left a comment
There was a problem hiding this comment.
Define "user://mods.cfg" once instead of thrice.
|
This is mostly superseded by #584 |
Spartan322
added
the
salvageable
3 participants
Implements an in game menu that allows for the selection and loading of available mods.