<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39678 >

On 07/09/07, Marko Lindqvist wrote:
>
>  This adds checking of requirement list sanity to ruleset load. S2_1 version.
>
>  Currently outputs only number of conflicting requirement type (not
> very user-friendly error message). Functionality to access requirement
> type names outside requirements.c is needed before this can be fixed
> (and this should be fixed before committing)

 Above fixed and ported to trunk also.

 This handles requirement lists for effects only. Other requirements
are in requirement vectors (not in lists).


 - ML

diff -Nurd -X.diff_ignore freeciv/common/effects.c freeciv/common/effects.c
--- freeciv/common/effects.c	2007-08-21 21:41:47.000000000 +0300
+++ freeciv/common/effects.c	2007-09-09 20:14:00.000000000 +0300
@@ -1019,3 +1019,21 @@
   } requirement_list_iterate_end;
 }
 
+/**************************************************************************
+  Iterate through all the effects in cache, and call callback for each.
+  This is currently not very generic implementation, as we have only one user;
+  ruleset sanity checking. If any callback returns FALSE, there is no
+  further checking and this will return FALSE.
+**************************************************************************/
+bool iterate_effect_cache(iec_cb cb)
+{
+  assert(cb != NULL);
+
+  effect_list_iterate(ruleset_cache.tracker, peffect) {
+    if (!cb(peffect)) {
+      return FALSE;
+    }
+  } effect_list_iterate_end;
+
+  return TRUE;
+}
diff -Nurd -X.diff_ignore freeciv/common/effects.h freeciv/common/effects.h
--- freeciv/common/effects.h	2007-08-13 22:23:46.000000000 +0300
+++ freeciv/common/effects.h	2007-09-09 20:14:00.000000000 +0300
@@ -223,5 +223,7 @@
 Impr_type_id ai_find_source_building(struct player *pplayer,
 				     enum effect_type effect_type);
 
-#endif  /* FC__EFFECTS_H */
+typedef bool (*iec_cb)(const struct effect*);
+bool iterate_effect_cache(iec_cb cb);
 
+#endif  /* FC__EFFECTS_H */
diff -Nurd -X.diff_ignore freeciv/common/requirements.c freeciv/common/requirements.c
--- freeciv/common/requirements.c	2007-08-13 22:23:46.000000000 +0300
+++ freeciv/common/requirements.c	2007-09-09 20:22:17.000000000 +0300
@@ -1058,3 +1058,12 @@
 
   return buf;
 }
+
+
+/****************************************************************************
+  Return untranslated name of the requirement source name.
+*****************************************************************************/
+const char *get_req_source_type_name_orig(const struct req_source *psource)
+{
+  return req_source_type_names[psource->type];
+}
diff -Nurd -X.diff_ignore freeciv/common/requirements.h freeciv/common/requirements.h
--- freeciv/common/requirements.h	2007-08-13 22:23:46.000000000 +0300
+++ freeciv/common/requirements.h	2007-09-09 20:20:49.000000000 +0300
@@ -142,4 +142,6 @@
 char *get_req_source_text(const struct req_source *psource,
 			  char *buf, size_t bufsz);
 
+const char *get_req_source_type_name_orig(const struct req_source *psource);
+
 #endif  /* FC__REQUIREMENTS_H */
diff -Nurd -X.diff_ignore freeciv/server/ruleset.c freeciv/server/ruleset.c
--- freeciv/server/ruleset.c	2007-09-01 04:51:56.000000000 +0300
+++ freeciv/server/ruleset.c	2007-09-09 21:30:12.000000000 +0300
@@ -32,6 +32,7 @@
 #include "nation.h"
 #include "packets.h"
 #include "registry.h"
+#include "requirements.h"
 #include "shared.h"
 #include "support.h"
 #include "tech.h"
@@ -3265,11 +3266,145 @@
 }
 
 /**************************************************************************
+  Check if requirement list is free of conflicting requirements.
+  max_tiles is number of tiles that can provide requirement. Value -1
+  disables checking based on number of tiles.
+
+  Returns TRUE iff everything ok.
+
+  TODO: This is based on current hardcoded range limitations.
+        - There should be method of automatically determining these
+          limitations for each requirement type
+        - This function should check also problems caused by defining
+          range to less than hardcoded max for requirement type
+**************************************************************************/
+static bool sanity_check_req_list(const struct requirement_list *preqs,
+                                  int max_tiles,
+                                  const char *list_for)
+{
+  int reqs_of_type[REQ_LAST];
+
+  /* Initialize requirement counters */
+  memset(reqs_of_type, 0, sizeof(reqs_of_type));
+
+  requirement_list_iterate(preqs, preq) {
+    int rc;
+
+    assert(preq->source.type >= 0 && preq->source.type < REQ_LAST);
+
+    /* Add to counter */
+    reqs_of_type[preq->source.type]++;
+    rc = reqs_of_type[preq->source.type];
+
+    if (rc > 1) {
+      /* Multiple requirements of same the type */
+      switch (preq->source.type) {
+       case REQ_GOV:
+       case REQ_NATION:
+       case REQ_UNITTYPE:
+       case REQ_UNITCLASS:
+       case REQ_OUTPUTTYPE:
+       case REQ_SPECIALIST:
+       case REQ_MINSIZE: /* Breaks nothing, but has no sense either */
+         /* There can be only one requirement of these types (with current
+          * range limitations)
+          * Requirements might be identical, but we consider multiple
+          * declarations error anyway. */
+
+         freelog(LOG_ERROR,
+                 "%s: Requirement list has multiple %s requirements",
+                 list_for,
+                 get_req_source_type_name_orig(&preq->source));
+         return FALSE;
+         break;
+
+       case REQ_SPECIAL:
+       case REQ_TERRAIN:
+         /* There can be only up to max_tiles requirements of these types */
+         if (max_tiles != 1 && rc > max_tiles) {
+           freelog(LOG_ERROR,
+                   "%s: Requirement list has more %s requirements than "
+                   "can ever be fullfilled.",
+                   list_for,
+                   get_req_source_type_name_orig(&preq->source));
+           return FALSE;
+         }
+         break;
+
+       case REQ_NONE:
+       case REQ_TECH:
+       case REQ_BUILDING:
+       case REQ_UNITFLAG:
+         /* Can have multiple requirements of these types */
+         break;
+       case REQ_LAST:
+         /* Should never be in requirement vector */
+         assert(FALSE);
+         return FALSE;
+         break;
+       /* No default handling here, as we want compiler warning
+        * if new requirement type is added to enum and it's not handled
+        * here. */
+      }
+    }
+  } requirement_list_iterate_end;
+
+  return TRUE;
+}
+
+/**************************************************************************
+  Check that requirement vector and negated requirements vector do not have
+  confliciting requirements.
+
+  Returns TRUE iff everything ok.
+**************************************************************************/
+static bool sanity_check_req_nreq_list(const struct requirement_list *preqs,
+                                       const struct requirement_list *pnreqs,
+                                       int one_tile,
+                                       const char *list_for)
+{
+  /* Check internal sanity of requirement list */
+  if (!sanity_check_req_list(preqs, one_tile, list_for)) {
+    return FALSE;
+  }
+
+  /* There is no pnreqs in all cases */
+  if (pnreqs != NULL) {
+    /* Check sanity between reqs and nreqs */
+    requirement_list_iterate(preqs, preq) {
+      requirement_list_iterate(pnreqs, pnreq) {
+        if (are_requirements_equal(preq, pnreq)) {
+          freelog(LOG_ERROR,
+                  "%s: Identical %s requirement in requirements and negated requirements.",
+                  list_for,
+                  get_req_source_type_name_orig(&preq->source));
+          return FALSE;
+        }
+      } requirement_list_iterate_end;
+    } requirement_list_iterate_end;
+  }
+
+  return TRUE;
+}
+
+/**************************************************************************
+  Sanity check callback for iterating effects cache.
+**************************************************************************/
+static bool effect_sanity_cb(const struct effect *peffect)
+{
+  int one_tile = -1; /* TODO: Determine correct value from effect.
+                      *       -1 disables checking */
+
+  return sanity_check_req_nreq_list(peffect->reqs, peffect->nreqs, one_tile,
+                                    effect_type_name(peffect->type));
+}
+
+/**************************************************************************
   Some more sanity checking once all rulesets are loaded. These check
   for some cross-referencing which was impossible to do while only one
   party was loaded in load_ruleset_xxx()
 
-  Returns TRUE if everything ok.
+  Returns TRUE iff everything ok.
 **************************************************************************/
 static bool sanity_check_ruleset_data(void)
 {
@@ -3340,5 +3475,15 @@
     }
   } unit_type_iterate_end;
 
+  /* Check requirement lists against conflicting requirements */
+  /* Effects */
+  if (!iterate_effect_cache(effect_sanity_cb)) {
+    exit(EXIT_FAILURE);
+  }
+  /* TODO: Check other requirement lists also
+   *       (Governments, Buildings, Specialists, City styles)
+   *       These use currently requirement_vector, when effects
+   *       (and sanity checking) use requirement_list. */
+
   return TRUE;
 }
diff -Nurd -X.diff_ignore freeciv/common/effects.c freeciv/common/effects.c
--- freeciv/common/effects.c	2007-08-13 20:51:04.000000000 +0300
+++ freeciv/common/effects.c	2007-09-09 20:43:48.000000000 +0300
@@ -1053,3 +1053,21 @@
   } requirement_list_iterate_end;
 }
 
+/**************************************************************************
+  Iterate through all the effects in cache, and call callback for each.
+  This is currently not very generic implementation, as we have only one user;
+  ruleset sanity checking. If any callback returns FALSE, there is no
+  further checking and this will return FALSE.
+**************************************************************************/
+bool iterate_effect_cache(iec_cb cb)
+{
+  assert(cb != NULL);
+
+  effect_list_iterate(ruleset_cache.tracker, peffect) {
+    if (!cb(peffect)) {
+      return FALSE;
+    }
+  } effect_list_iterate_end;
+
+  return TRUE;
+}
diff -Nurd -X.diff_ignore freeciv/common/effects.h freeciv/common/effects.h
--- freeciv/common/effects.h	2007-08-13 22:44:48.000000000 +0300
+++ freeciv/common/effects.h	2007-09-09 20:43:48.000000000 +0300
@@ -226,5 +226,7 @@
 Impr_type_id ai_find_source_building(struct player *pplayer,
 				     enum effect_type effect_type);
 
-#endif  /* FC__EFFECTS_H */
+typedef bool (*iec_cb)(const struct effect*);
+bool iterate_effect_cache(iec_cb cb);
 
+#endif  /* FC__EFFECTS_H */
diff -Nurd -X.diff_ignore freeciv/common/requirements.c freeciv/common/requirements.c
--- freeciv/common/requirements.c	2007-08-13 20:51:04.000000000 +0300
+++ freeciv/common/requirements.c	2007-09-09 20:49:14.000000000 +0300
@@ -1270,6 +1270,14 @@
   return buf;
 }
 
+/****************************************************************************
+  Return untranslated name of the universal source name.
+*****************************************************************************/
+const char *universal_type_rule_name(const struct universal *psource)
+{
+  return universal_names[psource->kind];
+}
+
 /**************************************************************************
   Return the number of shields it takes to build this universal.
 **************************************************************************/
diff -Nurd -X.diff_ignore freeciv/common/requirements.h freeciv/common/requirements.h
--- freeciv/common/requirements.h	2007-08-11 00:20:33.000000000 +0300
+++ freeciv/common/requirements.h	2007-09-09 20:49:05.000000000 +0300
@@ -111,6 +111,7 @@
 const char *universal_rule_name(const struct universal *psource);
 const char *universal_name_translation(const struct universal *psource,
 				       char *buf, size_t bufsz);
+const char *universal_type_rule_name(const struct universal *psource);
 
 int universal_build_shield_cost(const struct universal *target);
 
diff -Nurd -X.diff_ignore freeciv/server/ruleset.c freeciv/server/ruleset.c
--- freeciv/server/ruleset.c	2007-09-01 04:51:45.000000000 +0300
+++ freeciv/server/ruleset.c	2007-09-09 21:31:28.000000000 +0300
@@ -34,6 +34,7 @@
 #include "nation.h"
 #include "packets.h"
 #include "registry.h"
+#include "requirements.h"
 #include "shared.h"
 #include "support.h"
 #include "tech.h"
@@ -3599,11 +3600,148 @@
 }
 
 /**************************************************************************
+  Check if requirement list is free of conflicting requirements.
+  max_tiles is number of tiles that can provide requirement. Value -1
+  disables checking based on number of tiles.
+
+  Returns TRUE iff everything ok.
+
+  TODO: This is based on current hardcoded range limitations.
+        - There should be method of automatically determining these
+          limitations for each requirement type
+        - This function should check also problems caused by defining
+          range to less than hardcoded max for requirement type
+**************************************************************************/
+static bool sanity_check_req_list(const struct requirement_list *preqs,
+                                  int max_tiles,
+                                  const char *list_for)
+{
+  int reqs_of_type[VUT_LAST];
+
+  /* Initialize requirement counters */
+  memset(reqs_of_type, 0, sizeof(reqs_of_type));
+
+  requirement_list_iterate(preqs, preq) {
+    int rc;
+
+    assert(preq->source.kind >= 0 && preq->source.kind < VUT_LAST);
+
+    /* Add to counter */
+    reqs_of_type[preq->source.kind]++;
+    rc = reqs_of_type[preq->source.kind];
+
+    if (rc > 1) {
+      /* Multiple requirements of same the type */
+      switch (preq->source.kind) {
+       case VUT_GOVERNMENT:
+       case VUT_NATION:
+       case VUT_UTYPE:
+       case VUT_UCLASS:
+       case VUT_OTYPE:
+       case VUT_SPECIALIST:
+       case VUT_MINSIZE: /* Breaks nothing, but has no sense either */
+       case VUT_AI_LEVEL:
+       case VUT_TERRAINCLASS:
+         /* There can be only one requirement of these types (with current
+          * range limitations)
+          * Requirements might be identical, but we consider multiple
+          * declarations error anyway. */
+
+         freelog(LOG_ERROR,
+                 "%s: Requirement list has multiple %s requirements",
+                 list_for,
+                 universal_type_rule_name(&preq->source));
+         return FALSE;
+         break;
+
+       case VUT_SPECIAL:
+       case VUT_TERRAIN:
+         /* There can be only up to max_tiles requirements of these types */
+         if (max_tiles != 1 && rc > max_tiles) {
+           freelog(LOG_ERROR,
+                   "%s: Requirement list has more %s requirements than "
+                   "can ever be fullfilled.",
+                   list_for,
+                   universal_type_rule_name(&preq->source));
+           return FALSE;
+         }
+         break;
+
+       case VUT_NONE:
+       case VUT_ADVANCE:
+       case VUT_IMPROVEMENT:
+       case VUT_UTFLAG:
+       case VUT_UCFLAG:
+         /* Can have multiple requirements of these types */
+         break;
+       case VUT_LAST:
+         /* Should never be in requirement vector */
+         assert(FALSE);
+         return FALSE;
+         break;
+       /* No default handling here, as we want compiler warning
+        * if new requirement type is added to enum and it's not handled
+        * here. */
+      }
+    }
+  } requirement_list_iterate_end;
+
+  return TRUE;
+}
+
+/**************************************************************************
+  Check that requirement vector and negated requirements vector do not have
+  confliciting requirements.
+
+  Returns TRUE iff everything ok.
+**************************************************************************/
+static bool sanity_check_req_nreq_list(const struct requirement_list *preqs,
+                                       const struct requirement_list *pnreqs,
+                                       int one_tile,
+                                       const char *list_for)
+{
+  /* Check internal sanity of requirement list */
+  if (!sanity_check_req_list(preqs, one_tile, list_for)) {
+    return FALSE;
+  }
+
+  /* There is no pnreqs in all cases */
+  if (pnreqs != NULL) {
+    /* Check sanity between reqs and nreqs */
+    requirement_list_iterate(preqs, preq) {
+      requirement_list_iterate(pnreqs, pnreq) {
+        if (are_requirements_equal(preq, pnreq)) {
+          freelog(LOG_ERROR,
+                  "%s: Identical %s requirement in requirements and negated requirements.",
+                  list_for,
+                  universal_type_rule_name(&preq->source));
+          return FALSE;
+        }
+      } requirement_list_iterate_end;
+    } requirement_list_iterate_end;
+  }
+
+  return TRUE;
+}
+
+/**************************************************************************
+  Sanity check callback for iterating effects cache.
+**************************************************************************/
+static bool effect_sanity_cb(const struct effect *peffect)
+{
+  int one_tile = -1; /* TODO: Determine correct value from effect.
+                      *       -1 disables checking */
+
+  return sanity_check_req_nreq_list(peffect->reqs, peffect->nreqs, one_tile,
+                                    effect_type_name(peffect->type));
+}
+
+/**************************************************************************
   Some more sanity checking once all rulesets are loaded. These check
   for some cross-referencing which was impossible to do while only one
   party was loaded in load_ruleset_xxx()
 
-  Returns TRUE if everything ok.
+  Returns TRUE iff everything ok.
 **************************************************************************/
 static bool sanity_check_ruleset_data(void)
 {
@@ -3679,5 +3817,15 @@
     }
   } unit_type_iterate_end;
 
+  /* Check requirement lists against conflicting requirements */
+  /* Effects */
+  if (!iterate_effect_cache(effect_sanity_cb)) {
+    exit(EXIT_FAILURE);
+  }
+  /* TODO: Check other requirement lists also
+   *       (Governments, Buildings, Specialists, City styles)
+   *       These use currently requirement_vector, when effects
+   *       (and sanity checking) use requirement_list. */
+
   return TRUE;
 }
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to