From d1686849e0c3114227afc98a0fad37b8da870fe7 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 12 Sep 2015 16:20:46 +0200 Subject: [PATCH 1/6] refactoring: move produceexp into unit module, for want of a better one. add a bit of test coverage. https://bugs.eressea.de/view.php?id=2137 - fix learning-by-doing with less than u->number people. --- conf/e4/config.json | 1 + src/kernel/config.c | 10 ---------- src/kernel/unit.c | 19 +++++++++++++++++++ src/kernel/unit.h | 3 ++- src/kernel/unit.test.c | 25 +++++++++++++++++++++++++ 5 files changed, 47 insertions(+), 11 deletions(-) diff --git a/conf/e4/config.json b/conf/e4/config.json index a32d7aab7..894f6e3f9 100644 --- a/conf/e4/config.json +++ b/conf/e4/config.json @@ -29,6 +29,7 @@ "recruit.allow_merge": true, "study.expensivemigrants": true, "study.speedup": 2, + "study.from_use": 2, "world.era": 3, "rules.migrants.max": 0, "rules.reserve.twophase": true, diff --git a/src/kernel/config.c b/src/kernel/config.c index f31083130..dbc2d5c10 100644 --- a/src/kernel/config.c +++ b/src/kernel/config.c @@ -1599,16 +1599,6 @@ int maintenance_cost(const struct unit *u) return u_race(u)->maintenance * u->number; } -int produceexp(struct unit *u, skill_t sk, int n) -{ - if (global.producexpchance > 0.0F) { - if (n == 0 || !playerrace(u_race(u))) - return 0; - learn_skill(u, sk, global.producexpchance); - } - return 0; -} - int lovar(double xpct_x2) { int n = (int)(xpct_x2 * 500) + 1; diff --git a/src/kernel/unit.c b/src/kernel/unit.c index 95608e709..df97d2f41 100644 --- a/src/kernel/unit.c +++ b/src/kernel/unit.c @@ -1933,3 +1933,22 @@ bool unit_name_equals_race(const unit *u) { bool unit_can_study(const unit *u) { return !((u_race(u)->flags & RCF_NOLEARN) || fval(u, UFL_WERE)); } + +static double produceexp_chance(void) { + return global.producexpchance; +} + +void produceexp_ex(struct unit *u, skill_t sk, int n, void(*learn)(unit *, skill_t, double)) +{ + if (n != 0 && playerrace(u_race(u))) { + double chance = produceexp_chance(); + if (chance > 0.0F) { + learn(u, sk, (n * chance) / u->number); + } + } +} + +void produceexp(struct unit *u, skill_t sk, int n) +{ + produceexp_ex(u, sk, n, learn_skill); +} diff --git a/src/kernel/unit.h b/src/kernel/unit.h index dcb7ad6e3..7a2f3e80f 100644 --- a/src/kernel/unit.h +++ b/src/kernel/unit.h @@ -161,8 +161,9 @@ extern "C" { struct skill *unit_skill(const struct unit *u, skill_t id); bool has_skill(const unit * u, skill_t sk); int effskill(const struct unit *u, skill_t sk, const struct region *r); - int produceexp(struct unit *u, skill_t sk, int n); int SkillCap(skill_t sk); + void produceexp(struct unit *u, skill_t sk, int n); + void produceexp_ex(struct unit *u, skill_t sk, int n, void(*learn)(unit *, skill_t, double)); void set_level(struct unit *u, skill_t id, int level); int get_level(const struct unit *u, skill_t id); diff --git a/src/kernel/unit.test.c b/src/kernel/unit.test.c index 8dffbca3f..7106db7c2 100644 --- a/src/kernel/unit.test.c +++ b/src/kernel/unit.test.c @@ -350,6 +350,30 @@ static void test_age_familiar(CuTest *tc) { test_cleanup(); } +static CuTest *g_tc; + +static void cb_learn_one(unit *u, skill_t sk, double chance) { + CuAssertIntEquals(g_tc, SK_ALCHEMY, sk); + CuAssertDblEquals(g_tc, global.producexpchance / u->number, chance, 0.01); +} + +static void cb_learn_two(unit *u, skill_t sk, double chance) { + CuAssertIntEquals(g_tc, SK_ALCHEMY, sk); + CuAssertDblEquals(g_tc, 2 * global.producexpchance / u->number, chance, 0.01); +} + +static void test_produceexp(CuTest *tc) { + unit *u; + + g_tc = tc; + test_cleanup(); + u = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); + global.producexpchance = 1.0; + produceexp_ex(u, SK_ALCHEMY, 1, cb_learn_one); + produceexp_ex(u, SK_ALCHEMY, 2, cb_learn_two); + test_cleanup(); +} + CuSuite *get_unit_suite(void) { CuSuite *suite = CuSuiteNew(); @@ -368,5 +392,6 @@ CuSuite *get_unit_suite(void) SUITE_ADD_TEST(suite, test_skill_hunger); SUITE_ADD_TEST(suite, test_skill_familiar); SUITE_ADD_TEST(suite, test_age_familiar); + SUITE_ADD_TEST(suite, test_produceexp); return suite; } From 68c448b3fbc805ccc3df93c9d93d125b8af0a1ae Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 12 Sep 2015 16:29:57 +0200 Subject: [PATCH 2/6] fix signature of callback --- conf/e4/config.json | 2 +- src/kernel/unit.c | 2 +- src/kernel/unit.h | 2 +- src/kernel/unit.test.c | 6 ++++-- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/conf/e4/config.json b/conf/e4/config.json index 894f6e3f9..831b973a8 100644 --- a/conf/e4/config.json +++ b/conf/e4/config.json @@ -29,7 +29,7 @@ "recruit.allow_merge": true, "study.expensivemigrants": true, "study.speedup": 2, - "study.from_use": 2, + "study.from_use": 0.4, "world.era": 3, "rules.migrants.max": 0, "rules.reserve.twophase": true, diff --git a/src/kernel/unit.c b/src/kernel/unit.c index df97d2f41..e44c024dd 100644 --- a/src/kernel/unit.c +++ b/src/kernel/unit.c @@ -1938,7 +1938,7 @@ static double produceexp_chance(void) { return global.producexpchance; } -void produceexp_ex(struct unit *u, skill_t sk, int n, void(*learn)(unit *, skill_t, double)) +void produceexp_ex(struct unit *u, skill_t sk, int n, bool (*learn)(unit *, skill_t, double)) { if (n != 0 && playerrace(u_race(u))) { double chance = produceexp_chance(); diff --git a/src/kernel/unit.h b/src/kernel/unit.h index 7a2f3e80f..3aef7bd0f 100644 --- a/src/kernel/unit.h +++ b/src/kernel/unit.h @@ -163,7 +163,7 @@ extern "C" { int effskill(const struct unit *u, skill_t sk, const struct region *r); int SkillCap(skill_t sk); void produceexp(struct unit *u, skill_t sk, int n); - void produceexp_ex(struct unit *u, skill_t sk, int n, void(*learn)(unit *, skill_t, double)); + void produceexp_ex(struct unit *u, skill_t sk, int n, bool (*learn)(unit *, skill_t, double)); void set_level(struct unit *u, skill_t id, int level); int get_level(const struct unit *u, skill_t id); diff --git a/src/kernel/unit.test.c b/src/kernel/unit.test.c index 7106db7c2..93c1cb95d 100644 --- a/src/kernel/unit.test.c +++ b/src/kernel/unit.test.c @@ -352,14 +352,16 @@ static void test_age_familiar(CuTest *tc) { static CuTest *g_tc; -static void cb_learn_one(unit *u, skill_t sk, double chance) { +static bool cb_learn_one(unit *u, skill_t sk, double chance) { CuAssertIntEquals(g_tc, SK_ALCHEMY, sk); CuAssertDblEquals(g_tc, global.producexpchance / u->number, chance, 0.01); + return false; } -static void cb_learn_two(unit *u, skill_t sk, double chance) { +static bool cb_learn_two(unit *u, skill_t sk, double chance) { CuAssertIntEquals(g_tc, SK_ALCHEMY, sk); CuAssertDblEquals(g_tc, 2 * global.producexpchance / u->number, chance, 0.01); + return false; } static void test_produceexp(CuTest *tc) { From 9bdc811582394fc5389b205cf54a3f905ac9adff Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 12 Sep 2015 16:38:39 +0200 Subject: [PATCH 3/6] produceexp uses a json-configurable constant make get_param_flt return a double, floats are for wimps. --- src/battle.c | 4 ++-- src/chaos.c | 2 +- src/kernel/config.c | 4 ++-- src/kernel/config.h | 2 +- src/kernel/unit.c | 2 +- src/kernel/unit.test.c | 6 +++--- src/laws.c | 6 +++--- src/monsters.c | 2 +- src/move.c | 4 ++-- src/randenc.c | 6 +++--- 10 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/battle.c b/src/battle.c index 075e2327c..3770199f6 100644 --- a/src/battle.c +++ b/src/battle.c @@ -2895,10 +2895,10 @@ static void aftermath(battle * b) if (sh && fval(sh, SF_DAMAGED)) { int n = b->turn - 2; if (n > 0) { - float dmg = + double dmg = get_param_flt(global.parameters, "rules.ship.damage.battleround", 0.05F); - damage_ship(sh, dmg * (float)n); + damage_ship(sh, dmg * n); freset(sh, SF_DAMAGED); } } diff --git a/src/chaos.c b/src/chaos.c index df478e0f4..5b6f9c89b 100644 --- a/src/chaos.c +++ b/src/chaos.c @@ -191,7 +191,7 @@ static void chaos(region * r) while (sh) { ship *nsh = sh->next; - float dmg = + double dmg = get_param_flt(global.parameters, "rules.ship.damage.atlantis", 0.50); damage_ship(sh, dmg); diff --git a/src/kernel/config.c b/src/kernel/config.c index dbc2d5c10..1bd2e6c21 100644 --- a/src/kernel/config.c +++ b/src/kernel/config.c @@ -1123,10 +1123,10 @@ void set_basepath(const char *path) g_basedir = path; } -float get_param_flt(const struct param *p, const char *key, float def) +double get_param_flt(const struct param *p, const char *key, double def) { const char *str = get_param(p, key); - return str ? (float)atof(str) : def; + return str ? atof(str) : def; } void set_param(struct param **p, const char *key, const char *data) diff --git a/src/kernel/config.h b/src/kernel/config.h index 23d887b99..c8fc106e6 100644 --- a/src/kernel/config.h +++ b/src/kernel/config.h @@ -284,7 +284,7 @@ extern "C" { const char *get_param(const struct param *p, const char *key); int get_param_int(const struct param *p, const char *key, int def); int check_param(const struct param *p, const char *key, const char *searchvalue); - float get_param_flt(const struct param *p, const char *key, float def); + double get_param_flt(const struct param *p, const char *key, double def); void free_params(struct param **pp); bool ExpensiveMigrants(void); diff --git a/src/kernel/unit.c b/src/kernel/unit.c index e44c024dd..5fb841d9a 100644 --- a/src/kernel/unit.c +++ b/src/kernel/unit.c @@ -1935,7 +1935,7 @@ bool unit_can_study(const unit *u) { } static double produceexp_chance(void) { - return global.producexpchance; + return get_param_flt(global.parameters, "study.from_use", 1.0 / 3); } void produceexp_ex(struct unit *u, skill_t sk, int n, bool (*learn)(unit *, skill_t, double)) diff --git a/src/kernel/unit.test.c b/src/kernel/unit.test.c index 93c1cb95d..91333aa74 100644 --- a/src/kernel/unit.test.c +++ b/src/kernel/unit.test.c @@ -354,13 +354,13 @@ static CuTest *g_tc; static bool cb_learn_one(unit *u, skill_t sk, double chance) { CuAssertIntEquals(g_tc, SK_ALCHEMY, sk); - CuAssertDblEquals(g_tc, global.producexpchance / u->number, chance, 0.01); + CuAssertDblEquals(g_tc, 0.5 / u->number, chance, 0.01); return false; } static bool cb_learn_two(unit *u, skill_t sk, double chance) { CuAssertIntEquals(g_tc, SK_ALCHEMY, sk); - CuAssertDblEquals(g_tc, 2 * global.producexpchance / u->number, chance, 0.01); + CuAssertDblEquals(g_tc, 2 * 0.5 / u->number, chance, 0.01); return false; } @@ -370,7 +370,7 @@ static void test_produceexp(CuTest *tc) { g_tc = tc; test_cleanup(); u = test_create_unit(test_create_faction(0), test_create_region(0, 0, 0)); - global.producexpchance = 1.0; + set_param(&global.parameters, "study.from_use", "0.5"); produceexp_ex(u, SK_ALCHEMY, 1, cb_learn_one); produceexp_ex(u, SK_ALCHEMY, 2, cb_learn_two); test_cleanup(); diff --git a/src/laws.c b/src/laws.c index fce556ec7..41a0e72f3 100755 --- a/src/laws.c +++ b/src/laws.c @@ -2762,14 +2762,14 @@ void sinkships(struct region * r) if (fval(r->terrain, SEA_REGION)) { if (!enoughsailors(sh, crew_skill(sh))) { // ship is at sea, but not enough people to control it - float dmg = get_param_flt(global.parameters, + double dmg = get_param_flt(global.parameters, "rules.ship.damage.nocrewocean", 0.30F); damage_ship(sh, dmg); } } else if (!ship_owner(sh)) { // any ship lying around without an owner slowly rots - float dmg = get_param_flt(global.parameters, "rules.ship.damage.nocrew", 0.05F); + double dmg = get_param_flt(global.parameters, "rules.ship.damage.nocrew", 0.05F); damage_ship(sh, dmg); } } @@ -3496,7 +3496,7 @@ static int use_item(unit * u, const item_type * itype, int amount, struct order static double heal_factor(const unit * u) { - static float elf_regen = -1; + static double elf_regen = -1; switch (old_race(u_race(u))) { case RC_TROLL: case RC_DAEMON: diff --git a/src/monsters.c b/src/monsters.c index e010ee7ff..8de81c4c0 100644 --- a/src/monsters.c +++ b/src/monsters.c @@ -80,7 +80,7 @@ static void give_peasants(unit *u, const item_type *itype, int reduce) { unit_addorder(u, parse_order(buf, u->faction->locale)); } -static float monster_attack_chance(void) { +static double monster_attack_chance(void) { return get_param_flt(global.parameters, "rules.monsters.attack_chance", 0.4f); } diff --git a/src/move.c b/src/move.c index e1207f0fa..f5da898cc 100644 --- a/src/move.c +++ b/src/move.c @@ -704,7 +704,7 @@ static float damage_drift(void) { static float value = -1.0F; if (value < 0) { - value = get_param_flt(global.parameters, "rules.ship.damage_drift", 0.02F); + value = (float)get_param_flt(global.parameters, "rules.ship.damage_drift", 0.02F); } return value; } @@ -1955,7 +1955,7 @@ sail(unit * u, order * ord, bool move_on_land, region_list ** routep) ADDMSG(&f->msgs, msg_message("sailnolandingstorm", "ship region", sh, next_point)); } else { - float dmg = + double dmg = get_param_flt(global.parameters, "rules.ship.damage.nolanding", 0.10F); ADDMSG(&f->msgs, msg_message("sailnolanding", "ship region", sh, diff --git a/src/randenc.c b/src/randenc.c index cdf7b9380..801aac4c4 100644 --- a/src/randenc.c +++ b/src/randenc.c @@ -746,7 +746,7 @@ static void move_iceberg(region * r) for (sh = r->ships; sh; sh = sh->next) { /* Meldung an Kapitän */ - float dmg = + double dmg = get_param_flt(global.parameters, "rules.ship.damage.intoiceberg", 0.10F); damage_ship(sh, dmg); @@ -759,7 +759,7 @@ static void move_iceberg(region * r) translist(&rc->buildings, &r->buildings, rc->buildings); } while (rc->ships) { - float dmg = + double dmg = get_param_flt(global.parameters, "rules.ship.damage.withiceberg", 0.10F); fset(rc->ships, SF_SELECT); @@ -893,7 +893,7 @@ static void godcurse(void) ship *sh; for (sh = r->ships; sh;) { ship *shn = sh->next; - float dmg = + double dmg = get_param_flt(global.parameters, "rules.ship.damage.godcurse", 0.10F); damage_ship(sh, dmg); From 8a95ea0c0025b7a5b8e8f067b0167b44da3d46ab Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 12 Sep 2015 16:41:22 +0200 Subject: [PATCH 4/6] speeding up produceexp_chance --- src/kernel/config.h | 2 +- src/kernel/unit.c | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/kernel/config.h b/src/kernel/config.h index c8fc106e6..ade8bfa63 100644 --- a/src/kernel/config.h +++ b/src/kernel/config.h @@ -261,7 +261,7 @@ extern "C" { unsigned int data_turn; struct param *parameters; void *vm_state; - float producexpchance; + double producexpchance; int cookie; int data_version; /* TODO: eliminate in favor of gamedata.version */ struct _dictionary_ *inifile; diff --git a/src/kernel/unit.c b/src/kernel/unit.c index 5fb841d9a..696015aef 100644 --- a/src/kernel/unit.c +++ b/src/kernel/unit.c @@ -1935,7 +1935,12 @@ bool unit_can_study(const unit *u) { } static double produceexp_chance(void) { - return get_param_flt(global.parameters, "study.from_use", 1.0 / 3); + static int update = 0; + if (update != global.cookie) { + global.producexpchance = get_param_flt(global.parameters, "study.from_use", 1.0 / 3); + update = global.cookie; + } + return global.producexpchance; } void produceexp_ex(struct unit *u, skill_t sk, int n, bool (*learn)(unit *, skill_t, double)) From d65e9aaf94a88f1b57778fd5d20a21a41c6f6f77 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 12 Sep 2015 16:42:02 +0200 Subject: [PATCH 5/6] eliminate xml reading of produceexp chance --- src/kernel/xmlreader.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/kernel/xmlreader.c b/src/kernel/xmlreader.c index b10825dbd..d87caaef7 100644 --- a/src/kernel/xmlreader.c +++ b/src/kernel/xmlreader.c @@ -2064,9 +2064,6 @@ static int parse_main(xmlDocPtr doc) if (nodes->nodeNr > 0) { xmlNodePtr node = nodes->nodeTab[0]; - global.producexpchance = - (float)xml_fvalue(node, "learningbydoing", 1.0 / 3); - propValue = xmlGetProp(node, BAD_CAST "name"); if (propValue != NULL) { global.gamename = _strdup((const char *)propValue); From 7906cdbcb6450fc71d253bbebf372d97f33590a1 Mon Sep 17 00:00:00 2001 From: Enno Rehling Date: Sat, 12 Sep 2015 16:49:18 +0200 Subject: [PATCH 6/6] do not read gamename from XML, read it from JSON only (two mechanisms for the same feature are too many) --- conf/e2/config.xml | 2 +- conf/e3/config.xml | 2 +- conf/e4/config.xml | 2 +- src/kernel/config.h | 6 ++++-- src/kernel/unit.c | 4 ++-- src/kernel/xmlreader.c | 10 +--------- 6 files changed, 10 insertions(+), 16 deletions(-) diff --git a/conf/e2/config.xml b/conf/e2/config.xml index 45870fded..98c118088 100644 --- a/conf/e2/config.xml +++ b/conf/e2/config.xml @@ -53,7 +53,7 @@ - + diff --git a/conf/e3/config.xml b/conf/e3/config.xml index 663b56d26..6a32c95da 100644 --- a/conf/e3/config.xml +++ b/conf/e3/config.xml @@ -42,7 +42,7 @@ - + diff --git a/conf/e4/config.xml b/conf/e4/config.xml index 2e68f6bf2..bbf712c09 100644 --- a/conf/e4/config.xml +++ b/conf/e4/config.xml @@ -42,7 +42,7 @@ - + diff --git a/src/kernel/config.h b/src/kernel/config.h index ade8bfa63..00518c37b 100644 --- a/src/kernel/config.h +++ b/src/kernel/config.h @@ -261,8 +261,6 @@ extern "C" { unsigned int data_turn; struct param *parameters; void *vm_state; - double producexpchance; - int cookie; int data_version; /* TODO: eliminate in favor of gamedata.version */ struct _dictionary_ *inifile; @@ -271,6 +269,10 @@ extern "C" { const struct race * rc, int in_turn); int(*maintenance) (const struct unit * u); } functions; + /* the following are some cached values, because get_param can be slow. + * you should almost never need to touch them */ + int cookie; + double producexpchance_; } settings; typedef struct helpmode { diff --git a/src/kernel/unit.c b/src/kernel/unit.c index 696015aef..a12edd310 100644 --- a/src/kernel/unit.c +++ b/src/kernel/unit.c @@ -1937,10 +1937,10 @@ bool unit_can_study(const unit *u) { static double produceexp_chance(void) { static int update = 0; if (update != global.cookie) { - global.producexpchance = get_param_flt(global.parameters, "study.from_use", 1.0 / 3); + global.producexpchance_ = get_param_flt(global.parameters, "study.from_use", 1.0 / 3); update = global.cookie; } - return global.producexpchance; + return global.producexpchance_; } void produceexp_ex(struct unit *u, skill_t sk, int n, bool (*learn)(unit *, skill_t, double)) diff --git a/src/kernel/xmlreader.c b/src/kernel/xmlreader.c index d87caaef7..c2114641d 100644 --- a/src/kernel/xmlreader.c +++ b/src/kernel/xmlreader.c @@ -2060,16 +2060,9 @@ static int parse_main(xmlDocPtr doc) xmlNodeSetPtr nodes = result->nodesetval; int i; - xmlChar *propValue; if (nodes->nodeNr > 0) { xmlNodePtr node = nodes->nodeTab[0]; - propValue = xmlGetProp(node, BAD_CAST "name"); - if (propValue != NULL) { - global.gamename = _strdup((const char *)propValue); - xmlFree(propValue); - } - xmlXPathFreeObject(result); xpath->node = node; @@ -2079,9 +2072,8 @@ static int parse_main(xmlDocPtr doc) for (i = 0; i != nodes->nodeNr; ++i) { xmlNodePtr node = nodes->nodeTab[i]; xmlChar *propName = xmlGetProp(node, BAD_CAST "name"); - bool disable = xml_bvalue(node, "disable", false); - if (disable) { + if (xml_bvalue(node, "disable", false)) { int k; for (k = 0; k != MAXKEYWORDS; ++k) { if (strcmp(keywords[k], (const char *)propName) == 0) {