<URL: http://bugs.freeciv.org/Ticket/Display.html?id=40176 >
On 03/04/2008, Marko Lindqvist wrote:
>
> On 03/04/2008, Marko Lindqvist wrote:
> > This should be fixable simply by adding ref_count for vision sites.
>
> Patch attached.
- Handle reassigning same vision site to tile correctly
- Some more comments added
- ML
diff -Nurd -X.diff_ignore freeciv/common/vision.c freeciv/common/vision.c
--- freeciv/common/vision.c 2008-03-08 16:32:49.000000000 +0200
+++ freeciv/common/vision.c 2008-04-03 03:01:51.000000000 +0300
@@ -17,6 +17,7 @@
#include <assert.h>
+#include "log.h"
#include "mem.h"
#include "shared.h"
@@ -77,6 +78,11 @@
****************************************************************************/
void free_vision_site(struct vision_site *psite)
{
+ if (psite->ref_count > 0) {
+ /* Somebody still uses this vision site. Do not free */
+ return;
+ }
+ assert(psite->ref_count == 0);
free(psite);
}
@@ -119,3 +125,25 @@
psite->size = pcity->size;
sz_strlcpy(psite->name, city_name(pcity));
}
+
+/****************************************************************************
+ Copy relevant information from one site struct to another.
+ Currently only user expects everything except ref_count to be copied.
+ If other kind of users are added, parameter defining copied information
+ set should be added.
+****************************************************************************/
+void copy_vision_site(struct vision_site *dest, struct vision_site *src)
+{
+ /* Copy everything except ref_count. */
+ strcpy(dest->name, src->name);
+ dest->location = src->location;
+ dest->owner = src->owner;
+ dest->identity = src->identity;
+ dest->size = src->size;
+ dest->border_radius_sq = src->border_radius_sq;
+ dest->occupied = src->occupied;
+ dest->walls = src->walls;
+ dest->happy = src->happy;
+ dest->unhappy = src->unhappy;
+ dest->improvements = src->improvements;
+}
diff -Nurd -X.diff_ignore freeciv/common/vision.h freeciv/common/vision.h
--- freeciv/common/vision.h 2008-03-08 16:32:49.000000000 +0200
+++ freeciv/common/vision.h 2008-04-03 00:05:41.000000000 +0300
@@ -124,6 +124,8 @@
bool happy;
bool unhappy;
+ int ref_count;
+
bv_imprs improvements;
};
@@ -134,6 +136,7 @@
struct vision_site *create_vision_site_from_city(const struct city *pcity);
void update_vision_site_from_city(struct vision_site *psite,
const struct city *pcity);
+void copy_vision_site(struct vision_site *dest, struct vision_site *src);
/* get 'struct site_list' and related functions: */
#define SPECLIST_TAG site
diff -Nurd -X.diff_ignore freeciv/server/citytools.c freeciv/server/citytools.c
--- freeciv/server/citytools.c 2008-03-12 21:58:27.000000000 +0200
+++ freeciv/server/citytools.c 2008-04-03 03:48:32.000000000 +0300
@@ -1562,6 +1562,8 @@
continue;
}
whole_map_iterate(ptile) {
+ /* FIXME: map_get_player_base()???
+ * Should be cities only: map_get_player_city() */
if (!pplayer || NULL != map_get_player_base(ptile, pplayer)) {
send_city_info_at_tile(pplayer, pconn->self, NULL, ptile);
}
@@ -1791,8 +1793,8 @@
} improvement_iterate_end;
if (NULL == pdcity) {
- map_get_player_tile(pcenter, pplayer)->site =
pdcity = create_vision_site_from_city(pcity);
+ change_playertile_site(map_get_player_tile(pcenter, pplayer), pdcity);
} else if (pdcity->location != pcenter) {
freelog(LOG_ERROR, "Trying to update bad city (wrong location)"
" at %i,%i for player %s",
@@ -1840,8 +1842,12 @@
struct player_tile *playtile = map_get_player_tile(ptile, pplayer);
dlsend_packet_city_remove(pplayer->connections, pdcity->identity);
+ assert(playtile->site == pdcity);
playtile->site = NULL;
- free(pdcity);
+ pdcity->ref_count--; /* Subtract ref_count before calling
+ * free_vision_site() */
+ assert(pdcity->ref_count >= 0);
+ free_vision_site(pdcity);
}
}
}
diff -Nurd -X.diff_ignore freeciv/server/maphand.c freeciv/server/maphand.c
--- freeciv/server/maphand.c 2008-03-12 21:58:27.000000000 +0200
+++ freeciv/server/maphand.c 2008-04-03 11:06:06.000000000 +0300
@@ -1073,6 +1073,34 @@
}
/***************************************************************
+ Changes site information for player tile.
+***************************************************************/
+void change_playertile_site(struct player_tile *ptile,
+ struct vision_site *new_site)
+{
+ if (ptile->site == new_site) {
+ /* Do nothing. Especially: don't decrease ref_count and
+ * free vision site... */
+ return;
+ }
+
+ if (ptile->site != NULL) {
+ /* Releasing old site from tile */
+ ptile->site->ref_count--;
+ assert(ptile->site->ref_count >= 0);
+ if (ptile->site->ref_count == 0) {
+ /* Free vision site before losing its address completely */
+ free_vision_site(ptile->site);
+ }
+ }
+ if (new_site != NULL) {
+ /* Assigning new site to tile */
+ new_site->ref_count++;
+ }
+ ptile->site = new_site;
+}
+
+/***************************************************************
...
***************************************************************/
void map_set_known(struct tile *ptile, struct player *pplayer)
@@ -1148,8 +1176,8 @@
whole_map_iterate(ptile) {
struct player_tile *playtile = map_get_player_tile(ptile, pplayer);
- /* cleverly uses return that is NULL for non-site tile */
- playtile->site = map_get_player_base(ptile, pplayer);
+ /* map_get_player_base() will return NULL for non-site tile */
+ change_playertile_site(playtile, map_get_player_base(ptile, pplayer));
} whole_map_iterate_end;
/* only after removing borders! */
@@ -1157,6 +1185,8 @@
struct vision_site *psite = map_get_player_base(ptile, pplayer);
if (NULL != psite) {
+ /* Player tile will be freed, so ref_count goes down */
+ psite->ref_count--;
free_vision_site(psite);
}
} whole_map_iterate_end;
@@ -1197,7 +1227,9 @@
}
/****************************************************************************
- ...
+ Returns vision site located at given tile from player map.
+ FIXME: Rename function as it's not returning only bases, but
+ any vision sites.
****************************************************************************/
struct vision_site *map_get_player_base(const struct tile *ptile,
const struct player *pplayer)
@@ -1211,7 +1243,7 @@
}
/****************************************************************************
- ...
+ Returns city located at given tile from player map.
****************************************************************************/
struct vision_site *map_get_player_city(const struct tile *ptile,
const struct player *pplayer)
@@ -1355,10 +1387,17 @@
/* Set and send new city info */
if (from_tile->site && from_tile->site->location == ptile) {
if (!dest_tile->site || dest_tile->site->location != ptile) {
- dest_tile->site = fc_calloc(1, sizeof(*dest_tile->site));
+ change_playertile_site(dest_tile, NULL);
+
+ /* We cannot assign new vision site with change_playertile_site(),
+ * since location is not yet set up for new site */
+ dest_tile->site = create_vision_site(0, NULL, NULL);
+ dest_tile->site->ref_count++;
}
- /* struct assignment copy */
- *dest_tile->site = *from_tile->site;
+ /* Copy vision information.
+ * Note that we don't care if receiver knows vision source city
+ * or not. */
+ copy_vision_site(dest_tile->site, from_tile->site);
send_city_info_at_tile(pdest, pdest->connections, NULL, ptile);
}
@@ -1758,7 +1797,7 @@
struct player_tile *playtile = map_get_player_tile(ptile, ploser);
/* cleverly uses return that is NULL for non-site tile */
- playtile->site = map_get_player_base(ptile, ploser);
+ change_playertile_site(playtile, map_get_player_base(ptile, ploser));
if (NULL != playtile->site && ptile == psource) {
/* has new owner */
@@ -1774,7 +1813,7 @@
if (ptile != psource) {
struct player_tile *playtile = map_get_player_tile(ptile, powner);
assert(NULL == playtile->site);
- playtile->site = psite;
+ change_playertile_site(playtile, psite);
} else if (NULL != pcity) {
update_vision_site_from_city(psite, pcity);
} else {
@@ -1790,8 +1829,7 @@
} else {
psite = create_vision_site(IDENTITY_NUMBER_ZERO, psource, powner);
}
-
- playsite->site = psite;
+ change_playertile_site(playsite, psite);
}
} else {
assert(NULL == powner && NULL == psource);
diff -Nurd -X.diff_ignore freeciv/server/maphand.h freeciv/server/maphand.h
--- freeciv/server/maphand.h 2008-03-10 19:54:21.000000000 +0200
+++ freeciv/server/maphand.h 2008-04-03 01:55:03.000000000 +0300
@@ -110,4 +110,7 @@
int radius_sq);
void vision_clear_sight(struct vision *vision);
+void change_playertile_site(struct player_tile *ptile,
+ struct vision_site *new_site);
+
#endif /* FC__MAPHAND_H */
diff -Nurd -X.diff_ignore freeciv/server/savegame.c freeciv/server/savegame.c
--- freeciv/server/savegame.c 2008-03-25 10:46:50.000000000 +0200
+++ freeciv/server/savegame.c 2008-04-03 03:38:31.000000000 +0300
@@ -2835,7 +2835,7 @@
for (i = 0; i < total_ncities; i++) {
/* similar to create_vision_site() */
- struct vision_site *pdcity = fc_calloc(1, sizeof(*pdcity));
+ struct vision_site *pdcity = create_vision_site(0, NULL, NULL);
nat_y = secfile_lookup_int(file, "player%d.dc%d.y", plrno, i);
nat_x = secfile_lookup_int(file, "player%d.dc%d.x", plrno, i);
@@ -2905,7 +2905,8 @@
}
sz_strlcpy(pdcity->name, secfile_lookup_str(file, "player%d.dc%d.name", plrno, i));
- map_get_player_tile(pdcity->location, plr)->site = pdcity;
+ change_playertile_site(map_get_player_tile(pdcity->location, plr),
+ pdcity);
identity_number_reserve(pdcity->identity);
}
diff -Nurd -X.diff_ignore freeciv/server/unittools.c freeciv/server/unittools.c
--- freeciv/server/unittools.c 2008-03-12 21:58:28.000000000 +0200
+++ freeciv/server/unittools.c 2008-04-03 03:59:02.000000000 +0300
@@ -2884,6 +2884,8 @@
circle_iterate(punit->tile, radius_sq, ptile) {
struct unit *penemy = is_non_allied_unit_tile(ptile, pplayer);
+
+ /* FIXME: Should be map_get_player_city()? */
struct vision_site *pdcity = map_get_player_base(ptile, pplayer);
if ((penemy && can_player_see_unit(pplayer, penemy))
_______________________________________________
Freeciv-dev mailing list
[email protected]
https://mail.gna.org/listinfo/freeciv-dev
