From b6eb009e3255f9f0be4a562bbdd912548de8daec Mon Sep 17 00:00:00 2001 From: Marko Lindqvist Date: Wed, 14 Dec 2022 01:37:34 +0200 Subject: [PATCH 40/40] ruleset.c: Stop processing after failure in actions loading When ruleset loading encountered an error on processing action enablers, it often still tried to do some further actions. See osdn #46196 Signed-off-by: Marko Lindqvist --- server/ruleset.c | 289 ++++++++++++++++++++++++----------------------- 1 file changed, 147 insertions(+), 142 deletions(-) diff --git a/server/ruleset.c b/server/ruleset.c index 401012257a..f30526bf37 100644 --- a/server/ruleset.c +++ b/server/ruleset.c @@ -6341,11 +6341,13 @@ static bool load_action_range_max(struct rscompat_info *compat, ruleset_error(NULL, LOG_ERROR, "Bad actions.%s", action_max_range_ruleset_var_name(act)); action_by_number(act)->max_distance = action_max_range_default(paction->result); + return FALSE; } } action_by_number(act)->max_distance = max_range; + return TRUE; } @@ -6987,58 +6989,58 @@ static bool load_ruleset_game(struct section_file *file, bool act, "auto_attack"); if (reqs == NULL) { ok = FALSE; - } - - requirement_vector_copy(&auto_perf->reqs, reqs); + } else { + requirement_vector_copy(&auto_perf->reqs, reqs); - if (!load_action_auto_actions(file, auto_perf, - "auto_attack.attack_actions", - filename)) { - /* Failed to load auto attack actions */ - ruleset_error(NULL, LOG_ERROR, - "\"%s\": %s: failed load %s.", - filename, "auto_attack", "attack_actions"); - ok = FALSE; - } + if (!load_action_auto_actions(file, auto_perf, + "auto_attack.attack_actions", + filename)) { + /* Failed to load auto attack actions */ + ruleset_error(NULL, LOG_ERROR, + "\"%s\": %s: failed load %s.", + filename, "auto_attack", "attack_actions"); + ok = FALSE; + } - if (compat->compat_mode) { - enum unit_type_flag_id *protecor_flag; - size_t psize; + if (ok && compat->compat_mode) { + enum unit_type_flag_id *protecor_flag; + size_t psize; - if (secfile_entry_lookup(file, "%s", "auto_attack.will_never")) { - protecor_flag = + if (secfile_entry_lookup(file, "%s", "auto_attack.will_never")) { + protecor_flag = secfile_lookup_enum_vec(file, &psize, unit_type_flag_id, "%s", "auto_attack.will_never"); - if (!protecor_flag) { - /* Entity exists but couldn't read it. */ - ruleset_error(NULL, LOG_ERROR, - "\"%s\": %s: bad unit type flag list.", - filename, "auto_attack.will_never"); + if (!protecor_flag) { + /* Entity exists but couldn't read it. */ + ruleset_error(NULL, LOG_ERROR, + "\"%s\": %s: bad unit type flag list.", + filename, "auto_attack.will_never"); - ok = FALSE; + ok = FALSE; + } + } else { + psize = 0; + protecor_flag = NULL; } - } else { - psize = 0; - protecor_flag = NULL; - } - if (!rscompat_auto_attack_3_1(compat, auto_perf, - psize, protecor_flag)) { - /* Upgrade failed */ - ruleset_error(NULL, LOG_ERROR, - "\"%s\": %s: failed to upgrade.", - filename, "auto_attack"); - ok = FALSE; - } + if (ok && !rscompat_auto_attack_3_1(compat, auto_perf, + psize, protecor_flag)) { + /* Upgrade failed */ + ruleset_error(NULL, LOG_ERROR, + "\"%s\": %s: failed to upgrade.", + filename, "auto_attack"); + ok = FALSE; + } - if (psize) { - FC_FREE(protecor_flag); + if (psize) { + FC_FREE(protecor_flag); + } } } } - /* section: actions */ + /* Section: actions */ if (ok) { if (compat->compat_mode && compat->version < RSFORMAT_3_1) { int force_capture_units, force_bombard, force_explode_nuclear; @@ -7046,16 +7048,15 @@ static bool load_ruleset_game(struct section_file *file, bool act, if (secfile_lookup_bool_default(file, FALSE, "actions.force_trade_route")) { /* Forbid entering the marketplace when a trade route can be - * established. */ + * established. */ BV_SET(action_by_number(ACTION_MARKETPLACE)->blocked_by, ACTION_TRADE_ROUTE); } /* Forbid bombarding, exploading nuclear or attacking when it is - * legal to capture units. */ + * legal to capture units. */ force_capture_units - = secfile_lookup_bool_default(file, - FALSE, + = secfile_lookup_bool_default(file, FALSE, "actions.force_capture_units"); if (force_capture_units) { @@ -7086,7 +7087,7 @@ static bool load_ruleset_game(struct section_file *file, bool act, } /* Forbid exploding nuclear or attacking when it is legal to - * bombard. */ + * bombard. */ force_bombard = secfile_lookup_bool_default(file, FALSE, "actions.force_bombard"); @@ -7150,9 +7151,8 @@ static bool load_ruleset_game(struct section_file *file, bool act, /* Forbid attacking when it is legal to do explode nuclear. */ force_explode_nuclear - = secfile_lookup_bool_default(file, - FALSE, - "actions.force_explode_nuclear"); + = secfile_lookup_bool_default(file, FALSE, + "actions.force_explode_nuclear"); if (force_explode_nuclear) { BV_SET(action_by_number(ACTION_SUICIDE_ATTACK)->blocked_by, @@ -7196,112 +7196,121 @@ static bool load_ruleset_game(struct section_file *file, bool act, action_iterate(act_id) { struct action *paction = action_by_number(act_id); + if (!load_action_blocked_by_list(file, filename, paction)) { ok = FALSE; + break; } } action_iterate_end; - if (!lookup_bv_actions(file, filename, - &game.info.diplchance_initial_odds, - "actions.diplchance_initial_odds")) { + if (ok && !lookup_bv_actions(file, filename, + &game.info.diplchance_initial_odds, + "actions.diplchance_initial_odds")) { ok = FALSE; } - /* If the "Poison City" action or the "Poison City Escape" action - * should empty the granary. */ - /* TODO: empty granary and reduce population should become separate - * action effect flags when actions are generalized. */ - game.info.poison_empties_food_stock - = secfile_lookup_bool_default(file, - RS_DEFAULT_POISON_EMPTIES_FOOD_STOCK, - "actions.poison_empties_food_stock"); - - /* If the "Steal Maps" action or the "Steal Maps Escape" action always - * will reveal all cities when successful. */ - game.info.steal_maps_reveals_all_cities - = secfile_lookup_bool_default(file, - RS_DEFAULT_STEAL_MAP_REVEALS_CITIES, - "actions.steal_maps_reveals_all_cities"); - - /* Allow setting certain properties for some actions before - * generalized actions. */ - action_iterate(act_id) { - if (!load_action_range(compat, file, act_id)) { - ok = FALSE; - } - if (!load_action_kind(file, act_id)) { - ok = FALSE; - } - if (!load_action_actor_consuming_always(file, act_id)) { - ok = FALSE; - } - load_action_ui_name(file, act_id, - action_ui_name_ruleset_var_name(act_id), - rscompat_action_ui_name_S3_1(compat, act_id)); - } action_iterate_end; + if (ok) { + /* If the "Poison City" action or the "Poison City Escape" action + * should empty the granary. */ + /* TODO: empty granary and reduce population should become separate + * action effect flags when actions are generalized. */ + game.info.poison_empties_food_stock + = secfile_lookup_bool_default(file, + RS_DEFAULT_POISON_EMPTIES_FOOD_STOCK, + "actions.poison_empties_food_stock"); + + /* If the "Steal Maps" action or the "Steal Maps Escape" action always + * will reveal all cities when successful. */ + game.info.steal_maps_reveals_all_cities + = secfile_lookup_bool_default(file, + RS_DEFAULT_STEAL_MAP_REVEALS_CITIES, + "actions.steal_maps_reveals_all_cities"); + + /* Allow setting certain properties for some actions before + * generalized actions. */ + action_iterate(act_id) { + if (!load_action_range(compat, file, act_id)) { + ok = FALSE; + } else if (!load_action_kind(file, act_id)) { + ok = FALSE; + } else if (!load_action_actor_consuming_always(file, act_id)) { + ok = FALSE; + } else { + load_action_ui_name(file, act_id, + action_ui_name_ruleset_var_name(act_id), + rscompat_action_ui_name_S3_1(compat, act_id)); + } + + if (!ok) { + break; + } + } action_iterate_end; - /* The quiet (don't auto generate help for) property of all actions - * live in a single enum vector. This avoids generic action - * expectations. */ - if (secfile_entry_by_path(file, "actions.quiet_actions")) { - enum gen_action *quiet_actions; - size_t asize; - int j; + /* The quiet (don't auto generate help for) property of all actions + * live in a single enum vector. This avoids generic action + * expectations. */ + if (ok && secfile_entry_by_path(file, "actions.quiet_actions")) { + enum gen_action *quiet_actions; + size_t asize; + int j; - quiet_actions = + quiet_actions = secfile_lookup_enum_vec(file, &asize, gen_action, "actions.quiet_actions"); - if (!quiet_actions) { - /* Entity exists but couldn't read it. */ - ruleset_error(NULL, LOG_ERROR, - "\"%s\": actions.quiet_actions: bad action list", - filename); + if (!quiet_actions) { + /* Entity exists but couldn't read it. */ + ruleset_error(NULL, LOG_ERROR, + "\"%s\": actions.quiet_actions: bad action list", + filename); - ok = FALSE; - } + ok = FALSE; + } else { + for (j = 0; j < asize; j++) { + /* Don't auto generate help text for this action. */ + action_by_number(quiet_actions[j])->quiet = TRUE; + } - for (j = 0; j < asize; j++) { - /* Don't auto generate help text for this action. */ - action_by_number(quiet_actions[j])->quiet = TRUE; + free(quiet_actions); + } } - free(quiet_actions); - } - - /* Hard code action sub results for now. */ - - /* Unit Enter Hut */ - action_by_result_iterate(paction, act_id, ACTRES_HUT_ENTER) { - BV_SET(paction->sub_results, ACT_SUB_RES_HUT_ENTER); - } action_by_result_iterate_end; - BV_SET(action_by_number(ACTION_PARADROP_ENTER)->sub_results, - ACT_SUB_RES_HUT_ENTER); - BV_SET(action_by_number(ACTION_PARADROP_ENTER_CONQUER)->sub_results, - ACT_SUB_RES_HUT_ENTER); - - /* Unit Frighten Hut */ - action_by_result_iterate(paction, act_id, ACTRES_HUT_FRIGHTEN) { - BV_SET(paction->sub_results, ACT_SUB_RES_HUT_FRIGHTEN); - } action_by_result_iterate_end; - BV_SET(action_by_number(ACTION_PARADROP_FRIGHTEN)->sub_results, - ACT_SUB_RES_HUT_FRIGHTEN); - BV_SET(action_by_number(ACTION_PARADROP_FRIGHTEN_CONQUER)->sub_results, - ACT_SUB_RES_HUT_FRIGHTEN); - - /* Unit May Embark */ - action_iterate(act_id) { - struct action *paction = action_by_number(act_id); + if (ok) { + /* Hard code action sub results for now. */ + + /* Unit Enter Hut */ + action_by_result_iterate(paction, act_id, ACTRES_HUT_ENTER) { + BV_SET(paction->sub_results, ACT_SUB_RES_HUT_ENTER); + } action_by_result_iterate_end; + BV_SET(action_by_number(ACTION_PARADROP_ENTER)->sub_results, + ACT_SUB_RES_HUT_ENTER); + BV_SET(action_by_number(ACTION_PARADROP_ENTER_CONQUER)->sub_results, + ACT_SUB_RES_HUT_ENTER); + + /* Unit Frighten Hut */ + action_by_result_iterate(paction, act_id, ACTRES_HUT_FRIGHTEN) { + BV_SET(paction->sub_results, ACT_SUB_RES_HUT_FRIGHTEN); + } action_by_result_iterate_end; + BV_SET(action_by_number(ACTION_PARADROP_FRIGHTEN)->sub_results, + ACT_SUB_RES_HUT_FRIGHTEN); + BV_SET(action_by_number(ACTION_PARADROP_FRIGHTEN_CONQUER)->sub_results, + ACT_SUB_RES_HUT_FRIGHTEN); + + /* Unit May Embark */ + action_iterate(act_id) { + struct action *paction = action_by_number(act_id); + + if (secfile_lookup_bool_default(file, FALSE, + "civstyle.paradrop_to_transport") + && (action_has_result(paction, ACTRES_PARADROP) + || action_has_result(paction, ACTRES_PARADROP_CONQUER))) { + BV_SET(paction->sub_results, ACT_SUB_RES_MAY_EMBARK); + } - if (secfile_lookup_bool_default(file, FALSE, - "civstyle.paradrop_to_transport") - && (action_has_result(paction, ACTRES_PARADROP) - || action_has_result(paction, ACTRES_PARADROP_CONQUER))) { - BV_SET(paction->sub_results, ACT_SUB_RES_MAY_EMBARK); + /* Embark actions will always embark, not maybe embark. */ + } action_iterate_end; } - - /* Embark actions will always embark, not maybe embark. */ - } action_iterate_end; + } } if (ok) { @@ -7313,17 +7322,12 @@ static bool load_ruleset_game(struct section_file *file, bool act, action_by_number( ACTION_SPY_BRIBE_UNIT))) { ok = FALSE; - } - - /* "Attack" */ - if (!load_action_post_success_force(file, filename, - ACTION_AUTO_POST_ATTACK, - action_by_number( - ACTION_ATTACK))) { + } else if (!load_action_post_success_force(file, filename, + ACTION_AUTO_POST_ATTACK, + action_by_number( + ACTION_ATTACK))) { ok = FALSE; } - - /* No "Suicide Attack". Can't act when dead. */ } if (ok) { @@ -7415,6 +7419,7 @@ static bool load_ruleset_game(struct section_file *file, bool act, action_enabler_add(enabler); } section_list_iterate_end; + section_list_destroy(sec); } } -- 2.35.1