<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39765 >
In the earlier reports (PR#14969), only part of the proposed patch
was committed to 2.1, as that was easiest to quickly review.
However, the remainder of the patch optimized the performance by a
significant change to the packet and map generation. The patch was
fairly old and most patch hunks failed. These issues were
problematic this close to a release candidate.
Instead, this patch optimizes performance with a different bit-test,
and also eliminates another test for NULL in each place, thus
outperforming the original:
- if (ptile->resource
- && terrain_has_resource(ptile->terrain, ptile->resource)) {
+ if (tile_resource_is_valid(ptile)) {
No changes to packet and map generation. The change is compatible
with expected future map importation from commercial civ1/2/3.
Additionally, this patch fixes some potential problems with
settler/worker terrain+resource estimates, because the standard
functions were not used:
- ptile->terrain = new_terrain;
+ tile_change_terrain(ptile, new_terrain);
In trunk and 2.2, a coding error is fixed. The pillaging code used an
out-of-range constant in some enum variables. The constant has been
added to the proper enum.
Other minor bug fixes, found by comparing 2.1 and 2.2 divergence.
Index: server/settlers.c
===================================================================
--- server/settlers.c (revision 13728)
+++ server/settlers.c (working copy)
@@ -312,7 +312,7 @@
if (ptile->city && terrain_has_flag(new_terrain, TER_NO_CITIES)) {
return -1;
}
- ptile->terrain = new_terrain;
+ tile_change_terrain(ptile, new_terrain);
tile_clear_special(ptile, S_MINE);
goodness = city_tile_value(pcity, city_x, city_y, 0, 0);
ptile->terrain = old_terrain;
@@ -376,7 +376,7 @@
if (ptile->city && terrain_has_flag(new_terrain, TER_NO_CITIES)) {
return -1;
}
- ptile->terrain = new_terrain;
+ tile_change_terrain(ptile, new_terrain);
tile_clear_special(ptile, S_IRRIGATION);
tile_clear_special(ptile, S_FARMLAND);
goodness = city_tile_value(pcity, city_x, city_y, 0, 0);
@@ -440,16 +440,7 @@
return -1;
}
- ptile->terrain = new_terrain;
-
- if (new_terrain->mining_result != new_terrain) {
- tile_clear_special(ptile, S_MINE);
- }
- if (new_terrain->irrigation_result != new_terrain) {
- tile_clear_special(ptile, S_FARMLAND);
- tile_clear_special(ptile, S_IRRIGATION);
- }
-
+ tile_change_terrain(ptile, new_terrain);
goodness = city_tile_value(pcity, city_x, city_y, 0, 0);
ptile->terrain = old_terrain;
Index: server/savegame.c
===================================================================
--- server/savegame.c (revision 13728)
+++ server/savegame.c (working copy)
@@ -846,6 +846,15 @@
set_savegame_old_resource(&ptile->resource, ptile->terrain, ch, 1));
}
+ /* after the resources are loaded, indicate those currently valid */
+ whole_map_iterate(ptile) {
+ if (ptile->resource
+ && terrain_has_resource(ptile->terrain, ptile->resource)) {
+ /* cannot use set_special() for internal values */
+ BV_SET(ptile->special, S_RESOURCE_VALID);
+ }
+ } whole_map_iterate_end;
+
/* Owner and ownership source are stored as plain numbers */
if (has_capability("new_owner_map", savefile_options)) {
int x, y;
Index: common/city.c
===================================================================
--- common/city.c (revision 13728)
+++ common/city.c (working copy)
@@ -30,7 +30,7 @@
#include "mem.h"
#include "movement.h"
#include "packets.h"
-#include "unitlist.h"
+#include "unit.h"
#include "cm.h"
@@ -594,31 +594,29 @@
int city_x, int city_y, bool is_celebrating,
Output_type_id otype)
{
- const struct terrain *pterrain = ptile->terrain;
struct tile tile;
int prod;
- const struct output_type *output = &output_types[otype];
+ struct terrain *pterrain = tile_get_terrain(ptile);
assert(otype >= 0 && otype < O_LAST);
- if (ptile->terrain == T_UNKNOWN) {
+ if (T_UNKNOWN == pterrain) {
/* Special case for the client. The server doesn't allow unknown tiles
* to be worked but we don't necessarily know what player is involved. */
return 0;
}
prod = pterrain->output[otype];
- if (ptile->resource
- && terrain_has_resource(ptile->terrain, ptile->resource)) {
- prod += ptile->resource->output[otype];
+ if (tile_resource_is_valid(ptile)) {
+ prod += tile_get_resource(ptile)->output[otype];
}
/* create dummy tile which has the city center bonuses. */
- tile.terrain = tile_get_terrain(ptile);
+ tile.terrain = pterrain;
tile.special = tile_get_special(ptile);
if (pcity && is_city_center(city_x, city_y)
- && ptile->terrain == pterrain->irrigation_result
+ && pterrain == pterrain->irrigation_result
&& terrain_control.may_irrigate) {
/* The center tile is auto-irrigated. */
tile_set_special(&tile, S_IRRIGATION);
@@ -659,6 +657,8 @@
}
if (pcity) {
+ const struct output_type *output = &output_types[otype];
+
prod += get_city_tile_output_bonus(pcity, ptile, output,
EFT_OUTPUT_ADD_TILE);
if (prod > 0) {
Index: common/tile.c
===================================================================
--- common/tile.c (revision 13728)
+++ common/tile.c (working copy)
@@ -68,6 +68,14 @@
void tile_set_terrain(struct tile *ptile, struct terrain *pterrain)
{
ptile->terrain = pterrain;
+ if (NULL != pterrain
+ && NULL != ptile->resource
+ && terrain_has_resource(pterrain, ptile->resource)) {
+ /* cannot use set_special() for internal values */
+ BV_SET(ptile->special, S_RESOURCE_VALID);
+ } else {
+ BV_CLR(ptile->special, S_RESOURCE_VALID);
+ }
}
/****************************************************************************
@@ -117,6 +125,14 @@
void tile_set_resource(struct tile *ptile, struct resource *presource)
{
ptile->resource = presource;
+ if (NULL != ptile->terrain
+ && NULL != presource
+ && terrain_has_resource(ptile->terrain, presource)) {
+ /* cannot use set_special() for internal values */
+ BV_SET(ptile->special, S_RESOURCE_VALID);
+ } else {
+ BV_CLR(ptile->special, S_RESOURCE_VALID);
+ }
}
/****************************************************************************
@@ -283,7 +299,7 @@
case S_RIVER:
case S_AIRBASE:
case S_FALLOUT:
- case S_LAST:
+ default:
break;
}
}
@@ -313,7 +329,7 @@
case S_FARMLAND:
case S_AIRBASE:
case S_FALLOUT:
- case S_LAST:
+ default:
break;
}
}
@@ -447,9 +463,9 @@
sz_strlcat(s, special_name_translation(S_RIVER));
}
- if (ptile->resource
- && terrain_has_resource(ptile->terrain, ptile->resource)) {
- cat_snprintf(s, sizeof(s), " (%s)",
resource_name_translation(ptile->resource));
+ if (tile_resource_is_valid(ptile)) {
+ cat_snprintf(s, sizeof(s), " (%s)",
+ resource_name_translation(ptile->resource));
}
first = TRUE;
Index: common/tile.h
===================================================================
--- common/tile.h (revision 13728)
+++ common/tile.h (working copy)
@@ -17,7 +17,7 @@
#include "fc_types.h"
#include "player.h"
#include "terrain.h"
-#include "unit.h"
+#include "unitlist.h"
/* Convenience macro for accessing tile coordinates. This should only be
* used for debugging. */
@@ -28,14 +28,14 @@
int x, y; /* Cartesian (map) coordinates of the tile. */
int nat_x, nat_y; /* Native coordinates of the tile. */
int index; /* Index coordinate of the tile. */
- struct terrain *terrain; /* May be NULL for unknown tiles. */
+ Continent_id continent;
+ bv_player tile_known, tile_seen[V_COUNT];
bv_special special;
struct resource *resource; /* available resource, or NULL */
+ struct terrain *terrain; /* May be NULL for unknown tiles. */
struct city *city; /* city standing on the tile, NULL if none */
struct unit_list *units;
- bv_player tile_known, tile_seen[V_COUNT];
struct city *worked; /* city working tile, or NULL if none */
- Continent_id continent;
struct player *owner; /* Player owning this tile, or NULL. */
struct tile *owner_source; /* what makes it owned by owner */
char *spec_sprite;
@@ -53,20 +53,27 @@
/* Tile accessor functions. */
struct player *tile_get_owner(const struct tile *ptile);
void tile_set_owner(struct tile *ptile, struct player *pplayer);
+
struct city *tile_get_city(const struct tile *ptile);
void tile_set_city(struct tile *ptile, struct city *pcity);
+
struct terrain *tile_get_terrain(const struct tile *ptile);
void tile_set_terrain(struct tile *ptile, struct terrain *pterrain);
+
bv_special tile_get_special(const struct tile *ptile);
bool tile_has_special(const struct tile *ptile,
enum tile_special_type to_test_for);
void tile_set_special(struct tile *ptile, enum tile_special_type spe);
+void tile_clear_special(struct tile *ptile, enum tile_special_type spe);
+void tile_clear_all_specials(struct tile *ptile);
+
+#define tile_resource_is_valid(vtile) BV_ISSET(vtile->special,
S_RESOURCE_VALID)
const struct resource *tile_get_resource(const struct tile *ptile);
void tile_set_resource(struct tile *ptile, struct resource *presource);
-void tile_clear_special(struct tile *ptile, enum tile_special_type spe);
-void tile_clear_all_specials(struct tile *ptile);
+
+Continent_id tile_get_continent(const struct tile *ptile);
void tile_set_continent(struct tile *ptile, Continent_id val);
-Continent_id tile_get_continent(const struct tile *ptile);
+
enum known_type tile_get_known(const struct tile *ptile,
const struct player *pplayer);
Index: common/terrain.h
===================================================================
--- common/terrain.h (revision 13728)
+++ common/terrain.h (working copy)
@@ -18,7 +18,10 @@
#include "fc_types.h"
enum special_river_move {
- RMV_NORMAL=0, RMV_FAST_STRICT=1, RMV_FAST_RELAXED=2, RMV_FAST_ALWAYS=3
+ RMV_NORMAL = 0,
+ RMV_FAST_STRICT = 1,
+ RMV_FAST_RELAXED = 2,
+ RMV_FAST_ALWAYS = 3,
};
enum tile_special_type {
@@ -33,13 +36,19 @@
S_FARMLAND,
S_AIRBASE,
S_FALLOUT,
- S_LAST
+
+ /* internal values not saved */
+ S_LAST,
+ S_RESOURCE_VALID = S_LAST,
+
+ /* internal values not saved and never set */
+ S_LAST_PLUS,
};
/* S_LAST-terminated */
extern enum tile_special_type infrastructure_specials[];
-BV_DEFINE(bv_special, S_LAST);
+BV_DEFINE(bv_special, S_LAST_PLUS);
#define T_NONE (NULL) /* A special flag meaning no terrain type. */
#define T_UNKNOWN (NULL) /* An unknown terrain. */
Index: client/packhand.c
===================================================================
--- client/packhand.c (revision 13728)
+++ client/packhand.c (working copy)
@@ -1388,7 +1388,7 @@
}
if (packet->owner == game.info.player_idx) {
- freelog(LOG_ERROR, "Got packet_short_unit for own unit.");
+ freelog(LOG_ERROR, "handle_unit_short_info() for own unit.");
}
punit = unpackage_short_unit(packet);
@@ -2012,7 +2012,14 @@
}
}
- ptile->resource = resource_by_number(packet->resource);
+ if (ptile->resource
+ && ptile->resource->index != packet->resource) {
+ tile_changed = TRUE;
+ }
+
+ /* always called after setting terrain */
+ tile_set_resource(ptile, resource_by_number(packet->resource));
+
if (packet->owner == MAP_TILE_OWNER_NULL) {
if (ptile->owner) {
ptile->owner = NULL;
@@ -2047,7 +2054,9 @@
case TILE_UNKNOWN:
break;
default:
- freelog(LOG_NORMAL, "Unknown tile value %d.", packet->known);
+ freelog(LOG_ERROR,
+ "handle_tile_info() unknown tile value %d.",
+ packet->known);
break;
}
}
@@ -2273,8 +2282,8 @@
**************************************************************************/
void handle_ruleset_building(struct packet_ruleset_building *p)
{
+ int i;
struct impr_type *b = improvement_by_number(p->id);
- int i;
if (!b) {
freelog(LOG_ERROR,
@@ -2399,8 +2408,8 @@
**************************************************************************/
void handle_ruleset_terrain(struct packet_ruleset_terrain *p)
{
+ int j;
struct terrain *pterrain = terrain_by_number(p->id);
- int j;
if (!pterrain) {
freelog(LOG_ERROR,
Index: client/gui-sdl/mapview.c
===================================================================
--- client/gui-sdl/mapview.c (revision 13728)
+++ client/gui-sdl/mapview.c (working copy)
@@ -495,8 +495,7 @@
sz_strlcat(s, special_name_translation(S_RIVER));
}
- if (ptile->resource
- && terrain_has_resource(ptile->terrain, ptile->resource)) {
+ if (tile_resource_is_valid(ptile)) {
cat_snprintf(s, sizeof(s), " (%s)",
resource_name_translation(ptile->resource));
}
Index: client/tilespec.c
===================================================================
--- client/tilespec.c (revision 13728)
+++ client/tilespec.c (working copy)
@@ -4142,8 +4142,7 @@
case LAYER_SPECIAL1:
if (ptile && client_tile_get_known(ptile) != TILE_UNKNOWN) {
if (draw_specials) {
- if (ptile->resource
- && terrain_has_resource(ptile->terrain, ptile->resource)) {
+ if (tile_resource_is_valid(ptile)) {
ADD_SPRITE_SIMPLE(t->sprites.resource[ptile->resource->index]);
}
}
_______________________________________________
Freeciv-dev mailing list
[email protected]
https://mail.gna.org/listinfo/freeciv-dev
