Author: jtn Date: Tue Oct 4 11:06:30 2016 New Revision: 33983 URL: http://svn.gna.org/viewcvs/freeciv?rev=33983&view=rev Log: Give more useful diagnostic for invalid requirement type/value in ruleset.
See bug #25137. Modified: trunk/common/requirements.c Modified: trunk/common/requirements.c URL: http://svn.gna.org/viewcvs/freeciv/trunk/common/requirements.c?rev=33983&r1=33982&r2=33983&view=diff ============================================================================== --- trunk/common/requirements.c (original) +++ trunk/common/requirements.c Tue Oct 4 11:06:30 2016 @@ -657,25 +657,138 @@ const char *value) { struct requirement req; - bool invalid = TRUE; + bool invalid; const char *error = NULL; req.source = universal_by_rule_name(type, value); - /* Scan the range string to find the range. If no range is given a - * default fallback is used rather than giving an error. */ - req.range = req_range_by_name(range, fc_strcasecmp); - if (!req_range_is_valid(req.range)) { + invalid = !universals_n_is_valid(req.source.kind); + if (invalid) { + error = "bad type or name"; + } else { + /* Scan the range string to find the range. If no range is given a + * default fallback is used rather than giving an error. */ + req.range = req_range_by_name(range, fc_strcasecmp); + if (!req_range_is_valid(req.range)) { + switch (req.source.kind) { + case VUT_NONE: + case VUT_COUNT: + break; + case VUT_IMPROVEMENT: + case VUT_IMPR_GENUS: + case VUT_EXTRA: + case VUT_GOOD: + case VUT_TERRAIN: + case VUT_TERRFLAG: + case VUT_UTYPE: + case VUT_UTFLAG: + case VUT_UCLASS: + case VUT_UCFLAG: + case VUT_MINVETERAN: + case VUT_UNITSTATE: + case VUT_MINMOVES: + case VUT_MINHP: + case VUT_AGE: + case VUT_ACTION: + case VUT_OTYPE: + case VUT_SPECIALIST: + case VUT_TERRAINCLASS: + case VUT_BASEFLAG: + case VUT_ROADFLAG: + case VUT_EXTRAFLAG: + case VUT_TERRAINALTER: + case VUT_CITYTILE: + case VUT_MAXTILEUNITS: + req.range = REQ_RANGE_LOCAL; + break; + case VUT_MINSIZE: + case VUT_MINCULTURE: + case VUT_NATIONALITY: + req.range = REQ_RANGE_CITY; + break; + case VUT_GOVERNMENT: + case VUT_ACHIEVEMENT: + case VUT_STYLE: + case VUT_ADVANCE: + case VUT_TECHFLAG: + case VUT_NATION: + case VUT_NATIONGROUP: + case VUT_DIPLREL: + case VUT_AI_LEVEL: + req.range = REQ_RANGE_PLAYER; + break; + case VUT_MINYEAR: + case VUT_TOPO: + case VUT_MINTECHS: + req.range = REQ_RANGE_WORLD; + break; + } + } + + req.survives = survives; + req.present = present; + req.quiet = quiet; + + /* These checks match what combinations are supported inside + * is_req_active(). However, it's only possible to do basic checks, + * not anything that might depend on the rest of the ruleset which + * might not have been loaded yet. */ switch (req.source.kind) { - case VUT_NONE: - case VUT_COUNT: + case VUT_TERRAIN: + case VUT_EXTRA: + case VUT_TERRAINCLASS: + case VUT_TERRFLAG: + case VUT_BASEFLAG: + case VUT_ROADFLAG: + case VUT_EXTRAFLAG: + invalid = (req.range != REQ_RANGE_LOCAL + && req.range != REQ_RANGE_CADJACENT + && req.range != REQ_RANGE_ADJACENT + && req.range != REQ_RANGE_CITY + && req.range != REQ_RANGE_TRADEROUTE); break; - case VUT_IMPROVEMENT: - case VUT_IMPR_GENUS: - case VUT_EXTRA: + case VUT_ADVANCE: + case VUT_TECHFLAG: + case VUT_ACHIEVEMENT: + case VUT_MINTECHS: + invalid = (req.range < REQ_RANGE_PLAYER); + break; + case VUT_GOVERNMENT: + case VUT_AI_LEVEL: + case VUT_STYLE: + invalid = (req.range != REQ_RANGE_PLAYER); + break; + case VUT_MINSIZE: + case VUT_NATIONALITY: case VUT_GOOD: - case VUT_TERRAIN: - case VUT_TERRFLAG: + invalid = (req.range != REQ_RANGE_CITY + && req.range != REQ_RANGE_TRADEROUTE); + break; + case VUT_MINCULTURE: + invalid = (req.range != REQ_RANGE_CITY + && req.range != REQ_RANGE_TRADEROUTE + && req.range != REQ_RANGE_PLAYER + && req.range != REQ_RANGE_TEAM + && req.range != REQ_RANGE_ALLIANCE + && req.range != REQ_RANGE_WORLD); + break; + case VUT_DIPLREL: + invalid = (req.range != REQ_RANGE_LOCAL + && req.range != REQ_RANGE_PLAYER + && req.range != REQ_RANGE_TEAM + && req.range != REQ_RANGE_ALLIANCE + && req.range != REQ_RANGE_WORLD) + /* Non local foreign makes no sense. */ + || (req.source.value.diplrel == DRO_FOREIGN + && req.range != REQ_RANGE_LOCAL); + break; + case VUT_NATION: + case VUT_NATIONGROUP: + invalid = (req.range != REQ_RANGE_PLAYER + && req.range != REQ_RANGE_TEAM + && req.range != REQ_RANGE_ALLIANCE + && req.range != REQ_RANGE_WORLD); + break; case VUT_UTYPE: case VUT_UTFLAG: case VUT_UCLASS: @@ -684,156 +797,49 @@ case VUT_UNITSTATE: case VUT_MINMOVES: case VUT_MINHP: - case VUT_AGE: case VUT_ACTION: case VUT_OTYPE: case VUT_SPECIALIST: - case VUT_TERRAINCLASS: - case VUT_BASEFLAG: - case VUT_ROADFLAG: - case VUT_EXTRAFLAG: - case VUT_TERRAINALTER: + case VUT_TERRAINALTER: /* XXX could in principle support C/ADJACENT */ + invalid = (req.range != REQ_RANGE_LOCAL); + break; case VUT_CITYTILE: case VUT_MAXTILEUNITS: - req.range = REQ_RANGE_LOCAL; - break; - case VUT_MINSIZE: - case VUT_MINCULTURE: - case VUT_NATIONALITY: - req.range = REQ_RANGE_CITY; - break; - case VUT_GOVERNMENT: - case VUT_ACHIEVEMENT: - case VUT_STYLE: - case VUT_ADVANCE: - case VUT_TECHFLAG: - case VUT_NATION: - case VUT_NATIONGROUP: - case VUT_DIPLREL: - case VUT_AI_LEVEL: - req.range = REQ_RANGE_PLAYER; + invalid = (req.range != REQ_RANGE_LOCAL + && req.range != REQ_RANGE_CADJACENT + && req.range != REQ_RANGE_ADJACENT); break; case VUT_MINYEAR: case VUT_TOPO: - case VUT_MINTECHS: - req.range = REQ_RANGE_WORLD; + invalid = (req.range != REQ_RANGE_WORLD); break; - } - } - - req.survives = survives; - req.present = present; - req.quiet = quiet; - - /* These checks match what combinations are supported inside - * is_req_active(). However, it's only possible to do basic checks, - * not anything that might depend on the rest of the ruleset which - * might not have been loaded yet. */ - switch (req.source.kind) { - case VUT_TERRAIN: - case VUT_EXTRA: - case VUT_TERRAINCLASS: - case VUT_TERRFLAG: - case VUT_BASEFLAG: - case VUT_ROADFLAG: - case VUT_EXTRAFLAG: - invalid = (req.range != REQ_RANGE_LOCAL - && req.range != REQ_RANGE_CADJACENT - && req.range != REQ_RANGE_ADJACENT - && req.range != REQ_RANGE_CITY - && req.range != REQ_RANGE_TRADEROUTE); - break; - case VUT_ADVANCE: - case VUT_TECHFLAG: - case VUT_ACHIEVEMENT: - case VUT_MINTECHS: - invalid = (req.range < REQ_RANGE_PLAYER); - break; - case VUT_GOVERNMENT: - case VUT_AI_LEVEL: - case VUT_STYLE: - invalid = (req.range != REQ_RANGE_PLAYER); - break; - case VUT_MINSIZE: - case VUT_NATIONALITY: - case VUT_GOOD: - invalid = (req.range != REQ_RANGE_CITY - && req.range != REQ_RANGE_TRADEROUTE); - break; - case VUT_MINCULTURE: - invalid = (req.range != REQ_RANGE_CITY - && req.range != REQ_RANGE_TRADEROUTE - && req.range != REQ_RANGE_PLAYER - && req.range != REQ_RANGE_TEAM - && req.range != REQ_RANGE_ALLIANCE - && req.range != REQ_RANGE_WORLD); - break; - case VUT_DIPLREL: - invalid = (req.range != REQ_RANGE_LOCAL - && req.range != REQ_RANGE_PLAYER - && req.range != REQ_RANGE_TEAM - && req.range != REQ_RANGE_ALLIANCE - && req.range != REQ_RANGE_WORLD) - /* Non local foreign makes no sense. */ - || (req.source.value.diplrel == DRO_FOREIGN - && req.range != REQ_RANGE_LOCAL); - break; - case VUT_NATION: - case VUT_NATIONGROUP: - invalid = (req.range != REQ_RANGE_PLAYER - && req.range != REQ_RANGE_TEAM - && req.range != REQ_RANGE_ALLIANCE - && req.range != REQ_RANGE_WORLD); - break; - case VUT_UTYPE: - case VUT_UTFLAG: - case VUT_UCLASS: - case VUT_UCFLAG: - case VUT_MINVETERAN: - case VUT_UNITSTATE: - case VUT_MINMOVES: - case VUT_MINHP: - case VUT_ACTION: - case VUT_OTYPE: - case VUT_SPECIALIST: - case VUT_TERRAINALTER: /* XXX could in principle support C/ADJACENT */ - invalid = (req.range != REQ_RANGE_LOCAL); - break; - case VUT_CITYTILE: - case VUT_MAXTILEUNITS: - invalid = (req.range != REQ_RANGE_LOCAL - && req.range != REQ_RANGE_CADJACENT - && req.range != REQ_RANGE_ADJACENT); - break; - case VUT_MINYEAR: - case VUT_TOPO: - invalid = (req.range != REQ_RANGE_WORLD); - break; - case VUT_AGE: - /* FIXME: could support TRADEROUTE, TEAM, etc */ - invalid = (req.range != REQ_RANGE_LOCAL - && req.range != REQ_RANGE_CITY - && req.range != REQ_RANGE_PLAYER); - break; - case VUT_IMPR_GENUS: - /* TODO: Support other ranges too. */ - invalid = req.range != REQ_RANGE_LOCAL; - break; - case VUT_IMPROVEMENT: - /* Valid ranges depend on the building genus (wonder/improvement), - * which might not have been loaded from the ruleset yet. - * So we allow anything here, and do a proper check once ruleset - * loading is complete, in sanity_check_req_individual(). */ - case VUT_NONE: - invalid = FALSE; - break; - case VUT_COUNT: - break; - } - - if (invalid) { - error = "bad range"; - } else { + case VUT_AGE: + /* FIXME: could support TRADEROUTE, TEAM, etc */ + invalid = (req.range != REQ_RANGE_LOCAL + && req.range != REQ_RANGE_CITY + && req.range != REQ_RANGE_PLAYER); + break; + case VUT_IMPR_GENUS: + /* TODO: Support other ranges too. */ + invalid = req.range != REQ_RANGE_LOCAL; + break; + case VUT_IMPROVEMENT: + /* Valid ranges depend on the building genus (wonder/improvement), + * which might not have been loaded from the ruleset yet. + * So we allow anything here, and do a proper check once ruleset + * loading is complete, in sanity_check_req_individual(). */ + case VUT_NONE: + invalid = FALSE; + break; + case VUT_COUNT: + break; + } + if (invalid) { + error = "bad range"; + } + } + + if (!invalid) { /* Check 'survives'. */ switch (req.source.kind) { case VUT_IMPROVEMENT: _______________________________________________ Freeciv-commits mailing list Freeciv-commits@gna.org https://mail.gna.org/listinfo/freeciv-commits