Re: [Freeciv-Dev] (PR#39383) BUG: terrain and resource name translation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39383
William Allen Simpson wrote:
There's a warning, because the current resource list is const, and this
modifies the pointer. Updating the const can happen later.
trunk revision 12977
S2_1 revision 12978
Remaining const struct resource * caused 1 warning, and a report that
gcc 4.1 (or a stricter compile option) escalated it to a compile error.
Most const removed; 5 remain in function parameters, causing no warnings.
This is the S2_1 patch for posterity. The trunk is nearly identical
(different line numbers).
Happy hacking! Please let me know of any more problems!
Index: server/generator/mapgen.c
===
--- server/generator/mapgen.c (revision 12977)
+++ server/generator/mapgen.c (working copy)
@@ -1361,7 +1361,7 @@
}
if (!is_ocean(pterrain) || near_safe_tiles (ptile)) {
int i = 0;
- const struct resource **r;
+ struct resource **r;
for (r = pterrain-resources; *r; r++) {
/* This is a standard way to get a random element from the
Index: server/ruleset.c
===
--- server/ruleset.c(revision 12977)
+++ server/ruleset.c(working copy)
@@ -2788,7 +2788,7 @@
terrain_type_iterate(pterrain) {
const int i = pterrain-index;
-const struct resource **r;
+struct resource **r;
packet.id = i;
Index: server/maphand.h
===
--- server/maphand.h(revision 12977)
+++ server/maphand.h(working copy)
@@ -40,7 +40,7 @@
struct player_tile {
struct terrain *terrain; /* May be NULL for unknown tiles. */
bv_special special;
- const struct resource *resource;
+ struct resource *resource;
unsigned short seen_count[V_COUNT];
unsigned short own_seen[V_COUNT];
/* If you build a city with an unknown square within city radius
Index: server/savegame.c
===
--- server/savegame.c (revision 12977)
+++ server/savegame.c (working copy)
@@ -746,7 +746,7 @@
ch is the character read from the map, and n is the number of the special
(0 for special_1, 1 for special_2).
/
-static void set_savegame_old_resource(const struct resource **r,
+static void set_savegame_old_resource(struct resource **r,
const struct terrain *terrain,
char ch, int n)
{
@@ -769,7 +769,7 @@
/
Return the resource for the given identifier.
/
-static const struct resource *identifier_to_resource(char c)
+static struct resource *identifier_to_resource(char c)
{
if (c == RESOURCE_NULL_IDENTIFIER) {
return NULL;
Index: common/combat.c
===
--- common/combat.c (revision 12977)
+++ common/combat.c (working copy)
@@ -601,10 +601,10 @@
struct unit *punit = unit_list_get(ptile-units, 0);
freelog(LOG_ERROR, get_defender bug: %s's %s vs %s's %s (total %d
- units) on %s at (%d,%d). , unit_owner(attacker)-name,
+ units) on \%s\ at (%d,%d). , unit_owner(attacker)-name,
unit_type(attacker)-name, unit_owner(punit)-name,
unit_type(punit)-name, unit_list_size(ptile-units),
-terrain_name_translation(ptile-terrain), ptile-x, ptile-y);
+ptile-terrain-name_rule, ptile-x, ptile-y);
}
return bestdef;
Index: common/tile.c
===
--- common/tile.c (revision 12977)
+++ common/tile.c (working copy)
@@ -114,7 +114,7 @@
/
Set the given resource at the specified tile.
/
-void tile_set_resource(struct tile *ptile, const struct resource *presource)
+void tile_set_resource(struct tile *ptile, struct resource *presource)
{
ptile-resource = presource;
}
Index: common/tile.h
===
--- common/tile.h (revision 12977)
+++ common/tile.h (working copy)
@@ -30,7 +30,7 @@
int index; /* Index coordinate of the tile. */
struct terrain *terrain; /* May be NULL for unknown tiles. */
bv_special special;
- const struct resource *resource; /* available resource, or NULL */
+ struct resource *resource; /* available resource, or NULL */
struct city *city;/* city standing on the tile, NULL if none */
struct unit_list *units;
bv_player tile_known, tile_seen[V_COUNT];
@@ -62,7 +62,7 @@
[Freeciv-Dev] (PR#39383) BUG: terrain and resource name translation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39383
While trying to debug some of my terrain changes, it was frustrating that
some error messages had empty (missing/blank) fields. Numerous messages
tried to insert a translated name into log messages that are not translated.
(I'm running without translation, so it shows up badly here.)
So, I spent all day figuring out some problems, and found a few others!
For example, the req_ Terrain Mountains probably didn't work for foreign
languages, because the test compared the translated terrain name against a
list that is always in ruleset (original) language.
Tests were wildly inconsistent, sometimes testing against original, others
against translated. I made them *all* against original. Translated names
are now only accessed via a central function. (The function existed, but
not everybody used it.)
My solution was to rename the name field to name_translated (how it
was actually used in most places), and rename name_orig to name_original.
That allowed me to find all the uses.
For the same reason, I renamed the accessor functions, so that future
programmers will be more aware of the actual purpose (or less confused).
I found some error messages that could be reworded to be identical, with
different parameters, that should ease translation a little.
Also, I found that there were many places that terrain and tiles were
confused (such as calling terrain tile_type). That will take more
time to unravel.
Compiles, runs here, needs more testing.
Index: server/generator/mapgen.c
===
--- server/generator/mapgen.c (revision 12970)
+++ server/generator/mapgen.c (working copy)
@@ -303,8 +303,8 @@
**/
static struct terrain *pick_ocean(int depth)
{
- /* FIXME: get_flag_terrain may return NULL if there is no match. */
- struct terrain *best_terrain = get_flag_terrain(TER_OCEANIC);
+ /* FIXME: pick_terrain_by_flag may return NULL if there is no match. */
+ struct terrain *best_terrain = pick_terrain_by_flag(TER_OCEANIC);
int best_match = abs(depth - best_terrain-property[MG_OCEAN_DEPTH]);
terrain_type_iterate(pterrain) {
@@ -980,9 +980,9 @@
if (!terrain_has_flag(pterrain, TER_CAN_HAVE_RIVER)) {
/* We have to change the terrain to put a river here. */
- /* FIXME: get_flag_terrain may return NULL
+ /* FIXME: pick_terrain_by_flag may return NULL
* if there is no match. */
- pterrain = get_flag_terrain(TER_CAN_HAVE_RIVER);
+ pterrain = pick_terrain_by_flag(TER_CAN_HAVE_RIVER);
tile_set_terrain(tile1, pterrain);
}
tile_set_special(tile1, S_RIVER);
@@ -1109,7 +1109,7 @@
terrain_type_iterate(pterrain) {
freelog(loglevel, %20s : %4d %d%% ,
- get_name(pterrain), terrain_count[pterrain-index],
+ pterrain-name_original, terrain_count[pterrain-index],
(terrain_count[pterrain-index] * 100 + 50) / total);
} terrain_type_iterate_end;
}
Index: server/scripting/api.pkg
===
--- server/scripting/api.pkg(revision 12970)
+++ server/scripting/api.pkg(working copy)
@@ -100,7 +100,7 @@
};
struct Terrain {
- const char *name_orig @ name;
+ const char *name_original @ name;
};
Index: server/scripting/api_find.c
===
--- server/scripting/api_find.c (revision 12970)
+++ server/scripting/api_find.c (working copy)
@@ -147,15 +147,15 @@
**/
Terrain *api_find_terrain(int terrain_id)
{
- return get_terrain(terrain_id);
+ return get_terrain_by_number(terrain_id);
}
/**
- Return the tech type with the given name_orig.
+ Return the terrain with the given name_orig.
**/
Terrain *api_find_terrain_by_name(const char *name_orig)
{
- struct terrain *pterrain = get_terrain_by_name(name_orig);
+ struct terrain *pterrain = get_terrain_by_original_name(name_orig);
return pterrain;
}
Index: server/citytools.c
===
--- server/citytools.c (revision 12970)
+++ server/citytools.c (working copy)
@@ -1097,7 +1097,7 @@
_(Moved %s out of disbanded city %s
since it cannot stay on %s.),
unit_type(punit)-name, pcity-name,
- get_name(tile_get_terrain(ptile)));
+ terrain_name_translation(tile_get_terrain(ptile)));
break;
}
}
Index: server/cityturn.c
Re: [Freeciv-Dev] (PR#39383) BUG: terrain and resource name translation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39383
In this version, I moved the translation into the accessor functions, as
this seemed to be better than spread around in 3 places, especially as the
other places in the code forgot to test for NULL and '\0' (empty string).
Comments in the code seemed to indicate a move was desired.
There's a warning, because the current resource list is const, and this
modifies the pointer. Updating the const can happen later.
Also, renamed the previous name_orig (or name_original) to name_rule --
that better indicates where the name originated, and makes it easier to
search in the future.
I've left the rest alone, but more could be done
As far as I'm concerned, it's ready to check in.
Index: server/generator/mapgen.c
===
--- server/generator/mapgen.c (revision 12970)
+++ server/generator/mapgen.c (working copy)
@@ -303,8 +303,8 @@
**/
static struct terrain *pick_ocean(int depth)
{
- /* FIXME: get_flag_terrain may return NULL if there is no match. */
- struct terrain *best_terrain = get_flag_terrain(TER_OCEANIC);
+ /* FIXME: pick_terrain_by_flag may return NULL if there is no match. */
+ struct terrain *best_terrain = pick_terrain_by_flag(TER_OCEANIC);
int best_match = abs(depth - best_terrain-property[MG_OCEAN_DEPTH]);
terrain_type_iterate(pterrain) {
@@ -980,9 +980,9 @@
if (!terrain_has_flag(pterrain, TER_CAN_HAVE_RIVER)) {
/* We have to change the terrain to put a river here. */
- /* FIXME: get_flag_terrain may return NULL
+ /* FIXME: pick_terrain_by_flag may return NULL
* if there is no match. */
- pterrain = get_flag_terrain(TER_CAN_HAVE_RIVER);
+ pterrain = pick_terrain_by_flag(TER_CAN_HAVE_RIVER);
tile_set_terrain(tile1, pterrain);
}
tile_set_special(tile1, S_RIVER);
@@ -1109,7 +1109,7 @@
terrain_type_iterate(pterrain) {
freelog(loglevel, %20s : %4d %d%% ,
- get_name(pterrain), terrain_count[pterrain-index],
+ pterrain-name_rule, terrain_count[pterrain-index],
(terrain_count[pterrain-index] * 100 + 50) / total);
} terrain_type_iterate_end;
}
Index: server/scripting/api.pkg
===
--- server/scripting/api.pkg(revision 12970)
+++ server/scripting/api.pkg(working copy)
@@ -100,7 +100,7 @@
};
struct Terrain {
- const char *name_orig @ name;
+ const char *name_rule @ name;
};
Index: server/scripting/api_find.c
===
--- server/scripting/api_find.c (revision 12970)
+++ server/scripting/api_find.c (working copy)
@@ -147,15 +147,15 @@
**/
Terrain *api_find_terrain(int terrain_id)
{
- return get_terrain(terrain_id);
+ return get_terrain_by_number(terrain_id);
}
/**
- Return the tech type with the given name_orig.
+ Return the terrain with the given name_orig.
**/
Terrain *api_find_terrain_by_name(const char *name_orig)
{
- struct terrain *pterrain = get_terrain_by_name(name_orig);
+ struct terrain *pterrain = get_terrain_by_rule_name(name_orig);
return pterrain;
}
Index: server/citytools.c
===
--- server/citytools.c (revision 12970)
+++ server/citytools.c (working copy)
@@ -1097,7 +1097,7 @@
_(Moved %s out of disbanded city %s
since it cannot stay on %s.),
unit_type(punit)-name, pcity-name,
- get_name(tile_get_terrain(ptile)));
+ terrain_name_translation(tile_get_terrain(ptile)));
break;
}
}
Index: server/cityturn.c
===
--- server/cityturn.c (revision 12970)
+++ server/cityturn.c (working copy)
@@ -817,7 +817,7 @@
%s terrain is required. Postponing...),
pcity-name,
get_impr_name_ex(pcity, building-index),
- get_name(preq-source.value.terrain));
+
terrain_name_translation(preq-source.value.terrain));
script_signal_emit(building_cant_be_built, 3,
API_TYPE_BUILDING_TYPE, building,
API_TYPE_CITY, pcity,
Index: server/ruleset.c
===
--- server/ruleset.c(revision 12970)
+++
