Complete.Org: Mailing Lists: Archives: freeciv-dev: April 2005:
[Freeciv-Dev] (PR#12782) cleanups to tileset first-time reading
Home

[Freeciv-Dev] (PR#12782) cleanups to tileset first-time reading

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
Subject: [Freeciv-Dev] (PR#12782) cleanups to tileset first-time reading
From: "Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>
Date: Wed, 13 Apr 2005 00:09:45 -0700
Reply-to: bugs@xxxxxxxxxxx

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

tileset_fullname is ugly.

1.  It has code that automatically falls back to the fallback tileset if 
the given tileset is not determined to be there.  But it has no real way 
to determine if the tileset it's given is there.  It can check for a 
NULL tileset or a missing filename but not until tileset_read_toplevel 
is run do we know if the tileset is usable.

2.  It sets default_tileset_name as soon as a tileset name is chosen. 
This is wrong because it means you can't call tileset_fullname on a 
tileset without having your default tileset be changed to it.

This patch cleans it up big-time.

1.  tileset_fullname is very simple, and does what you'd expect it to do.

2.  tileset_read_toplevel returns NULL if the tileset can't be read (it 
already did this in most cases, but this patch fixes a few stragglers 
and makes sure the partiall-loaded tileset is always freed).

3.  A new function tilespec_try_read is added.  This basically does the 
logic of fallbacks: if we fail to read the first tileset name then we 
choose a fallback tileset and try to read that.

The most obvious feature is that if you do something like "civclient -t 
womoks" (where womoks is an old unworking tileset) you'll now get a 
fallback tileset instead of a crash.

This patch will work well with PR#12488 but is separate.  With PR#12488 
we can fairly easily choose a fallback automatically from the list of 
available tilesets.  Then we can add a "priority" to the tilesets and 
have the fallback be chosen to be the highest-priority tileset.

-jason

Index: client/civclient.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/civclient.c,v
retrieving revision 1.218
diff -u -r1.218 civclient.c
--- client/civclient.c  14 Mar 2005 21:37:09 -0000      1.218
+++ client/civclient.c  13 Apr 2005 07:09:05 -0000
@@ -341,10 +341,7 @@
   helpdata_init();
   boot_help_texts();
 
-  if (!(tileset = tileset_read_toplevel(tileset_name))) {
-    /* get tile sizes etc */
-    exit(EXIT_FAILURE);
-  }
+  tilespec_try_read(tileset_name);
 
   audio_real_init(sound_set_name, sound_plugin_name);
   audio_play_music("music_start", NULL);
Index: client/tilespec.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/tilespec.c,v
retrieving revision 1.291
diff -u -r1.291 tilespec.c
--- client/tilespec.c   13 Apr 2005 02:21:51 -0000      1.291
+++ client/tilespec.c   13 Apr 2005 07:09:06 -0000
@@ -409,6 +409,14 @@
 
 
 /****************************************************************************
+  Return the name of the given tileset.
+****************************************************************************/
+const char *tileset_get_name(const struct tileset *t)
+{
+  return t->name;
+}
+
+/****************************************************************************
   Return whether the current tileset is isometric.
 ****************************************************************************/
 bool tileset_is_isometric(const struct tileset *t)
@@ -659,41 +667,20 @@
 ***********************************************************************/
 static char *tilespec_fullname(const char *tileset_name)
 {
-  const char *tileset_default;
-  char *fname, *dname;
-
-  if (isometric_view_supported()) {
-    tileset_default = "isotrident"; /* Do not i18n! --dwp */
-  } else {
-    tileset_default = "trident";    /* Do not i18n! --dwp */
-  }
-
-  if (!tileset_name || tileset_name[0] == '\0') {
-    tileset_name = tileset_default;
-  }
+  if (tileset_name) {
+    char fname[strlen(tileset_name) + strlen(TILESPEC_SUFFIX) + 1], *dname;
 
-  /* Hack: this is the name of the tileset we're about to load.  We copy
-   * it here, since this is the only place where we know it.  Note this
-   * also means if you do "civ -t foo" this will change your *default*
-   * tileset to 'foo'. */
-  sz_strlcpy(default_tileset_name, tileset_name);
+    my_snprintf(fname, sizeof(fname),
+               "%s%s", tileset_name, TILESPEC_SUFFIX);
 
-  fname = fc_malloc(strlen(tileset_name) + strlen(TILESPEC_SUFFIX) + 1);
-  sprintf(fname, "%s%s", tileset_name, TILESPEC_SUFFIX);
-  
-  dname = datafilename(fname);
-  free(fname);
+    dname = datafilename(fname);
 
-  if (dname) {
-    return mystrdup(dname);
+    if (dname) {
+      return mystrdup(dname);
+    }
   }
 
-  if (strcmp(tileset_name, tileset_default) == 0) {
-    freelog(LOG_FATAL, _("No usable default tileset found, aborting!"));
-    exit(EXIT_FAILURE);
-  }
-  freelog(LOG_ERROR, _("Trying \"%s\" tileset."), tileset_default);
-  return tilespec_fullname(tileset_default);
+  return NULL;
 }
 
 /**********************************************************************
@@ -761,6 +748,33 @@
 }
 
 /**********************************************************************
+  Read a new tilespec in when first starting the game.
+
+  Call this function with the (guessed) name of the tileset, when
+  starting the client.
+***********************************************************************/
+void tilespec_try_read(const char *tileset_name)
+{
+  if (!(tileset = tileset_read_toplevel(tileset_name))) {
+    const char *tileset_default;
+
+    if (isometric_view_supported()) {
+      tileset_default = "isotrident"; /* Do not i18n! --dwp */
+    } else {
+      tileset_default = "trident";    /* Do not i18n! --dwp */
+    }
+
+    freelog(LOG_ERROR, _("Trying \"%s\" tileset."), tileset_default);
+
+    if (!(tileset = tileset_read_toplevel(tileset_default))) {
+      freelog(LOG_FATAL, _("No usable default tileset found, aborting!"));
+      exit(EXIT_FAILURE);
+    }
+  }
+  sz_strlcpy(default_tileset_name, tileset_get_name(tileset));
+}
+
+/**********************************************************************
   Read a new tilespec in from scratch.
 
   Unlike the initial reading code, which reads pieces one at a time,
@@ -808,6 +822,7 @@
       die("Failed to re-read the currently loaded tileset.");
     }
   }
+  sz_strlcpy(default_tileset_name, tileset->name);
   tileset_load_tiles(tileset);
 
   /* Step 3: Setup
@@ -1147,15 +1162,21 @@
   struct tileset *t = tileset_new();
 
   fname = tilespec_fullname(tileset_name);
+  if (!fname) {
+    tileset_free(t);
+    return NULL;
+  }
   freelog(LOG_VERBOSE, "tilespec file is %s", fname);
 
   if (!section_file_load(file, fname)) {
     freelog(LOG_ERROR, _("Could not open \"%s\"."), fname);
+    tileset_free(t);
     return NULL;
   }
 
   if (!check_tilespec_capabilities(file, "tilespec",
                                   TILESPEC_CAPSTR, fname)) {
+    tileset_free(t);
     return NULL;
   }
 
@@ -1189,7 +1210,8 @@
     assert(tileset_name != NULL);
     section_file_free(file);
     free(fname);
-    return tileset_read_toplevel(NULL);
+    tileset_free(t);
+    return NULL;
   }
   if (!t->is_isometric && !overhead_view_supported()) {
     freelog(LOG_ERROR, _("Client does not support overhead view tilesets."
@@ -1197,7 +1219,8 @@
     assert(tileset_name != NULL);
     section_file_free(file);
     free(fname);
-    return tileset_read_toplevel(NULL);
+    tileset_free(t);
+    return NULL;
   }
 
   /* Create arrays of valid and cardinal tileset dirs.  These depend
@@ -1311,6 +1334,7 @@
   terrains = secfile_get_secnames_prefix(file, "terrain_", &num_terrains);
   if (num_terrains == 0) {
     freelog(LOG_ERROR, "No terrain types supported by tileset.");
+    tileset_free(t);
     return NULL;
   }
 
@@ -1429,6 +1453,7 @@
     if (!hash_insert(t->terrain_hash, terr->name, terr)) {
       freelog(LOG_NORMAL, "warning: duplicate terrain entry %s.",
              terrains[i]);
+      tileset_free(t);
       return NULL;
     }
   }
@@ -1439,6 +1464,7 @@
                                          "tilespec.files");
   if (num_spec_files == 0) {
     freelog(LOG_ERROR, "No tile files specified in \"%s\"", fname);
+    tileset_free(t);
     return NULL;
   }
 
Index: client/tilespec.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/tilespec.h,v
retrieving revision 1.147
diff -u -r1.147 tilespec.h
--- client/tilespec.h   11 Apr 2005 22:11:41 -0000      1.147
+++ client/tilespec.h   13 Apr 2005 07:09:06 -0000
@@ -107,6 +107,7 @@
 void tileset_load_tiles(struct tileset *t);
 void tileset_free_tiles(struct tileset *t);
 
+void tilespec_try_read(const char *tileset_name);
 void tilespec_reread(const char *tileset_name);
 void tilespec_reread_callback(struct client_option *option);
 
@@ -208,6 +209,7 @@
                                      const struct unit *punit);
 
 /* Tileset accessor functions. */
+const char *tileset_get_name(const struct tileset *t);
 bool tileset_is_isometric(const struct tileset *t);
 int tileset_hex_width(const struct tileset *t);
 int tileset_hex_height(const struct tileset *t);

[Prev in Thread] Current Thread [Next in Thread]
  • [Freeciv-Dev] (PR#12782) cleanups to tileset first-time reading, Jason Short <=