libretro/scummvm
1
0
Fork 0
mirror of https://github.com/libretro/scummvm.git synced 2025-01-24 19:45:07 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

ULTIMA8: Add error checking for treasure loader

Browse Source 
This might help with bug #12182 where the comment suggests there is a problem
with loading the treasure data.  To make sure it loads properly, added a lot
more error checking and error messages, and created unit tests for the same.
This commit is contained in:
Matthew Duggan 2022-03-25 16:44:59 +09:00
parent 3d2a2cc610
commit 2345a0d3a6
4 changed files with 187 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

91
engines/ultima/ultima8/games/treasure_loader.cpp
View File

@ -41,22 +41,23 @@ void TreasureLoader::loadDefaults() {
// load default treasure types
lootkeyvals = config->listKeyValues("game", "treasure");
KeyMap::const_iterator defaultiter;
for (defaultiter = lootkeyvals.begin();
defaultiter != lootkeyvals.end(); ++defaultiter) {
TreasureInfo ti;
bool ok = internalParse(defaultiter->_value, ti, true);
const istring &key = defaultiter->_key;
const Std::string &val = defaultiter->_value;
bool ok = internalParse(val, ti, true);
if (ok) {
_defaultTreasure[defaultiter->_key] = ti;
_defaultTreasure[key] = ti;
} else {
perr << "Failed to parse treasure type '" << defaultiter->_key
<< "': " << defaultiter->_value << Std::endl;
warning("Failed to parse treasure type '%s': %s", key.c_str(), val.c_str());
}
}
}
bool TreasureLoader::parse(const Std::string &desc,
Std::vector<TreasureInfo> &treasure) {
Std::vector<TreasureInfo> &treasure) const {
treasure.clear();
Std::vector<Std::string> tr;
@ -64,7 +65,6 @@ bool TreasureLoader::parse(const Std::string &desc,
TreasureInfo ti;
for (unsigned int i = 0; i < tr.size(); ++i) {
// pout << "parse: item=" << tr[i] << Std::endl;
if (internalParse(tr[i], ti, false)) {
treasure.push_back(ti);
} else {
@ -76,40 +76,55 @@ bool TreasureLoader::parse(const Std::string &desc,
}
bool TreasureLoader::internalParse(const Std::string &desc, TreasureInfo &ti,
bool loadingDefault) {
ti._special = "";
ti._chance = 1;
ti._map = 0;
ti._shapes.clear();
ti._frames.clear();
ti._minCount = ti._maxCount = 1;
bool loadingDefault) const {
ti.clear();
bool loadedDefault = false;
Std::vector<Std::pair<Std::string, Std::string> > kv;
SplitStringKV(desc, ' ', kv);
for (unsigned int i = 0; i < kv.size(); ++i) {
Std::string key = kv[i].first;
const Std::string &key = kv[i].first;
Std::string val = kv[i].second;
// pout << "internalParse: key=" << key << " val=" << val << Std::endl;
if (key == "shape") {
if (!parseUInt32Vector(val, ti._shapes))
if (!parseUInt32Vector(val, ti._shapes)) {
warning("Failed to parse treasure shape list '%s'", val.c_str());
return false;
}
// validate the shapes are > 0 and < max shape
for (unsigned int j = 0; j < ti._shapes.size(); j++) {
if (ti._shapes[j] <= 0 || ti._shapes[j] > 65535) {
warning("Invalid treasure shape in list '%s'", val.c_str());
return false;
}
}
} else if (key == "frame") {
if (!parseUInt32Vector(val, ti._frames))
if (!parseUInt32Vector(val, ti._frames)) {
warning("Failed to parse treasure frame list '%s'", val.c_str());
return false;
}
// validate the frames are > 0 and < max frame
for (unsigned int j = 0; j < ti._frames.size(); j++) {
if (ti._frames[j] < 0 || ti._frames[j] > 65535) {
warning("Invalid treasure frame in list '%s'", val.c_str());
return false;
}
}
} else if (key == "count") {
if (!parseUIntRange(val, ti._minCount, ti._maxCount)) {
int x;
if (!parseInt(val, x))
if (!parseInt(val, x) || x <= 0) {
warning("Invalid treasure count '%s'", val.c_str());
return false;
}
ti._minCount = ti._maxCount = x;
}
} else if (key == "chance") {
if (!parseDouble(val, ti._chance))
if (!parseDouble(val, ti._chance) || ti._chance <= 0) {
warning("Invalid treasure chance '%s'", val.c_str());
return false;
}
} else if (key == "map") {
if (val.size() > 1 && val[0] == '!')
val[0] = '-'; // HACK: invert map for 'not this map'
@ -128,17 +143,23 @@ bool TreasureLoader::internalParse(const Std::string &desc, TreasureInfo &ti,
return false;
loadedDefault = true;
} else if (key == "mult" && !loadingDefault) {
if (!loadedDefault) return false;
if (!loadedDefault) {
warning("Need defaults before applying multiplier in treasure data '%s'", val.c_str());
return false;
}
unsigned int minmult, maxmult;
if (!parseUIntRange(val, minmult, maxmult)) {
int x;
if (!parseInt(val, x))
if (!parseInt(val, x) || x <= 0) {
warning("Invalid treasure multiplier '%s'", val.c_str());
return false;
}
minmult = maxmult = x;
}
ti._minCount *= minmult;
ti._maxCount *= maxmult;
} else {
warning("Unknown key parsing treasure '%s'", key.c_str());
return false;
}
}
@ -147,14 +168,16 @@ bool TreasureLoader::internalParse(const Std::string &desc, TreasureInfo &ti,
}
bool TreasureLoader::parseUInt32Vector(const Std::string &val_,
Std::vector<uint32> &vec) {
Std::vector<uint32> &vec) const {
Std::string val = val_;
vec.clear();
Std::string::size_type pos;
if (val.empty())
return false;
while (!val.empty()) {
pos = val.find(',');
Std::string item = val.substr(0, pos);
Std::string::size_type pos = val.find(',');
const Std::string item = val.substr(0, pos);
Std::string::size_type itempos = val.find('-');
if (itempos != Std::string::npos) {
@ -165,7 +188,7 @@ bool TreasureLoader::parseUInt32Vector(const Std::string &val_,
vec.push_back(i);
} else {
int x;
if (!parseInt(item, x))
if (!parseInt(item, x) || x < 0)
return false;
vec.push_back(x);
}
@ -178,7 +201,7 @@ bool TreasureLoader::parseUInt32Vector(const Std::string &val_,
}
bool TreasureLoader::parseUIntRange(const Std::string &val,
unsigned int &min, unsigned int &max) {
unsigned int &min, unsigned int &max) const {
Std::string::size_type pos = val.find('-');
if (pos == 0 || pos == Std::string::npos || pos + 1 >= val.size())
return false;
@ -186,20 +209,24 @@ bool TreasureLoader::parseUIntRange(const Std::string &val,
bool ok = true;
ok &= parseInt(val.substr(0, pos), t1);
ok &= parseInt(val.substr(pos + 1), t2);
if (ok) {
if (ok && t1 <= t2 && t1 >= 0 && t2 >= 0) {
min = t1;
max = t2;
}
return ok;
}
bool TreasureLoader::parseDouble(const Std::string &val, double &d) {
bool TreasureLoader::parseDouble(const Std::string &val, double &d) const {
if (val.empty())
return false;
// TODO: error checking
d = atof(val.c_str());
return true;
}
bool TreasureLoader::parseInt(const Std::string &val, int &i) {
bool TreasureLoader::parseInt(const Std::string &val, int &i) const {
if (val.empty())
return false;
// TODO: error checking
i = strtol(val.c_str(), 0, 0);
return true;

13
engines/ultima/ultima8/games/treasure_loader.h
View File

@ -23,6 +23,7 @@
#define ULTIMA8_GAMES_TREASURELOADER_H
#include "ultima/ultima8/world/actors/treasure_info.h"
#include "ultima/ultima8/misc/istring.h"
#include "ultima/shared/std/containers.h"
namespace Ultima {
@ -39,17 +40,17 @@ public:
void loadDefaults();
//! parse treasure string into vector of TreasureInfo objects
bool parse(const Std::string &, Std::vector<TreasureInfo> &treasure);
bool parse(const Std::string &, Std::vector<TreasureInfo> &treasure) const;
private:
TreasureMap _defaultTreasure;
bool internalParse(const Std::string &desc, TreasureInfo &ti, bool loadingDefault);
bool internalParse(const Std::string &desc, TreasureInfo &ti, bool loadingDefault) const;
bool parseUInt32Vector(const Std::string &val, Std::vector<uint32> &vec);
bool parseUIntRange(const Std::string &val, unsigned int &min, unsigned int &max);
bool parseDouble(const Std::string &val, double &d);
bool parseInt(const Std::string &val, int &i);
bool parseUInt32Vector(const Std::string &val, Std::vector<uint32> &vec) const;
bool parseUIntRange(const Std::string &val, unsigned int &min, unsigned int &max) const;
bool parseDouble(const Std::string &val, double &d) const;
bool parseInt(const Std::string &val, int &i) const;
};
} // End of namespace Ultima8

9
engines/ultima/ultima8/world/actors/treasure_info.h
View File

@ -37,6 +37,15 @@ struct TreasureInfo {
unsigned int _minCount, _maxCount;
TreasureInfo() : _chance(1), _map(0), _minCount(1), _maxCount(1) {}
void clear() {
_special.clear();
_chance = 1;
_map = 0;
_shapes.clear();
_frames.clear();
_minCount = _maxCount = 1;
}
};
} // End of namespace Ultima8

112
test/engines/ultima/ultima8/games/treasure_loader.h Normal file
View File

@ -0,0 +1,112 @@
#include <cxxtest/TestSuite.h>
#include "engines/ultima/ultima8/games/treasure_loader.h"
/**
* Test suite for the functions in engines/ultima/ultima8/games/treasure_loader.h
*
* TODO: We should test type= values, but they are loaded from the config file
* so we need a way to add those.
*
* That would also allow testing "type", "special", and "mult" values which
* need defaults.
*/
class U8TreasureLoaderTestSuite : public CxxTest::TestSuite {
public:
U8TreasureLoaderTestSuite() {
}
Ultima::Ultima8::TreasureLoader loader;
/* Parse nothing -> should return nothing */
void test_parse_empty() {
Ultima::Std::vector<Ultima::Ultima8::TreasureInfo> t;
bool result = loader.parse("", t);
TS_ASSERT(result);
TS_ASSERT(t.empty());
}
/* Parse a single treasure type */
void test_parse_basic() {
Ultima::Std::vector<Ultima::Ultima8::TreasureInfo> t;
bool result = loader.parse("shape=123,456 frame=2,3 count=4-20 map=23 chance=0.234", t);
TS_ASSERT(result);
TS_ASSERT_EQUALS(t.size(), 1);
const Ultima::Ultima8::TreasureInfo ti = t[0];
TS_ASSERT_EQUALS(ti._shapes.size(), 2);
TS_ASSERT_EQUALS(ti._shapes[0], 123);
TS_ASSERT_EQUALS(ti._shapes[1], 456);
TS_ASSERT_EQUALS(ti._frames.size(), 2);
TS_ASSERT_EQUALS(ti._frames[0], 2);
TS_ASSERT_EQUALS(ti._frames[1], 3);
TS_ASSERT_EQUALS(ti._minCount, 4);
TS_ASSERT_EQUALS(ti._maxCount, 20);
TS_ASSERT_EQUALS(ti._special, "");
TS_ASSERT_EQUALS(ti._map, 23);
TS_ASSERT_EQUALS(ti._chance, 0.234);
}
/* Parse multiple treasure types */
void test_parse_multi() {
Ultima::Std::vector<Ultima::Ultima8::TreasureInfo> t;
bool result = loader.parse("shape=123;shape=456 frame=2-5;shape=888 map=-12", t);
TS_ASSERT(result);
TS_ASSERT_EQUALS(t.size(), 3);
TS_ASSERT_EQUALS(t[0]._shapes.size(), 1);
TS_ASSERT_EQUALS(t[0]._shapes[0], 123);
TS_ASSERT_EQUALS(t[0]._frames.size(), 0);
TS_ASSERT_EQUALS(t[0]._minCount, 1);
TS_ASSERT_EQUALS(t[0]._maxCount, 1);
TS_ASSERT_EQUALS(t[0]._special, "");
TS_ASSERT_EQUALS(t[0]._map, 0);
TS_ASSERT_EQUALS(t[0]._chance, 1);
TS_ASSERT_EQUALS(t[1]._shapes.size(), 1);
TS_ASSERT_EQUALS(t[1]._shapes[0], 456);
TS_ASSERT_EQUALS(t[1]._frames.size(), 4);
TS_ASSERT_EQUALS(t[1]._frames[0], 2);
TS_ASSERT_EQUALS(t[1]._frames[1], 3);
TS_ASSERT_EQUALS(t[1]._frames[2], 4);
TS_ASSERT_EQUALS(t[1]._frames[3], 5);
TS_ASSERT_EQUALS(t[2]._shapes.size(), 1);
TS_ASSERT_EQUALS(t[2]._shapes[0], 888);
TS_ASSERT_EQUALS(t[2]._map, -12);
}
/* Check that various invalid strings don't parse */
void test_parse_invalid() {
Ultima::Std::vector<Ultima::Ultima8::TreasureInfo> t;
bool result;
result = loader.parse("shape=", t);
TS_ASSERT(!result);
result = loader.parse("what", t);
TS_ASSERT(!result);
result = loader.parse("shape=abc", t);
TS_ASSERT(!result);
result = loader.parse("shape=123,123456789", t);
TS_ASSERT(!result);
result = loader.parse("shape=-123", t);
TS_ASSERT(!result);
result = loader.parse("frame=-1,5", t);
TS_ASSERT(!result);
/* TODO: This case falls back to parsing the 10, not great.
result = loader.parse("count=10-1", t);
TS_ASSERT(!result);
*/
result = loader.parse("chance=-1", t);
TS_ASSERT(!result);
}
};