Skip to content

Commit

Permalink
MP Lobby: actually properly display required addon names
Browse files Browse the repository at this point in the history
Turns out I implemented ba9274e incorrectly. I had naively thought
that the name= key in both instances (required addon parsing and version compatibility check) was
the right key to use. I was wrong.

In the case of required addon parsing, it turns out there was no name= key in the first place, so
the "Missing addon: foo" message was using the addon id. In the case of the version compatibility
check, it was using the *content* name, not the *addon* name. For example, it was displaying the name
of a given modification shipped with an addon, but not the actual addon that shipped it. That was
definitely not what we wanted.

To fix this issue, I've added a name field to the config for required addons. This is populated with
the addon_title= key (see below) of the required addon when the host creates a game. Do note the list
of required addons is sent over the network. In the case of the version compatibility check, I made
use of the same addon_title= key, except taken from the player's local config (this is because the
version compatibility check only happens if you already have the given modification/era/etc installed
from an addon).

The addon_title= key itself is added to an era/mod/scenario/campaign/etc's config when addons are
loaded, same as the addon_id= and addon_version= key. Currently it takes its value from _info.cfg's
[info] title= field. If that isn't present, it falls back to addon_id=.
  • Loading branch information
Vultraz committed Feb 15, 2018
1 parent 908b936 commit 1e3508f
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 29 deletions.
39 changes: 29 additions & 10 deletions src/game_config_manager.cpp
Expand Up @@ -339,9 +339,11 @@ void game_config_manager::load_game_config(FORCE_RELOAD_CONFIG force_reload,
paths_manager_.set_paths(game_config());
}

struct addon_source {
struct addon_source
{
std::string main_cfg;
std::string addon_id;
std::string addon_title;
version_info version;
};

Expand Down Expand Up @@ -394,21 +396,37 @@ void game_config_manager::load_addons_cfg()
addon.main_cfg = main_cfg;
addon.addon_id = addon_id;

if (filesystem::file_exists(main_cfg)) {
if (filesystem::file_exists(info_cfg)) {
if(filesystem::file_exists(main_cfg)) {
if(filesystem::file_exists(info_cfg)) {
config info;
cache_.get_config(info_cfg, info);
const config info_tag = info.child_or_empty("info");

const config& info_tag = info.child_or_empty("info");

std::string core = info_tag["core"];
if (core.empty()) core = "default";
if ( !info_tag.empty() && // Don't skip addons which have no [info], they are most likely manually installed.
info_tag["type"] != "core" && // Don't skip cores, we want them selectable at all times.
core != preferences::core_id() // Don't skip addons matching our current core.
) {
continue; // Skip add-ons not matching our current core.
if(core.empty()) {
core = "default";
}

addon.addon_title = info_tag["title"];

// Skip add-ons not matching our current core.
//
// Don't skip:
// - addons which have no [info], they are most likely manually installed.
// - cores, we want them selectable at all times.
// - addons matching our current core.
if(!info_tag.empty() && info_tag["type"] != "core" && core != preferences::core_id()) {
continue;
}
}

// Fall back to ID if no title was found (most likely in the case of missing _info.cfg).
// TODO: account for case where _info.cfg is missing but a .pbl file is present.
if(addon.addon_title.empty()) {
addon.addon_title = addon_id;
}

// Ask the addon manager to find version info for us (from info, pbl file)
addon.version = get_addon_version_info(addon_id);
addons_to_load.push_back(addon);
Expand All @@ -429,6 +447,7 @@ void game_config_manager::load_addons_cfg()
if(tags_with_addon_id.count(child.key) > 0) {
auto& cfg = child.cfg;
cfg["addon_id"] = addon.addon_id;
cfg["addon_title"] = addon.addon_title;
// Note that this may reformat the string in a canonical form.
cfg["addon_version"] = addon.version.str();
}
Expand Down
10 changes: 2 additions & 8 deletions src/game_initialization/lobby_data.cpp
Expand Up @@ -474,18 +474,12 @@ game_info::ADDON_REQ game_info::check_addon_version_compatibility(const config&

remote_min_ver = std::min(remote_min_ver, remote_ver);

// Use content name if provided, else fall back on the addon id.
// TODO: should this use t_string?
const std::string content_name = local_item.has_attribute("name")
? local_item["name"].str()
: local_item["addon_id"].str();

// Check if the host is too out of date to play.
if(local_min_ver > remote_ver) {
r.outcome = CANNOT_SATISFY;

r.message = vgettext("The host's version of <i>$addon</i> is incompatible. They have version <b>$host_ver</b> while you have version <b>$local_ver</b>.", {
{"addon", content_name},
{"addon", local_item["addon_title"]},
{"host_ver", remote_ver.str()},
{"local_ver", local_ver.str()}
});
Expand All @@ -499,7 +493,7 @@ game_info::ADDON_REQ game_info::check_addon_version_compatibility(const config&
r.outcome = NEED_DOWNLOAD;

r.message = vgettext("Your version of <i>$addon</i> is incompatible. You have version <b>$local_ver</b> while the host has version <b>$host_ver</b>.", {
{"addon", content_name},
{"addon", local_item["addon_title"]},
{"host_ver", remote_ver.str()},
{"local_ver", local_ver.str()}
});
Expand Down
3 changes: 3 additions & 0 deletions src/mp_game_settings.cpp
Expand Up @@ -145,6 +145,7 @@ config mp_game_settings::to_config() const
mp_game_settings::addon_version_info::addon_version_info(const config & cfg)
: version()
, min_version()
, name(cfg["name"])
{
if (!cfg["version"].empty()) {
version = cfg["version"].str();
Expand All @@ -161,6 +162,8 @@ void mp_game_settings::addon_version_info::write(config & cfg) const {
if (min_version) {
cfg["min_version"] = *min_version;
}

cfg["name"] = name;
}

void mp_game_settings::update_addon_requirements(const config & cfg) {
Expand Down
14 changes: 10 additions & 4 deletions src/mp_game_settings.hpp
Expand Up @@ -72,17 +72,23 @@ struct mp_game_settings

config options;

struct addon_version_info {
struct addon_version_info
{
boost::optional<version_info> version;
boost::optional<version_info> min_version;

std::string name;

explicit addon_version_info(const config &);
void write(config &) const;
};

std::map<std::string, addon_version_info> addons; // the key is the addon_id

// Takes a config with addon metadata (id =, version =, min_version =), formatted similarly to how mp_game_settings is written that is,
// and adds this as a requirement, updating the min_version if there was already an entry for this addon_id.
void update_addon_requirements(const config & addon_cfg);
/**
* Takes a config with addon metadata (id, name, version, min_version) and adds
* it as a requirement for this game. It also updates min_version if there was
* already an entry for this addon_id.
*/
void update_addon_requirements(const config& addon_cfg);
};
23 changes: 16 additions & 7 deletions src/saved_game.cpp
Expand Up @@ -233,11 +233,14 @@ void saved_game::expand_scenario()

// Add addon_id information if it exists.
if(!scenario["addon_id"].empty() && scenario["require_scenario"].to_bool(false)) {
mp_settings_.update_addon_requirements(config {
"id", scenario["addon_id"],
"version", scenario["addon_version"],
"min_version", scenario["addon_min_version"]
});
config required_scenario;

required_scenario["id"] = scenario["addon_id"];
required_scenario["name"] = scenario["addon_title"];
required_scenario["version"] = scenario["addon_version"];
required_scenario["min_version"] = scenario["addon_min_version"];

mp_settings_.update_addon_requirements(required_scenario);
}

update_label();
Expand Down Expand Up @@ -282,8 +285,14 @@ void saved_game::load_mod(const std::string& type, const std::string& id)
bool require_default = (type == "era");

if(!cfg["addon_id"].empty() && cfg[require_attr].to_bool(require_default)) {
mp_settings_.update_addon_requirements(config{
"id", cfg["addon_id"], "version", cfg["addon_version"], "min_version", cfg["addon_min_version"]});
config required_mod;

required_mod["id"] = cfg["addon_id"];
required_mod["name"] = cfg["addon_title"];
required_mod["version"] = cfg["addon_version"];
required_mod["min_version"] = cfg["addon_min_version"];

mp_settings_.update_addon_requirements(required_mod);
}

// Copy events
Expand Down

0 comments on commit 1e3508f

Please sign in to comment.