Remove superfluous 'sane' flag from version_info

This also removes the accompanying version_info::not_sane_exception
exception type, as well as the version_info::good() method.

The flag is never naturally set to false during version_info
construction/parsing and actual invalid version numbers are always some
variation of "0.0.0foo" once reserialized. The few cases in code where
the flag is forced to be false can be trivially adapted to not require
its existence. In effect, the associated exception was never thrown and
the good() method always returned true, save for the aforementioned
exceptions.

All in all, this was a preposterous waste of time and indentation
levels.
This commit is contained in:
Ignacio R. Morelle 2015-04-02 23:20:56 -03:00
parent 5c70bd2ffb
commit 02a31a5b89
10 changed files with 74 additions and 127 deletions

View file

@ -32,32 +32,27 @@ addon_tracking_info get_addon_tracking_info(const addon_info& addon)
t.installed_version = version_info(0, 0, 0, false);
if(is_addon_installed(id)) {
try {
if(t.can_publish) {
// Try to obtain the version number from the .pbl first.
config pbl;
get_addon_pbl_info(id, pbl);
if(t.can_publish) {
// Try to obtain the version number from the .pbl first.
config pbl;
get_addon_pbl_info(id, pbl);
if(pbl.has_attribute("version")) {
t.installed_version = pbl["version"].str();
}
} else {
// We normally use the _info.cfg version instead.
t.installed_version = get_addon_version_info(id);
if(pbl.has_attribute("version")) {
t.installed_version = pbl["version"].str();
}
} else {
// We normally use the _info.cfg version instead.
t.installed_version = get_addon_version_info(id);
}
const version_info& remote_version = addon.version;
const version_info& remote_version = addon.version;
if(remote_version == t.installed_version) {
t.state = ADDON_INSTALLED;
} else if(remote_version > t.installed_version) {
t.state = ADDON_INSTALLED_UPGRADABLE;
} else /* if(remote_version < t.installed_version) */ {
t.state = ADDON_INSTALLED_OUTDATED;
}
} catch(version_info::not_sane_exception const&) {
LOG_AC << "local add-on " << id << " has invalid or missing version info, skipping from updates check...\n";
t.state = ADDON_NOT_TRACKED;
if(remote_version == t.installed_version) {
t.state = ADDON_INSTALLED;
} else if(remote_version > t.installed_version) {
t.state = ADDON_INSTALLED_UPGRADABLE;
} else /* if(remote_version < t.installed_version) */ {
t.state = ADDON_INSTALLED_OUTDATED;
}
} else {
t.state = ADDON_NONE;

View file

@ -596,10 +596,6 @@ void server::handle_upload(const server::request& req)
std::string message = "Add-on accepted.";
if(!version_info(upload["version"]).good()) {
message += "\n\nNote: The version you specified is invalid. This add-on will be ignored for automatic update checks.";
}
if(campaign == NULL) {
campaign = &campaigns().add_child("campaign");
(*campaign)["original_timestamp"] = upload_ts;

View file

@ -315,10 +315,6 @@ namespace game_config
if (s.has_attribute("ready_for_start")) ready_for_start = s["ready_for_start"].str();
if (s.has_attribute("game_has_begun")) game_has_begun = s["game_has_begun"].str();
}
assert(wesnoth_version.good());
assert(min_savegame_version.good());
assert(test_version.good());
}
void add_color_info(const config &v)

View file

@ -407,11 +407,8 @@ void game_config_manager::load_addons_cfg()
{
BOOST_FOREACH(config & cfg, umc_cfg.child_range(*type)) {
cfg["addon_id"] = addon.addon_id;
if (addon.version.good()) {
// If the addon string was not "sane" then reject it, we can't compare non-sane version strings.
// This may also happen if no version info could be found.
cfg["addon_version"] = addon.version.str(); // Note that this may reformat the string in a canonical form.
}
// Note that this may reformat the string in a canonical form.
cfg["addon_version"] = addon.version.str();
}
}

View file

@ -619,59 +619,54 @@ void gamebrowser::populate_game_item_map_info(gamebrowser::game_item & item, con
// versions are compatible. If it's not, a record is made in the req_list passed as argument.
static mp::ADDON_REQ check_addon_version_compatibility (const config & local_item, const config & game, std::vector<required_addon> & req_list)
{
try {
if (local_item.has_attribute("addon_id") && local_item.has_attribute("addon_version")) {
if (const config & game_req = game.find_child("addon", "id", local_item["addon_id"])) {
// Record object which we will potentially store for this check
required_addon r;
r.addon_id = local_item["addon_id"].str();
if (local_item.has_attribute("addon_id") && local_item.has_attribute("addon_version")) {
if (const config & game_req = game.find_child("addon", "id", local_item["addon_id"])) {
// Record object which we will potentially store for this check
required_addon r;
r.addon_id = local_item["addon_id"].str();
const version_info local_ver(local_item["addon_version"].str());
version_info local_min_ver(local_item.has_attribute("addon_min_version") ? local_item["addon_min_version"] : local_item["addon_version"]);
// If UMC didn't specify last compatible version, assume no backwards compatibility.
if (local_min_ver > local_ver) {
// Some sanity checking regarding min version. If the min ver doens't make sense, ignore it.
local_min_ver = local_ver;
}
const version_info local_ver(local_item["addon_version"].str());
version_info local_min_ver(local_item.has_attribute("addon_min_version") ? local_item["addon_min_version"] : local_item["addon_version"]);
// If UMC didn't specify last compatible version, assume no backwards compatibility.
if (local_min_ver > local_ver) {
// Some sanity checking regarding min version. If the min ver doens't make sense, ignore it.
local_min_ver = local_ver;
}
const version_info remote_ver(game_req["version"].str());
version_info remote_min_ver(game_req.has_attribute("min_version") ? game_req["min_version"] : game_req["version"]);
if (remote_min_ver > remote_ver) {
remote_min_ver = remote_ver;
}
const version_info remote_ver(game_req["version"].str());
version_info remote_min_ver(game_req.has_attribute("min_version") ? game_req["min_version"] : game_req["version"]);
if (remote_min_ver > remote_ver) {
remote_min_ver = remote_ver;
}
// Check if the host is too out of date to play.
if (local_min_ver > remote_ver) {
r.outcome = CANNOT_SATISFY;
// Check if the host is too out of date to play.
if (local_min_ver > remote_ver) {
r.outcome = CANNOT_SATISFY;
utils::string_map symbols;
symbols["addon"] = r.addon_id; // TODO: Figure out how to ask the add-on manager for the user-friendly name of this add-on.
symbols["host_ver"] = remote_ver.str();
symbols["local_ver"] = local_ver.str();
r.message = vgettext("Host's version of $addon is too old: host's version $host_ver < your version $local_ver.", symbols);
req_list.push_back(r);
return r.outcome;
}
utils::string_map symbols;
symbols["addon"] = r.addon_id; // TODO: Figure out how to ask the add-on manager for the user-friendly name of this add-on.
symbols["host_ver"] = remote_ver.str();
symbols["local_ver"] = local_ver.str();
r.message = vgettext("Host's version of $addon is too old: host's version $host_ver < your version $local_ver.", symbols);
req_list.push_back(r);
return r.outcome;
}
// Check if our version is too out of date to play.
if (remote_min_ver > local_ver) {
r.outcome = NEED_DOWNLOAD;
// Check if our version is too out of date to play.
if (remote_min_ver > local_ver) {
r.outcome = NEED_DOWNLOAD;
utils::string_map symbols;
symbols["addon"] = r.addon_id; // TODO: Figure out how to ask the add-on manager for the user-friendly name of this add-on.
symbols["host_ver"] = remote_ver.str();
symbols["local_ver"] = local_ver.str();
r.message = vgettext("Your version of $addon is out of date: host's version $host_ver > your version $local_ver.", symbols);
req_list.push_back(r);
return r.outcome;
}
utils::string_map symbols;
symbols["addon"] = r.addon_id; // TODO: Figure out how to ask the add-on manager for the user-friendly name of this add-on.
symbols["host_ver"] = remote_ver.str();
symbols["local_ver"] = local_ver.str();
r.message = vgettext("Your version of $addon is out of date: host's version $host_ver > your version $local_ver.", symbols);
req_list.push_back(r);
return r.outcome;
}
}
} catch (version_info::not_sane_exception &) {
WRN_MP << "Caught a version_info not_sane_exception when checking version compatability.\n";
DBG_MP << "WML item:\n***\n" << local_item.debug() << "\n***\n\n";
DBG_MP << "Game Record:\n***\n" << game.debug() << "\n***\n\n";
}
return SATISFIED;
}

View file

@ -185,22 +185,18 @@ void mp_game_settings::update_addon_requirements(const config & cfg) {
if (it != addons.end()) {
addon_version_info & addon = it->second;
try {
if (new_data.version) {
if (!addon.version || (*addon.version != *new_data.version)) {
WRN_NG << "Addon version data mismatch -- not all local WML has same version of '" << cfg["id"].str() << "' addon.\n";
}
}
if (addon.version && !new_data.version) {
if (new_data.version) {
if (!addon.version || (*addon.version != *new_data.version)) {
WRN_NG << "Addon version data mismatch -- not all local WML has same version of '" << cfg["id"].str() << "' addon.\n";
}
if (new_data.min_version) {
if (!addon.min_version || (*new_data.min_version > *addon.min_version)) {
addon.min_version = *new_data.min_version;
}
}
if (addon.version && !new_data.version) {
WRN_NG << "Addon version data mismatch -- not all local WML has same version of '" << cfg["id"].str() << "' addon.\n";
}
if (new_data.min_version) {
if (!addon.min_version || (*new_data.min_version > *addon.min_version)) {
addon.min_version = *new_data.min_version;
}
} catch (version_info::not_sane_exception &) {
WRN_NG << "Caught a version_info not_sane_exception when determining a scenario's add-on dependencies. addon_id = " << cfg["id"].str() << "\n";
}
} else {
// Didn't find this addon-id in the map, so make a new entry.

View file

@ -273,15 +273,6 @@ bool loadgame::check_version_compatibility(const version_info & save_version, CV
}
const version_info &wesnoth_version = game_config::wesnoth_version;
// If the version isn't good, it probably isn't a compatible stable one,
// and the following comparisons would throw.
if (!save_version.good()) {
const std::string message = _("The save has corrupt version information ($version_number|) and cannot be loaded.");
utils::string_map symbols;
symbols["version_number"] = save_version.str();
gui2::show_error_message(video, utils::interpolate_variables_into_string(message, &symbols));
return false;
}
// Even minor version numbers indicate stable releases which are
// compatible with each other.

View file

@ -21,10 +21,6 @@ BOOST_AUTO_TEST_SUITE( version )
BOOST_AUTO_TEST_CASE( test_version_info )
{
version_info invalid(0,0,0,false,'!',"d'oh");
BOOST_CHECK( !invalid.good() );
version_info canonical("1.2.3");
BOOST_CHECK( canonical.is_canonical() );

View file

@ -20,13 +20,13 @@
#include <functional>
version_info::version_info()
: nums_(3,0), special_(""), special_separator_('\0'), sane_(true)
: nums_(3,0), special_(""), special_separator_('\0')
{
}
version_info::version_info(unsigned int major, unsigned int minor, unsigned int revision_level, bool sane,
version_info::version_info(unsigned int major, unsigned int minor, unsigned int revision_level,
char special_separator, const std::string& special)
: nums_(3,0), special_(special), special_separator_(special_separator), sane_(sane)
: nums_(3,0), special_(special), special_separator_(special_separator)
{
nums_[0] = major;
nums_[1] = minor;
@ -37,7 +37,6 @@ version_info::version_info(const std::string& str)
: nums_(3,0)
, special_("")
, special_separator_('\0')
, sane_(true)
{
std::string v = str;
utils::strip(v);
@ -83,15 +82,8 @@ version_info::version_info(const std::string& str)
nums_.resize(s, 0);
}
try {
for(size_t i = 0; (i < s); ++i) {
nums_[i] = lexical_cast<unsigned int>(components[i]);
}
}
catch(bad_lexical_cast const&) {
// FIXME: actually, this is virtually impossible to occur
// due to the find_first_not_of() call above...
sane_ = false;
for(size_t i = 0; (i < s); ++i) {
nums_[i] = lexical_cast_default<unsigned int>(components[i]);
}
}
@ -144,7 +136,7 @@ unsigned int version_info::revision_level() const {
}
bool version_info::is_canonical() const {
return nums_.size() <= 3 && sane_;
return nums_.size() <= 3;
}
namespace {
@ -187,8 +179,6 @@ namespace {
#endif
bool version_numbers_comparison_internal(const version_info& l, const version_info& r, COMP_TYPE o)
{
if((!l.good()) || !r.good()) throw version_info::not_sane_exception();
std::vector<unsigned int> lc = l.components();
std::vector<unsigned int> rc = r.components();

View file

@ -30,17 +30,13 @@
class version_info
{
public:
/** Thrown when trying to compare an non-sane version_info object. */
struct not_sane_exception {};
version_info(); /**< Default constructor. */
version_info(const std::string&); /**< String constructor. */
/** Simple list constructor. */
version_info(unsigned int major, unsigned int minor, unsigned int revision_level, bool sane = true,
version_info(unsigned int major, unsigned int minor, unsigned int revision_level,
char special_separator='\0', const std::string& special=std::string());
bool good() const { return this->sane_; }
bool is_canonical() const;
// Good old setters and getters for this class. Their names should be
@ -132,7 +128,6 @@ private:
std::vector<unsigned int> nums_;
std::string special_;
char special_separator_;
bool sane_;
};
/** Equality operator for version_info. */