Re: [Freeciv-Dev] (PR#40113) multi-player connection missing client.playing during nation selection
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40113 Madeline Book wrote: ... In the case where the client's player is then created (attach_connection_to_player(pconn, NULL) returns TRUE, server/connecthand.c +137) the server does not notify the client that its game.info.player_idx has changed (hence the client keeps its client.playing equal to NULL even though it should now have a player, and indeed receives conn and player info packets to that effect). ... Good analysis. This kind of inconsistency is always a problem whenever there are multiple copies of the same data (in this case the player pointer in several data structures). Solution: The server sends another game info packet after the client's player has been created. This ensures that the client receives an updated player_idx field and accordingly sets its client.playing to a valid pointer. Ahem. That's not exactly a solution, that's generally called a hack -- from the old days of applying a hacksaw to model railroads. I've been trying to reduce *_info spam over the past 6 months, see PR#4 and its predecessors. A better (temporary) solution is to remove the validity checking, as the player_idx is not always valid, despite the comments in the code. I'm not a supporter of temporary solutions, when there are obvious permanent solutions. (See below.) Hence when running a forked server (started by the client using Start New Game), which sends hardcoded commands right upon connection (i.e. /set aifill 5), the client.playing variable in the client is made consistent (i.e. not NULL) before the user can notice any discrepancies. Excellent catch! This is how the problem was missed in the past. Only by pretending to be a second player in a multi-player game (when you are actually the first), you bypassed the usual command processing. Sending those hardcoded commands is a bad idea, too. There are several existing bug reports. It is strongly recommended that the game.info.player_idx field be completely removed as soon as convenient, and the client rely on a better designed and more logical mecha- nism to update its client.playing global variable (the conn info packet handler comes to mind). I concur. Almost all uses were removed in PR#39872. However, a deeper analysis shows that game.info.player_idx is based upon: server/gamehand.c:370: /* ? fixme: check for non-players: */ ginfo.player_idx = (pconn-player ? player_number(pconn-player) : -1); That is, a duplicate of the connection player! Duplication be bad! That fixme has been sitting there like a land-mine since before 1.14.2 (the earliest version that I have on hand). As you suggest, the comprehensive solution is to replace player_idx with an indication of the connection! In turn, the connection has (or soon will have) the player and observer status in it. There is *no* need for the client.playing (nee game.player_ptr) variable. This could also eliminate the duplicate aconnection.player (and other duplicate data). The connection number is learned during the login sequence. It is never changed or renumbered, unlike the player number. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40113) connection missing client.playing during nation selection
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40113
I indicated earlier in the discussion thread where you added the revised
test that is now failing, because the two pointers are updated by
different packets and apparently aren't always equal
I assumed you had a reason for making the change. Since you don't, I'll
remove the change. Thanks for letting us know.
Per I. Mathisen wrote:
I do not recall doing any such thing. It is not the code that I have
typically worked on. Generally I did not touch the client if I could
avoid it.
This aconnection.player change appeared in PR#16459:
static void pick_nation_callback(GtkWidget *w, gpointer data)
{
- if (game.player_ptr) {
+ if (aconnection.player) {
popup_races_dialog(game.player_ptr);
+ } else if (game.info.is_new_game) {
+send_chat(/take -);
}
}
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40115) vestigial game.info.player_idx removal
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40115
Almost all references to game.info.player_idx were eliminated in PR#39872.
This removes the remaining 9 instances in 6 files.
While investigating, another bug was discovered! When players are
renumbered during pre-game, aconnection.player was not updated.
After considerable discussion in PR#40113, there are no intended differences
between client.playing (nee game.info.player_ptr) and aconnection.player.
This codifies the duplication by always setting them at the same time.
client.playing: 910 instances in 91 files.
aconnection.player: 8 instances in 4 files.
Both are duplicates of the actual connection[i].player.
pc-player: 2 instances in 2 files.
conn([.-]+)player: 82 instances in 20 files.
Index: server/gamehand.c
===
--- server/gamehand.c (revision 14424)
+++ server/gamehand.c (working copy)
@@ -367,8 +367,6 @@
}
conn_list_iterate(dest, pconn) {
-/* ? fixme: check for non-players: */
-ginfo.player_idx = (pconn-player ? player_number(pconn-player) : -1);
send_packet_game_info(pconn, ginfo);
}
conn_list_iterate_end;
Index: server/savegame.c
===
--- server/savegame.c (revision 14424)
+++ server/savegame.c (working copy)
@@ -4407,8 +4407,6 @@
shuffle_players();
}
- game.info.player_idx = 0;
-
/* Fix ferrying sanity */
players_iterate(pplayer) {
unit_list_iterate_safe(pplayer-units, punit) {
Index: common/packets.def
===
--- common/packets.def (revision 14424)
+++ common/packets.def (working copy)
@@ -380,7 +380,6 @@
PLAYER min_players;
PLAYER max_players;
PLAYER nplayers;
- PLAYER player_idx;
UINT32 globalwarming, heating, warminglevel;
UINT32 nuclearwinter, cooling, coolinglevel;
Index: common/game.c
===
--- common/game.c (revision 14424)
+++ common/game.c (working copy)
@@ -382,7 +382,7 @@
game.info.global_advances[i]=FALSE;
for (i=0; iB_LAST; i++) /* game.num_impr_types = 0 here */
game.info.great_wonders[i]=0;
- game.info.player_idx = 0;
+
terrain_control.river_help_text[0] = '\0';
game.meta_info.user_message_set = FALSE;
@@ -608,11 +608,6 @@
assert(city_list_size(game.players[i].cities) == 0);
}
- /* has no effect in server, but adjusts in client */
- if(game.info.player_idx plrno) {
-game.info.player_idx--;
- }
-
game.info.nplayers--;
player_init(game.players[game.info.nplayers]);
Index: client/packhand.c
===
--- client/packhand.c (revision 14424)
+++ client/packhand.c (working copy)
@@ -218,9 +218,9 @@
freelog(LOG_VERBOSE, join game accept:%s, message);
aconnection.established = TRUE;
aconnection.id = conn_id;
+
agents_game_joined();
update_menus();
-
set_server_busy(FALSE);
if (get_client_page() == PAGE_MAIN
@@ -235,11 +235,13 @@
my_snprintf(msg, sizeof(msg),
_(You were rejected from the game: %s), message);
append_output_window(msg);
-aconnection.id = 0;
+aconnection.id = -1; /* not in range of conn_info id */
+
if (auto_connect) {
freelog(LOG_NORMAL, %s, msg);
}
gui_server_connect();
+
if (!with_ggz) {
set_client_page(in_ggz ? PAGE_MAIN : PAGE_GGZ);
}
@@ -1540,7 +1542,7 @@
return;
}
- if (packet-owner == game.info.player_idx) {
+ if (valid_player_by_number(packet-owner) == client.playing) {
freelog(LOG_ERROR, handle_unit_short_info() for own unit.);
}
@@ -1612,7 +1614,6 @@
game.government_when_anarchy =
government_by_number(game.info.government_when_anarchy_id);
- client.playing = valid_player_by_number(game.info.player_idx);
if (C_S_PREPARING == client_state()) {
/* FIXME: only for change in nations */
@@ -1954,6 +1955,8 @@
aconnection.established = pconn-established;
aconnection.observer = pconn-observer;
aconnection.access_level = pconn-access_level;
+ /* FIXME: duplication */
+ client.playing =
aconnection.player = pplayer;
}
}
Index: client/climisc.c
===
--- client/climisc.c(revision 14424)
+++ client/climisc.c(working copy)
@@ -62,11 +62,15 @@
**/
void client_remove_player(int plrno)
{
+ struct connection *pc = find_conn_by_id(aconnection.id);
+
game_remove_player(player_by_number(plrno));
game_renumber_players(plrno);
- /* ensure our pointer is valid */
- client.playing = valid_player_by_number(game.info.player_idx);
+ /* ensure our duplicate pointers are valid */
+
Re: [Freeciv-Dev] (PR#40115) vestigial game.info.player_idx removal
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40115 Committed trunk revision 14425. Committed S2_2 revision 14426. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40113) multi-player connection missing client.playing during nation selection
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40113 It was too hard to do it all in one patch. Please check whether the current revision fixes your problem? I'll finish removing the duplicates later ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40113) connection missing client.playing during nation selection
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40113 Madeline Book wrote: If only I knew of some way to find the relevant PR#s (besides PR#39872)! How do you suggest I go about doing that? I am not very good yet with RT's search capabilities. Please re-read the thread, either in your email, or on RT (where the links are linked). Oh sorry, I thought you would immediately know how to fix the problem (thanks to your work on the relevant code when you updated to client.playing). But I see we are both in the dark on this one. ;) Yep. Let us know when you figure out what revision caused the problem. Hi Per, can you help us to understand? (I know you are reading this :. I've added him to the CC this time. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40114) vestigial city_map removal from client/agents/cma_core.c
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40114 Committed trunk revision 14423. Committed S2_2 revision 14424. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40111) Diplomatic immunity
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40111 Per I. Mathisen wrote: It is not a bug. Also, making diplomats partially invisible would upset a game balanced already tilted too much in favour of diplomats and spies as it is. Madeline didn't write that. I did. And we already know your opinion about diplomats, and the crippling code you've added to trunk. So, somebody deliberately removed the partial invisibility of diplomats? Where is this documented? Why isn't there a compatible flag in data/civ2/units.ruleset? (I was assuming poor file maintenance, not deliberate error) Definitely a bug! ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40113) civclient: player.c:293: player_number: Assertion `pplayer' failed.
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40113
Now that I've had some sleep
Madeline Book wrote:
Yes, I was afraid of something like that. So what are the
semantics of client.playing?
It is set from game.info.player_idx -- exclusively.
Is it supposed to be NULL
when the client is an observer (unlike aconnection.player
apparently) and during pregame before the user has picked
his nation and set his player name?
Had never noticed aconnection.player, and have no idea about its purpose.
Certainly, aconnection.player is a bad idea. That isn't properly updated
as players are removed (before start) -- unlike client.playing.
Yet Another Data Duplication headache.
What then is supposed
to be passed to popup_races_dialog?
Heck, you shouldn't be able to select a race before player is assigned.
Compare XAW:
if (client.playing) {
popup_races_dialog(client.playing);
}
With GTK2:
if (aconnection.player) {
popup_races_dialog(client.playing);
} else if (game.info.is_new_game) {
send_chat(/take -);
popup_races_dialog(client.playing);
}
I added the last call in PR#39927 back in December, both 2.1 and 2.2, so
that's not within the past 2 weeks:
send_chat(/take -);
+popup_races_dialog(game.player_ptr);
This aconnection.player change appeared in PR#16459, so perhaps Per can
explain:
static void pick_nation_callback(GtkWidget *w, gpointer data)
{
- if (game.player_ptr) {
+ if (aconnection.player) {
popup_races_dialog(game.player_ptr);
+ } else if (game.info.is_new_game) {
+send_chat(/take -);
}
}
For connection dialog, another call to popup_races_dialog() with another
parameter was added even earlier in PR#15688, so perhaps Jason can explain:
+static void conn_menu_nation_chosen(GtkMenuItem *menuitem, gpointer data)
+{
+ popup_races_dialog(conn_menu_player);
+}
So, a third static copy of player, always a joy to debug! :-(
Yet Another Data Duplication headache.
No, I just connected to a server running on my local
machine and tried to start the game (incidentally doing
it this way the Start button is also inactive, which
might indicate that there is some deeper problem).
Probably just sensitive to whether you've picked a Nation yet.
1. Start a server process (e.g. with ./ser in the build
directory).
2. Start a client process (e.g. with ./civ -a
in the build directory).
3. Client started in (2) connects to the server started
in (1).
Not on my setup. It hangs in some silly reverse DNS lookup, because I'm
running behind a firewall -- as all good developers always do!
Depending on a reverse DNS lookup is a bad idea, as there are many users
that run NAT with RFC-1918 addresses, and it is an unreasonable load on
(wearing another hat) our root servers.
Depending on a DNS lookup of any kind completing is a terrible idea
But that's another ticket. I'm pretty sure there's an existing one.
4. Now as the client, press the button labelled Pick Nation.
Sounds like that should be sensitive to whether you have a player yet.
5. The races (a.k.a. nations) dialog pops-up.
6. Click any nation in the list (e.g. Assyrian).
7. The button labelled Ok becomes active.
8. Press the button labelled Ok.
9. The assertion failure occurs.
A few weeks ago branch S2_2 did not experience this assertion
failure. As I was using it in this exact manner for working
on the editor.
Since it doesn't occur with the more usual New Game page, I suggest that
you try it for your Editor experiments, until this is tracked down.
Or you could try checking out earlier revisions by binary search until you
find the revision number that caused the problem
Until somebody does that, it's unlikely to be fixed.
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110 Madeline Book wrote: I mean a server setting (in server/settings.c) like savepalace or killcitizen, I don't see why this should be for things (?) that change only from ruleset to ruleset Neither should be server settings. That's the old way. Most of these have been gradually moved to the (newer) effects rulesets. Likewise diplomats/spies, as that would completely destroy their stealth. Spies and diplomats do not have the Partial_Invis flag. Definitely a ruleset bug. There's some oddity with the V_INVIS layer and various hidden test functions. Probably later additions without sufficient documentation. for stealth fighters/bombers, consider that submarines can in fact be detected by their effect on city workers when their controlling player is foolish enough to move them into the field of the enemy city Again, definitely a bug. Consider also that an air or civilian unit can be used to bypass ZOC, implying that they do in fact control the tile, and hence should bounce city workers. Interesting argument. ZOC is changed considerably in civ3. Bad design decisions shouldn't be enshrined in perpetuity. However, none of this is relevant to this ticket, which is on the path to fixing crashing bugs ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40111) Diplomatic immunity
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40111 Madeline Book wrote: Spies and diplomats do not have the Partial_Invis flag. This is a bug. These units should not show up until adjacent to cities, and should be completely invisible to passing units. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40112) air units and submarines affecting city workers
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40112 Madeline Book wrote: for stealth fighters/bombers, consider that submarines can in fact be detected by their effect on city workers when their controlling player is foolish enough to move them into the field of the enemy city This is a bug, although it may need to be a ruleset option. Need to investigate actual performance of civ1/2/3. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40114) vestigial city_map removal from client/agents/cma_core.c
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40114
This is the last substantive use on the client side. For unknown reasons,
it parallels code in common/aicore/cm.c -- which is called and over-writes
the calculations!
The only advantage is this code has some debug counting and logging; that
remains in place.
Renamed some functions and iterators to match the common code.
Index: client/agents/cma_core.c
===
--- client/agents/cma_core.c(revision 14422)
+++ client/agents/cma_core.c(working copy)
@@ -80,11 +80,12 @@
int apply_result_ignored, apply_result_applied, refresh_forced;
} stats;
-#define my_city_map_iterate(pcity, cx, cy) { \
- city_tile_iterate_cxy(city_tile(pcity), cx##cy##_ptile, cx, cy) {\
-if (!is_free_worked(pcity, cx##cy##_ptile)) {
+/* simple extension to skip is_free_worked() tiles. */
+#define city_tile_iterate_skip_free(pcity, ptile, cx, cy) {\
+ city_tile_iterate_cxy(city_tile(pcity), ptile, cx, cy) { \
+if (!is_free_worked(pcity, ptile)) {
-#define my_city_map_iterate_end
\
+#define city_tile_iterate_skip_free_end
\
} \
} city_tile_iterate_cxy_end; \
}
@@ -113,13 +114,13 @@
T(surplus[stat]);
} output_type_iterate_end;
- my_city_map_iterate(pcity, x, y) {
+ city_tile_iterate_skip_free(pcity, ptile, x, y) {
if (result1-worker_positions_used[x][y] !=
result2-worker_positions_used[x][y]) {
freelog(RESULTS_ARE_EQUAL_LOG_LEVEL, worker_positions_used);
return FALSE;
}
- } my_city_map_iterate_end;
+ } city_tile_iterate_skip_free_end;
return TRUE;
}
@@ -130,32 +131,29 @@
/
Copy the current city state (citizen assignment, production stats and
happy state) in the given result.
+
+ This is parallel to cm_copy_result_from_city() with extra counting!
*/
-static void get_current_as_result(struct city *pcity,
- struct cm_result *result)
+static void my_copy_result_from_city(struct city *pcity,
+struct cm_result *result)
{
int worker = 0, specialist = 0;
memset(result-worker_positions_used, 0,
sizeof(result-worker_positions_used));
- my_city_map_iterate(pcity, x, y) {
-result-worker_positions_used[x][y] =
- (pcity-city_map[x][y] == C_TILE_WORKER);
-if (result-worker_positions_used[x][y]) {
+ city_tile_iterate_skip_free(pcity, ptile, x, y) {
+if (NULL != tile_worked(ptile) tile_worked(ptile) == pcity) {
worker++;
}
- } my_city_map_iterate_end;
+ } city_tile_iterate_skip_free_end;
specialist_type_iterate(sp) {
-result-specialists[sp] = pcity-specialists[sp];
specialist += pcity-specialists[sp];
} specialist_type_iterate_end;
assert(worker + specialist == pcity-size);
- result-found_a_valid = TRUE;
-
cm_copy_result_from_city(pcity, result);
}
@@ -197,7 +195,7 @@
bool success;
assert(result-found_a_valid);
- get_current_as_result(pcity, current_state);
+ my_copy_result_from_city(pcity, current_state);
if (results_are_equal(pcity, result, current_state)
!ALWAYS_APPLY_AT_SERVER) {
@@ -221,16 +219,17 @@
}
/* Remove all surplus workers */
- my_city_map_iterate(pcity, x, y) {
-if ((pcity-city_map[x][y] == C_TILE_WORKER)
- !result-worker_positions_used[x][y]) {
+ city_tile_iterate_skip_free(pcity, ptile, x, y) {
+if (NULL != tile_worked(ptile) tile_worked(ptile) == pcity
+ !result-worker_positions_used[x][y]) {
freelog(APPLY_RESULT_LOG_LEVEL, Removing worker at %d,%d., x, y);
+
last_request_id = city_toggle_worker(pcity, x, y);
if (first_request_id == 0) {
first_request_id = last_request_id;
}
}
- } my_city_map_iterate_end;
+ } city_tile_iterate_skip_free_end;
/* Change the excess non-default specialists to default. */
specialist_type_iterate(sp) {
@@ -253,17 +252,18 @@
/* Set workers */
/* FIXME: This code assumes that any toggled worker will turn into a
* DEFAULT_SPECIALIST! */
- my_city_map_iterate(pcity, x, y) {
-if (result-worker_positions_used[x][y]
- pcity-city_map[x][y] != C_TILE_WORKER) {
- assert(pcity-city_map[x][y] == C_TILE_EMPTY);
+ city_tile_iterate_skip_free(pcity, ptile, x, y) {
+if (NULL == tile_worked(ptile)
+ result-worker_positions_used[x][y]) {
freelog(APPLY_RESULT_LOG_LEVEL, Putting worker at %d,%d., x, y);
+ assert(city_can_work_tile(pcity, ptile));
+
last_request_id = city_toggle_worker(pcity, x, y);
if
Re: [Freeciv-Dev] (PR#40113) civclient: player.c:293: player_number: Assertion `pplayer' failed.
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40113 Cannot duplicate, running client with internal server. Tried random nation button (my usual test), random nation button on nation dialog, and picking specific nation. All work. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110 Madeline Book wrote: Unfortunately for you, posting a comment about a side-effect of your patch does not imply that the person posting the comment tested your patch. But never fear, there is PR#40113. Unfortunately for all of us, posting a comment about a side-effect of a patch *DOES* imply the person has tested the patch! The policy of posting patches here is not just for the keyboard exercise Moreover, PR#40113 is not reproducible on my machine(s). Hopefully, you will find the problem. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40113) civclient: player.c:293: player_number: Assertion `pplayer' failed.
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40113 This appears to be wrong. This might set client.playing for an observer, which would violate quite a bit of existing logic. It should not be possible for an observer to pick a nation. Is that what you tried? (You didn't mention that in your report.) ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40113) civclient: player.c:293: player_number: Assertion `pplayer' failed.
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40113 I have examined all instances of send_conn_info(), and with one exception, they seem to come after send_player_info() -- which is how client.playing is set. Please post the exact commands that you send. And have you tested with any previous version? Nothing in any recent patch (that I'm aware of) would affect this sequence. It might be helpful to figure out how long this has been a problem ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110
The first and most obvious part of the patch updates the tile/vision
documentation somewhat, as it was woefully out of date!
Renamed some of the enum known_type to reflect their source, and replaced
the order dependent = = tests.
Moved city_can_work_tile() out of server/citytools into common/city, to be
used in both client and server. This meant changing it somewhat, as it
used some server-only functions.
The upside is a new feature: air and civilian units no longer prevent
working a tile. This better conforms with usual expectations. See new
unit_occupies_tile().
Although the purpose of this series of patches is removal of conflicts
between city_map[] and tile_worked(), there was (at least) one use that
wasn't duplicated:
* seen tiles worked by cities that are not seen by the client!
The city_map[] has them marked TILE_UNAVAILABLE. In the server, the tile
is set, but not in the client.
Now, the city id is passed to the client. So the client has to make some
invisible cities (unknown center tile) to set the worked field.
That makes 2 client uses of NULL == city-tile (the other being the Editor).
The server still expects the tile field is always set.
Also, in borderless games, there's no tile_owner() to use for these
invisible cities, so an out-of-band (but in range) owner is used instead:
MAX_NUM_PLAYERS (in the reserved for barbarian range). The owner is
corrected as soon as the city is seen, or a border expands.
This required some other small modifications, primarily LOG_DEBUG.
Extensively tested on savegames of multiple bug reports. Seems to do what
is expected. I'm sure many more edge cases will be found
Index: doc/HACKING
===
--- doc/HACKING (revision 14420)
+++ doc/HACKING (working copy)
@@ -592,28 +592,29 @@
Unknown tiles and Fog of War
=
-In the tile struct there is a field
+In common/tile.h, there are several fields:
struct tile {
...
- unsigned int known;
+ bv_player tile_known, tile_seen[V_COUNT];
...
};
-On the server the known fields is considered to be a bitvector, one
-bit for each player, 0==tile unknown, 1==tile known.
-On the client this field contains one of the following 3 values:
+While tile_get_known() returns:
+/* network, order dependent */
enum known_type {
- TILE_UNKNOWN, TILE_KNOWN_FOGGED, TILE_KNOWN
+ TILE_UNKNOWN = 0,
+ TILE_KNOWN_UNSEEN = 1,
+ TILE_KNOWN_SEEN = 2,
};
-The values TILE_UNKNOWN, TILE_KNOWN are straightforward. TILE_FOGGED
-is a tile of which the user knows the terrain (inclusive cities, roads,
-etc...).
+The values TILE_UNKNOWN, TILE_KNOWN_SEEN are straightforward.
+TILE_KNOWN_UNSEEN is a tile of which the user knows the terrain,
+but not recent cities, roads, etc.
-TILE_UNKNOWN tiles are (or should be) never sent to the client. In the past
-UNKNOWN tiles that were adjacent to FOGGED or KNOWN ones were sent to make
+TILE_UNKNOWN tiles never are (nor should be) sent to the client. In the
+past, UNKNOWN tiles that were adjacent to UNSEEN or SEEN were sent to make
the drawing process easier, but this has now been removed. This means
exploring new land may sometimes change the appearance of existing land (but
this is not fundamentally different from what might happen when you
@@ -623,9 +624,10 @@
Fog of war is the fact that even when you have seen a tile once you are
not sent updates unless it is inside the sight range of one of your units
or cities.
+
We keep track of fog of war by counting the number of units and cities
[and nifty future things like radar outposts] of each client that can
-see the tile. This requires a number per player, per tile, so each tile
+see the tile. This requires a number per player, per tile, so each player_tile
has a short[]. Every time a unit/city/miscellaneous can observe a tile
1 is added to its player's number at the tile, and when it can't observe
any more (killed/moved/pillaged) 1 is subtracted. In addition to the
Index: server/cityhand.c
===
--- server/cityhand.c (revision 14420)
+++ server/cityhand.c (working copy)
@@ -185,11 +185,11 @@
return;
}
- if (!city_can_work_tile(pcity, ptile)) {
+ if (0 == city_specialists(pcity)) {
return;
}
- if (0 == city_specialists(pcity)) {
+ if (!city_can_work_tile(pcity, ptile)) {
return;
}
Index: server/citytools.c
===
--- server/citytools.c (revision 14420)
+++ server/citytools.c (working copy)
@@ -1979,40 +1979,6 @@
}
/**
- Returns TRUE when a tile is available to be worked, or the city itself is
- currently working the tile (and can continue).
- city_x, city_y is in city map coords.
Re: [Freeciv-Dev] (PR#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110 Madeline Book wrote: And as for appealing to realism (I assume that is what you mean by usual expectations, I meant the actual behavior of civ1/2/3. We only need options for things that change from ruleset to ruleset. Does an air unit bounce a city worker in civ1? Does an air unit bounce a city worker in civ2? Does an air unit bounce a city worker in civ3? My memory is that they do not, but it's been a long time. Likewise diplomats/spies, as that would completely destroy their stealth. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39872) Re: game.player_ptr should be moved to client.playing
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39872 Originally had thought about client.player, but changed it to .playing after considering the meaning. In freeciv, player is the civilization, not the user. That's what's somewhat confusing. The clients are playing (or observing) the civilization. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40106) tile_info with worked, adjust field order and size
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40106 Committed trunk revision 14419. Committed S2_2 revision 14420. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40105) vestigial tile spec_sprite removal
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40105
I'm completely unable to find anywhere that documents why spec_sprite is
actually useful?!?!
/* Skip the normal drawing process. */
/* FIXME: this should avoid calling load_sprite since it's slow and
* increases the refcount without limit. */
if (ptile-spec_sprite (sprite = load_sprite(t, ptile-spec_sprite))) {
It can be loaded/saved in a savegame, but none of my examples has it.
No rulesets load it, nothing ever sets it!?!?
===
Here's every current reference:
client/packhand.c:2200:
if (packet-spec_sprite[0] != '\0') {
if (!ptile-spec_sprite
|| strcmp(ptile-spec_sprite, packet-spec_sprite) != 0) {
if (ptile-spec_sprite) {
free(ptile-spec_sprite);
}
ptile-spec_sprite = mystrdup(packet-spec_sprite);
client/packhand.c:2210:
if (ptile-spec_sprite) {
free(ptile-spec_sprite);
ptile-spec_sprite = NULL;
client/tilespec.c:3988:
if (ptile-spec_sprite (sprite = load_sprite(t, ptile-spec_sprite))) {
common/map.c:283:
ptile-spec_sprite = NULL;
common/map.c:384:
if (ptile-spec_sprite) {
free(ptile-spec_sprite);
ptile-spec_sprite = NULL;
common/packets.def:355:
STRING spec_sprite[MAX_LEN_NAME];
common/tile.h:47:
char *spec_sprite;
server/maphand.c:643:
if (ptile-spec_sprite) {
sz_strlcpy(info.spec_sprite, ptile-spec_sprite);
server/maphand.c:646:
info.spec_sprite[0] = '\0';
server/savegame.c:640:
ptile-spec_sprite = secfile_lookup_str_default(file, NULL,
map.spec_sprite_%d_%d,
if (ptile-spec_sprite) {
ptile-spec_sprite = mystrdup(ptile-spec_sprite);
server/savegame.c:1047:
if (ptile-spec_sprite) {
secfile_insert_str(file, ptile-spec_sprite, map.spec_sprite_%d_%d,
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40106) tile_info with worked, adjust field order and size
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40106
Currently, the worked tile fields are not sent. Instead, the worked pointers
are set using the city_info packet city_map[].
This patch merely passes the worked city id value. A future patch will
change the client to use the field.
Also, this adjusts the terrain and resource number size to match reality, as
neither can ever be more than 96 (a limitation of using a printable ASCII
character in the map data). The sizes are now 96 and 48 respectively (was
200 and 100), and the packet data is uint8.
Instead of passing -1 as a special case out-of-range value to indicate none,
just pass the *_count() -- guaranteed to generate NULL on receipt, and easy
to grep in the code.
Index: server/ruleset.c
===
--- server/ruleset.c(revision 14418)
+++ server/ruleset.c(working copy)
@@ -3277,19 +3277,19 @@
packet.irrigation_result = (pterrain-irrigation_result
? terrain_number(pterrain-irrigation_result)
- : -1);
+ : terrain_count());
packet.irrigation_food_incr = pterrain-irrigation_food_incr;
packet.irrigation_time = pterrain-irrigation_time;
packet.mining_result = (pterrain-mining_result
? terrain_number(pterrain-mining_result)
- : -1);
+ : terrain_count());
packet.mining_shield_incr = pterrain-mining_shield_incr;
packet.mining_time = pterrain-mining_time;
packet.transform_result = (pterrain-transform_result
? terrain_number(pterrain-transform_result)
- : -1);
+ : terrain_count());
packet.transform_time = pterrain-transform_time;
packet.rail_time = pterrain-rail_time;
packet.clean_pollution_time = pterrain-clean_pollution_time;
Index: server/maphand.c
===
--- server/maphand.c(revision 14418)
+++ server/maphand.c(working copy)
@@ -639,7 +639,7 @@
info.x = ptile-x;
info.y = ptile-y;
- info.owner = tile_owner(ptile) ? player_number(tile_owner(ptile)) :
MAP_TILE_OWNER_NULL;
+
if (ptile-spec_sprite) {
sz_strlcpy(info.spec_sprite, ptile-spec_sprite);
} else {
@@ -652,41 +652,65 @@
if (!pplayer !pconn-observer) {
continue;
}
+
if (!pplayer || map_is_known_and_seen(ptile, pplayer, V_MAIN)) {
info.known = TILE_KNOWN;
- info.type = tile_terrain(ptile) ? terrain_number(tile_terrain(ptile)) :
-1;
+ info.continent = tile_continent(ptile);
+ info.owner = (NULL != tile_owner(ptile))
+? player_number(tile_owner(ptile))
+: MAP_TILE_OWNER_NULL;
+ info.worked = (NULL != tile_worked(ptile))
+ ? tile_worked(ptile)-id
+ : IDENTITY_NUMBER_ZERO;
+ info.terrain = (NULL != tile_terrain(ptile))
+ ? terrain_number(tile_terrain(ptile))
+ : terrain_count();
+ info.resource = (NULL != tile_resource(ptile))
+ ? resource_number(tile_resource(ptile))
+ : resource_count();
+
tile_special_type_iterate(spe) {
info.special[spe] = BV_ISSET(ptile-special, spe);
} tile_special_type_iterate_end;
- info.resource = tile_resource(ptile) ?
resource_number(tile_resource(ptile)) : -1;
- info.continent = tile_continent(ptile);
send_packet_tile_info(pconn, info);
} else if (pplayer map_is_known(ptile, pplayer)
map_get_seen(ptile, pplayer, V_MAIN) == 0) {
struct player_tile *plrtile = map_get_player_tile(ptile, pplayer);
info.known = TILE_KNOWN_FOGGED;
- info.type = plrtile-terrain ? terrain_number(plrtile-terrain) : -1;
+ info.continent = tile_continent(ptile);
+ info.owner = (NULL != tile_owner(ptile))
+ ? player_number(tile_owner(ptile))
+ : MAP_TILE_OWNER_NULL;
+ info.worked = IDENTITY_NUMBER_ZERO;
+ info.terrain = (NULL != plrtile-terrain)
+ ? terrain_number(plrtile-terrain)
+ : terrain_count();
+ info.resource = (NULL != plrtile-resource)
+ ? resource_number(plrtile-resource)
+ : resource_count();
+
tile_special_type_iterate(spe) {
info.special[spe] = BV_ISSET(plrtile-special, spe);
} tile_special_type_iterate_end;
- info.resource = plrtile-resource ? resource_number(plrtile-resource) :
-1;
- info.continent = tile_continent(ptile);
send_packet_tile_info(pconn, info);
} else if (send_unknown) {
info.known = TILE_UNKNOWN;
- info.type = -1;
+ info.continent = 0;
+ info.owner = MAP_TILE_OWNER_NULL;
+
Re: [Freeciv-Dev] (PR#40101) minplayers typo
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40101 I tried to figure out how that came into the code base, and wasn't successful. It's not in 2.0, and none of my searching found it. Ah well, I wish all our problems were so easy to solve Thanks! Committed trunk revision 14412. Committed S2_2 revision 14413. Committed S2_1 revision 14414. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40100) vestigial city_map removal
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40100 Just noting in passing that there's Yet Another Copy of the city worker in citymap.c, this time as an -(pcity-id). What a mess! We have a citymap (whole world), city_map for each city, the world tiles, and player tiles With conversions from city_maps to CMA that I don't even want to think about at the moment. Also, the packet interface to the client sends this data per city, instead of per tile, so every tiny change sends a huge city packet. The code spends an inordinate amount of time circling outward from tiles looking for adjacent cities to update. Unfortunately, it's sufficiently embedded that ripping the entire thing out entirely is too hard. I'll have to break it down into steps. The first step will be trying to come up with a sensible naming scheme for functions, and combining many cases of nearly identical code into common functions. Maybe that will be enough to figure out how a city size is munged every few dozen turns! ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40098) BUG: 2.2-test myrandomly() range check
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40098
In PR#40073, I moved the code from client/tilespec.c into utility/rand.c,
but now it seems to assert() on larger maps. Changed the assert() to allow
larger values. And use standard header definitions!
Index: utility/rand.c
===
--- utility/rand.c (revision 14405)
+++ utility/rand.c (working copy)
@@ -265,7 +265,7 @@
#define SMALL_PRIME (1009)
/* Check for overflow and underflow */
- assert((int)(seed * LARGE_PRIME) 0);
+ assert(seed MAX_UINT32 / LARGE_PRIME);
assert(size SMALL_PRIME);
assert(size 0);
result = ((seed * LARGE_PRIME) % SMALL_PRIME) % size;
Index: utility/rand.h
===
--- utility/rand.h (revision 14405)
+++ utility/rand.h (working copy)
@@ -13,12 +13,14 @@
#ifndef FC__RAND_H
#define FC__RAND_H
+#include stdint.h
+
#include shared.h/* bool type */
/* This is duplicated in shared.h to avoid extra includes: */
#define MAX_UINT32 0x
-typedef unsigned int RANDOM_TYPE;
+typedef uint32_t RANDOM_TYPE;
typedef struct {
RANDOM_TYPE v[56];
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40098) BUG: 2.2-test myrandomly() range check
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40098 Committed trunk revision 14405. Committed S2_2 revision 14406. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40099) BUG: 2.1 assert() failure attacking a city
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40099 An assert() failure is *always* a bug! It's something the developer added to test and any such failure should never happen. Could you reply with the message, and attach the savegame? ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40096) deterministic borders (part 2)
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40096
After more testing, the previous patch still expanded the range much too
quickly, so I've scaled it some more, with every size having some effect.
Here's the list for borders 4:
City 2.1now
Size r**2 r**2
1 4 2 unit
2 9 4
3 16 6
4 16 8 city
5 16 12
6 25 16
7 25 20
8 36 24
(26) AWACS
9 36 28
10 49 32
11 49 36
12 64 40
13 64 44
14 81 48
15 81 52
16100 56
Committed revision 14403.
Committed revision 14404, patch as committed:
Index: server/citytools.c
===
--- server/citytools.c (revision 14403)
+++ server/citytools.c (working copy)
@@ -42,6 +42,7 @@
#include tech.h
#include unit.h
#include unitlist.h
+#include vision.h
#include script.h
@@ -845,10 +846,10 @@
/* Has to follow the unfog call above. */
city_list_unlink(pgiver-cities, pcity);
pcity-owner = ptaker;
+ map_claim_ownership(pcity-tile, ptaker, pcity-tile);
city_list_prepend(ptaker-cities, pcity);
/* Update the national borders. */
- map_claim_ownership(pcity-tile, ptaker, pcity-tile);
map_claim_border(pcity-tile, ptaker);
transfer_city_units(ptaker, pgiver, old_city_units,
@@ -997,17 +998,21 @@
}
}
+ /* Claim the ground we stand on */
+ tile_set_worked(ptile, pcity); /* partly redundant to server_set_tile_city()
*/
+ tile_set_owner(ptile, saved_owner);
+ map_claim_ownership(ptile, pplayer, ptile);
+
/* Before arranging workers to show unknown land */
pcity-server.vision = vision_new(pplayer, ptile);
vision_reveal_tiles(pcity-server.vision, game.info.city_reveal_tiles);
city_refresh_vision(pcity);
-
- tile_set_worked(ptile, pcity); /* partly redundant to server_set_tile_city()
*/
city_list_prepend(pplayer-cities, pcity);
- /* Claim the ground we stand on */
- tile_set_owner(ptile, saved_owner);
- map_claim_ownership(ptile, pplayer, ptile);
+ /* This is dependent on the current vision, and affects the citymap
+ * tile status, so must be done after vision is prepared and before
+ * arranging workers. */
+ map_claim_border(ptile, pplayer);
if (terrain_control.may_road) {
tile_set_special(ptile, S_ROAD);
@@ -1046,10 +1051,6 @@
* ptile-worked does not point to it, it will give tile up. */
server_set_tile_city(pcity, CITY_MAP_SIZE/2, CITY_MAP_SIZE/2, C_TILE_WORKER);
- /* Update the national borders. This affects the citymap tile status,
- * so must be done after the above and before arranging workers. */
- map_claim_border(ptile, pplayer);
-
/* Refresh the city. First a city refresh is done (this shouldn't
* send any packets to the client because the city has no supported units)
* then rearrange the workers. Note that auto_arrange_workers does its
@@ -2002,16 +2003,16 @@
return FALSE;
}
- if (tile_owner(ptile) tile_owner(ptile) != powner) {
+ if (NULL != tile_owner(ptile) tile_owner(ptile) != powner) {
return FALSE;
}
/* TODO: civ3-like option for borders */
- if (ptile-worked ptile-worked != pcity) {
+ if (NULL != ptile-worked ptile-worked != pcity) {
return FALSE;
}
- if (!map_is_known(ptile, powner)) {
+ if (!map_is_known_and_seen(ptile, powner, V_MAIN)) {
return FALSE;
}
@@ -2229,8 +2230,26 @@
/
void city_refresh_vision(struct city *pcity)
{
+ struct tile *pcenter = city_tile(pcity);
+ struct player *powner = city_owner(pcity);
+ struct vision_site *psite = map_get_player_site(pcenter, powner);
int radius_sq = get_city_bonus(pcity, EFT_CITY_VISION_RADIUS_SQ);
+ if (NULL != psite) {
+int delta = psite-size - game.info.borders;
+
+/* TODO: city size effect or ruleset steps instead */
+if (0 = delta) {
+ psite-border_radius_sq = 2 * psite-size;
+} else {
+ psite-border_radius_sq = (2 + delta) * game.info.borders;
+}
+
+if (radius_sq psite-border_radius_sq) {
+ radius_sq = psite-border_radius_sq;
+}
+ }
+
vision_change_sight(pcity-server.vision, V_MAIN, radius_sq);
vision_change_sight(pcity-server.vision, V_INVIS, 2);
Index: server/maphand.c
===
--- server/maphand.c(revision 14403)
+++ server/maphand.c(working copy)
@@ -1763,6 +1763,13 @@
/*
Claim ownership of a single tile.
+
+ This is called for two reasons:
+ (1) Set a base or city. The tile_owner() MUST be any previous owner.
+ Because of update_city_tile_status_map() above, the city SHOULD NOT
+ be in the cities list yet. Also, before city_refresh_vision() as
+ that now depends on the vision_site.
+ (2)
[Freeciv-Dev] (PR#40100) vestigial city_map removal
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40100
I've asked on the developers list, and nobody knows a reason for this
anachronistic structure. Any time there are 2 different copies of the
same data, there's bound to be trouble keeping them synchronized.
The server side doesn't depend on its validity. It is refreshed just
before use in common/aicore/cm.c cm_query_result():
/* Refresh the city. Otherwise the CM can give wrong results or just be
* slower than necessary. Note that cities are often passed in in an
* unrefreshed state (which should probably be fixed). */
generic_city_refresh(pcity, TRUE);
===
There are a total of 37 references. Many of them (client and server)
could be replaced with a direct reference to the main map tiles!
Most of this code is devoted to updating it, and sanity checking it.
ai/aicity.c:171:
if (acity-city_map[x][y] == C_TILE_WORKER) {
client/agents/cma_core.c:144:
(pcity-city_map[x][y] == C_TILE_WORKER);
client/agents/cma_core.c:225:
if ((pcity-city_map[x][y] == C_TILE_WORKER)
client/agents/cma_core.c:258:
pcity-city_map[x][y] != C_TILE_WORKER) {
client/agents/cma_core.c:259:
assert(pcity-city_map[x][y] == C_TILE_EMPTY);
client/citydlg_common.c:899:
if (pcity-city_map[city_x][city_y] == C_TILE_WORKER) {
client/citydlg_common.c:902:
} else if (pcity-city_map[city_x][city_y] == C_TILE_EMPTY) {
client/packhand.c:603:
pcity-city_map[x][y] =
client/packhand.c:607:
set_worker_city(pcity, x, y, packet-city_map[i]);
client/packhand.c:864:
pcity-city_map[x][y] = C_TILE_EMPTY;
client/tilespec.c:4068:
worked[i] = citymode-city_map[cx][cy] == C_TILE_WORKER;
client/tilespec.c:4137:
citymode-city_map[cx][cy] == C_TILE_UNAVAILABLE)
common/aicore/cm.c:640:
if (pcity-city_map[x][y] == C_TILE_WORKER) {
common/aicore/cm.c:641:
pcity-city_map[x][y] = C_TILE_EMPTY;
common/aicore/cm.c:670:
pcity-city_map[tile-x][tile-y] = C_TILE_WORKER;
common/aicore/cm.c:1123:
if (pcity-city_map[x][y] == C_TILE_UNAVAILABLE) {
common/aicore/cm.c:1910:
(pcity-city_map[x][y] == C_TILE_WORKER);
common/aicore/cm.c:2049:
if (pcity-city_map[x][y] == C_TILE_WORKER) {
common/city.c:301:
if (pcity-city_map[city_x][city_y] == C_TILE_WORKER
common/city.c:314:
pcity-city_map[city_x][city_y] = type;
common/city.c:327:
return pcity-city_map[city_x][city_y];
common/city.c:1713:
if (pcity-city_map[x][y] == C_TILE_WORKER) {
common/city.h:329:
enum city_tile_type city_map[CITY_MAP_SIZE][CITY_MAP_SIZE];
server/citytools.c:1037:
pcity-city_map[x_itr][y_itr] = C_TILE_EMPTY;
server/citytools.c:1039:
pcity-city_map[x_itr][y_itr] = C_TILE_UNAVAILABLE;
server/citytools.c:1715:
packet-city_map[x + y * CITY_MAP_SIZE] = get_worker_city(pcity, x, y);
server/citytools.c:2043:
current = pcity-city_map[city_x][city_y];
server/cityturn.c:172:
switch (pcity-city_map[x][y]) {
server/cityturn.c:439:
C_TILE_WORKER == pcity-city_map[city_x][city_y]
server/sanitycheck.c:284:
switch (pcity-city_map[x][y]) {
server/sanitycheck.c:410:
if (pcity-city_map[city_x][city_y] != C_TILE_WORKER) {
server/sanitycheck.c:416:
pcity-city_map[city_x][city_y],
server/savegame.c:2473:
/* Initialize pcity-city_map[][], using set_worker_city() so that
server/savegame.c:2488:
pcity-city_map[x][y] = valid ? C_TILE_EMPTY : C_TILE_UNAVAILABLE;
server/savegame.c:2533:
assert(pcity-city_map[x][y] == C_TILE_UNAVAILABLE);
server/savegame.c:2558:
pcity-city_map[city_x][city_y] = C_TILE_WORKER;
server/savegame.c:3695:
switch (pcity-city_map[x][y]) {
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39563
Thank you for finding my mistake in PR#39553!
This worthless function is only called 4 places, and the first place I've
looked is:
ai/advmilitary.c:1223:
if (best_choice.want choice-want) {
/* We want attacker more than what we have selected before */
copy_if_better_choice(best_choice, choice);
That expands to:
if (best_choice.want choice-want) {
/* We want attacker more than what we have selected before */
if (choice-want best_choice-want) {
choice = best_choice;
So, this test (and function) is redundant
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39563
Here's my proposed patch to rid us of this troublesome beast
Index: ai/advdomestic.c
===
--- ai/advdomestic.c(revision 14391)
+++ ai/advdomestic.c(working copy)
@@ -336,17 +336,23 @@
init_choice(cur);
/* Consider building caravan-type units to aid wonder construction */
ai_choose_help_wonder(pcity, cur, ai);
-copy_if_better_choice(cur, choice);
+if (choice-want cur.want) {
+ *choice = cur;
+}
init_choice(cur);
/* Consider city improvements */
ai_advisor_choose_building(pcity, cur);
-copy_if_better_choice(cur, choice);
+if (choice-want cur.want) {
+ *choice = cur;
+}
init_choice(cur);
/* Consider building caravan-type units for trade route */
ai_choose_trade_route(pcity, cur, ai);
-copy_if_better_choice(cur, choice);
+if (choice-want cur.want) {
+ *choice = cur;
+}
}
if (choice-want = 200) {
Index: ai/aitools.c
===
--- ai/aitools.c(revision 14391)
+++ ai/aitools.c(working copy)
@@ -1163,16 +1163,6 @@
}
/**
-...
-**/
-void copy_if_better_choice(struct ai_choice *cur, struct ai_choice *best)
-{
- if (best-want cur-want) {
-best = cur;
- }
-}
-
-/**
...
**/
bool is_unit_choice_type(enum choice_type type)
Index: ai/aitools.h
===
--- ai/aitools.h(revision 14391)
+++ ai/aitools.h(working copy)
@@ -92,7 +92,6 @@
void init_choice(struct ai_choice *choice);
void adjust_choice(int value, struct ai_choice *choice);
-void copy_if_better_choice(struct ai_choice *cur, struct ai_choice *best);
bool is_unit_choice_type(enum choice_type type);
Index: ai/aicity.c
===
--- ai/aicity.c (revision 14391)
+++ ai/aicity.c (working copy)
@@ -1365,7 +1365,9 @@
!(ai_on_war_footing(pplayer) pcity-ai.choice.want 0
pcity-id != ai-wonder_city)) {
domestic_advisor_choose_build(pplayer, pcity, newchoice);
- copy_if_better_choice(newchoice, (pcity-ai.choice));
+ if (pcity-ai.choice.want newchoice.want) {
+pcity-ai.choice = newchoice;
+ }
}
}
Index: ai/advmilitary.c
===
--- ai/advmilitary.c(revision 14391)
+++ ai/advmilitary.c(working copy)
@@ -1218,9 +1218,10 @@
best_choice, ferryboat, boattype);
}
- if (best_choice.want choice-want) {
+ if (choice-want best_choice.want) {
/* We want attacker more than what we have selected before */
-copy_if_better_choice(best_choice, choice);
+*choice = best_choice;
+
CITY_LOG(LOG_DEBUG, pcity, kill_something_with()
%s has chosen attacker, %s, want=%d,
city_name(pcity),
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39563 OK, seems reasonable. I'd say commit. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40092) 2.2-test: fix update_dumb_city() missing update_vision_site_from_city()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40092
In PR#40072, the second reported error has Teotitlan shown with the wrong
owner. This patch does not fix the underlying issue, but is a step toward
making the savegame playable.
The savegame.c:2800 code tries to fix inconsistent games. This leads to
reality_check_city() and update_dumb_city(). I've straightened the logic in
update_dumb_city(), and added a bit more error checking. Found a bug in
PR#39895, missing one crucial line: update_vision_site_from_city().
Comparing the current code with 2.1 and 2.0, and walk-through of PR#13718
changes, found a possible improvement through waiting to update the national
borders (a few lines), matching older code in 2.0.
Other minor cleanup.
Index: server/citytools.c
===
--- server/citytools.c (revision 14391)
+++ server/citytools.c (working copy)
@@ -186,10 +186,9 @@
terrain_type_iterate(pterrain) {
/* Now we do the same for every available terrain. */
-goodness
- = is_terrain_near_tile(ptile, pterrain)
- ? nc-terrain[terrain_index(pterrain)]
- : -nc-terrain[terrain_index(pterrain)];
+goodness = is_terrain_near_tile(ptile, pterrain)
+ ? nc-terrain[terrain_index(pterrain)]
+ : -nc-terrain[terrain_index(pterrain)];
if (goodness 0) {
priority /= mult_factor;
} else if (goodness 0) {
@@ -998,12 +997,11 @@
vision_reveal_tiles(pcity-server.vision, game.info.city_reveal_tiles);
city_refresh_vision(pcity);
- tile_set_city(ptile, pcity);
+ tile_set_city(ptile, pcity); /* partly redundant to server_set_tile_city() */
city_list_prepend(pplayer-cities, pcity);
/* Claim the ground we stand on */
map_claim_ownership(ptile, pplayer, ptile);
- map_claim_border(ptile, pplayer);
if (terrain_control.may_road) {
tile_set_special(ptile, S_ROAD);
@@ -1031,7 +1029,7 @@
}
}
- /* Place a worker at the city center; this is the free-worked tile.
+ /* Place a worker at the is_city_center() is_free_worked_tile().
* This must be done before the city refresh (below) so that the city
* is in a sane state. */
/* Ugly detail here is that if another city is currently working
@@ -1042,6 +1040,10 @@
* ptile-worked does not point to it, it will give tile up. */
server_set_tile_city(pcity, CITY_MAP_SIZE/2, CITY_MAP_SIZE/2, C_TILE_WORKER);
+ /* Update the national borders. This affects the citymap tile status,
+ * so must be done after the above and before arranging workers. */
+ map_claim_border(ptile, pplayer);
+
/* Refresh the city. First a city refresh is done (this shouldn't
* send any packets to the client because the city has no supported units)
* then rearrange the workers. Note that auto_arrange_workers does its
@@ -1172,9 +1174,9 @@
/* idex_unregister_city() is called in game_remove_city() below */
- /* identity_number_release(pcity-id) is *NOT* done! The cities may still be
- alive in the client, or in the player map. The number of removed
- cities is small, so the loss is acceptable.
+ /* identity_number_release(pcity-id) is *NOT* done! The cities may
+ still be alive in the client, or in the player map. The number of
+ removed cities is small, so the loss is acceptable.
*/
old_vision = pcity-server.vision;
@@ -1730,10 +1732,11 @@
bool update_dumb_city(struct player *pplayer, struct city *pcity)
{
bv_imprs improvements;
- struct vision_site *pdcity = map_get_player_city(pcity-tile, pplayer);
+ struct tile *ptile = city_tile(pcity);
+ struct vision_site *pdcity = map_get_player_city(ptile, pplayer);
/* pcity-occupied isn't used at the server, so we go straight to the
* unit list to check the occupied status. */
- bool occupied = (unit_list_size(pcity-tile-units) 0);
+ bool occupied = (unit_list_size(ptile-units) 0);
bool walls = city_got_citywalls(pcity);
bool happy = city_happy(pcity);
bool unhappy = city_unhappy(pcity);
@@ -1747,27 +1750,32 @@
} improvement_iterate_end;
if (NULL == pdcity) {
+map_get_player_tile(ptile, pplayer)-site =
pdcity = create_vision_site_from_city(pcity);
-map_get_player_tile(pcity-tile, pplayer)-site = pdcity;
- } else if (pdcity-identity == pcity-id
- vision_owner(pdcity) == city_owner(pcity)
- pdcity-size == pcity-size
- 0 == strcmp(pdcity-name, city_name(pcity))
- pdcity-occupied == occupied
+ } else if (pdcity-location != ptile) {
+freelog(LOG_ERROR, Trying to update bad city (wrong location)
+at %i,%i for player %s,
+ TILE_XY(pcity-tile),
+ player_name(pplayer));
+pdcity-location = ptile; /* ?? */
+ } else if (pdcity-identity != pcity-id) {
+freelog(LOG_ERROR, Trying to update old city (wrong identity)
+at %i,%i for player %s,
+ TILE_XY(pcity-tile),
+
Re: [Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything (fix)
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39563 Not having seen anything in 12 hours, and the proposed patch being so trivial, Committed trunk revision 14396. Committed S2_2 revision 14397. Thanks, Marko! ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40088) 2.2-test: wrong city size after attack
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40088 This is implied by the sanitycheck repair in PR#40086, that presumably happened in the PR#40072 savegame. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40086) city center tiles not worked, bad sanity check repair?
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40086 Leaving open, as this needs to to be retrofitted for 2.1, too. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40087) client needs sanity check and other server log messages
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40087
Moved the existing ruleset_error() intercept into the con_handle_log()
callback, so that server log messages can be sent to all connections.
Currently, only LOG_FATAL and LOG_ERROR.
Renamed E_MESSAGE_WALL to E_LOG_FATAL, as that was already used for the
ruleset_error() and operator message popups.
Renamed E_PLAYER_SETTINGS (unused) to E_LOG_ERROR, and these default to
chat/output only. Used instead of E_LOG_FATAL for scripting.
Changed the event sections to be properly translatable. Swapped the
data fields for easier sorting (by event instead of strings). Likewise
for the soundspec file. Used some existing sounds.
Committed trunk revision 14390.
Committed S2_2 revision 14391.
Index: server/console.c
===
--- server/console.c(revision 14389)
+++ server/console.c(working copy)
@@ -25,9 +25,11 @@
#include fciconv.h
#include fcintl.h
+#include game.h
#include log.h
#include support.h
+#include plrhand.h
#include srv_main.h
#include console.h
@@ -47,6 +49,21 @@
/
static void con_handle_log(int level, const char *message, bool file_too)
{
+ if (LOG_ERROR == level) {
+notify_conn(NULL, NULL, E_LOG_ERROR, message);
+ } else if (LOG_FATAL = level) {
+/* Make sure that message is not left to buffers when server dies */
+conn_list_iterate(game.est_connections, pconn) {
+ pconn-send_buffer-do_buffer_sends = 0;
+ pconn-compression.frozen_level = 0;
+} conn_list_iterate_end;
+
+notify_conn(NULL, NULL, E_LOG_FATAL, message);
+notify_conn(NULL, NULL, E_LOG_FATAL,
+_(Please report this message at %s),
+BUG_URL);
+ }
+
/* Write debug/verbose message to console only when not written to file. */
if (!file_too || level = LOG_NORMAL) {
if (console_rfcstyle) {
Index: server/scripting/api.pkg
===
--- server/scripting/api.pkg(revision 14389)
+++ server/scripting/api.pkg(working copy)
@@ -419,7 +419,7 @@
E_BROADCAST_REPORT @ BROADCAST_REPORT,
E_GAME_END @ GAME_END,
E_GAME_START @ GAME_START,
-E_MESSAGE_WALL @ MESSAGE_WALL,
+E_LOG_ERROR @ E_LOG_ERROR,
E_NATION_SELECTED @ NATION_SELECTED,
E_DESTROYED @ DESTROYED,
E_REPORT @ REPORT,
Index: server/ruleset.c
===
--- server/ruleset.c(revision 14389)
+++ server/ruleset.c(working copy)
@@ -140,19 +140,8 @@
va_start(args, format);
- if (LOG_FATAL = loglevel) {
-/* Make sure that message is not left to buffers when server dies */
-conn_list_iterate(game.est_connections, pconn) {
- pconn-send_buffer-do_buffer_sends = 0;
- pconn-compression.frozen_level = 0;
-} conn_list_iterate_end;
+ vreal_freelog(loglevel, format, args);
-vreal_freelog(loglevel, format, args);
-notify_conn(NULL, NULL, E_MESSAGE_WALL, _(Fatal ruleset error. Server
dies!));
- } else {
-vreal_freelog(loglevel, format, args);
- }
-
va_end(args);
if (LOG_FATAL = loglevel) {
Index: server/stdinhand.c
===
--- server/stdinhand.c (revision 14389)
+++ server/stdinhand.c (working copy)
@@ -1636,8 +1636,8 @@
static bool wall(char *str, bool check)
{
if (!check) {
-notify_conn(NULL, NULL, E_MESSAGE_WALL,
- _(Server Operator: %s), str);
+notify_conn(NULL, NULL, E_LOG_FATAL,
+ _(Server Operator: %s), str);
}
return TRUE;
}
Index: data/stdsounds.soundspec
===
--- data/stdsounds.soundspec(revision 14389)
+++ data/stdsounds.soundspec(working copy)
@@ -194,54 +194,38 @@
; This list contains all events up to E_TECH_GOAL
; (as numbered in common/events.h), in the sorted order.
-; Alphabetical sorting is based on event descriptions,
-; not to these tag names. These have been sorted using
-; Freeciv internal language (en_US)
+; Alphabetical sorting is based on these tag names,
+; as the message names and contents change. {was}
;e_ai_debug =
+;e_anarchy =
+;e_bad_command =
;e_broadcast_report =
;e_caravan_action =
;e_chat_error =
;e_chat_msg =
+;e_city_aq_building =
+;e_city_aqueduct =
+;e_city_build =
;e_city_cantbuild =
-;e_city_lost =
-;e_city_love =
+;e_city_cma_release =
;e_city_disorder =
;e_city_famine =
;e_city_famine_feared =
+;e_city_gran_throttle =
;e_city_growth =
+;e_city_lost =
+;e_city_love =
;e_city_may_soon_grow =
-;e_city_aqueduct =
-;e_city_aq_building =
;e_city_normal =
;e_city_nuked =
;e_city_production_changed =
-;e_city_cma_release =
-;e_city_gran_throttle =
;e_city_transfer =
-;e_city_build =
-;e_worklist =
-;e_uprising =
;e_civil_war =
[Freeciv-Dev] (PR#40087) client needs sanity check and other server log messages
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40087 Rewrite sanitycheck, and intercept other freelog() error and fatal messages. Send via the Chat/Event message interface. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40086) 2.2-test: city center tiles not worked
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40086
Jason Dorje Short wrote:
The CM code is in common/aicore/cm.c.
Well, I've spent a bit of time looking at that, and its pretty difficult.
Whomever wrote that was a genius, or an idiot
Anyway, using the old tried and true pedestrian debugging techniques, I've
found one very nasty place that this happens: IN THE SANITY CHECK!!!
Somebody felt the need to do more than check, without any message or
indication to the user!
In server/sanitycheck.c real_sanity_check_city():
if (workers + city_specialists(pcity) != pcity-size + 1) {
int diff = pcity-size + 1 - workers - city_specialists(pcity);
SANITY_CITY(pcity, workers + city_specialists(pcity) == pcity-size + 1);
...
if (diff 0) {
city_map_checked_iterate(pcity-tile, city_x, city_y, ptile) {
if (ptile-worked == pcity diff 0) {
server_remove_worker_city(pcity, city_x, city_y);
diff++;
}
} city_map_checked_iterate_end;
}
The first worker found is the city center!
Now, I still don't know how the dickens the city size is bad, but the
cure is worse than the disease
===
Other server_remove_worker_city() calls elsewhere have a check before them,
for example:
if (!is_valid || is_free_worked_tile(city_map_x, city_map_y)) {
/* Can't remove a worker here. */
continue;
}
server_remove_worker_city(acity, city_map_x, city_map_y);
===
This is called often -- sanitycheck needs a thorough cleaning. And all
these error messages need to be sent to the client. Currently, there's no
easy way for them to be seen during the game.
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40085) add maxconnectionsperhost option
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40085 Somebody had sent me private email bragging about his/her DoS against some game server, and telling me this was needed. I didn't bother to reply. Anyway, per host blocking will adversely affect NATs and VPNs. The real DoS problem is TCB saturation -- that this won't fix. For security, the correct method is to exchange cookies between endpoints, and rate limit the exchange(s). As we proved in Photuris, and multiple papers for *BSD The DoS limit is how fast you can refuse and close connections, not some arbitrary number of concurrent connections per game. Therefore, I oppose such an option. The only sensible number will be the same as the number of players. It's such a small number already (30) that it won't make any difference. I've been working on a complete replacement for login (PR#39957, etc.) ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40075) S2_2 memory error in map_get_player_city
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40075 Michael Kaufman wrote: On Mon, Feb 04, 2008 at 07:49:58PM -0800, Jason Dorje Short wrote: ... Why not just use the ptile-index as the city id? ... ... without knowing the tile location that the city sits on (the capital city?) I really like Jason's idea. It might take time to implement, but it could ameliorate half a dozen recent bug reports that come to mind. And this would be a good time to eliminate the id field, as 2.2 will never generate savegames readable by earlier versions (because of new terrain). Since the id variable elimination will discover all existing uses, it's a good time to fix the latter, as well. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40080) [tracking] replace city id with tile pointer or index
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40080 William Allen Simpson wrote: There are at least 148 *city-id references in 44 files (and untold others with other pointer names). This is a seriously daunting task! The agents.c code passes city ids around. 284 references to homecity in 56 files (is a city id) 408 references to *city-tile in 60 files (the proposed replacement for id) 548 references to *unit-tile in 63 files (distinguished from homecity) Therefore, this is now a tracking ticket for the series of changes to make this happen over time ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40084) identity_number, server.game_identifier, city_tile(), unit_tile()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40084
Some of the preliminary steps toward the PR#40080 long-term goal. This
patch mostly clears the decks for shorter-term crashing bug fixes.
It includes some code already in 2.1 (PR#39980), used for initializing the
game_identifier. Moved from game to server structure (it is server-only),
and renamed to help distinguish from other id
Modified PR#40079, moving (and renaming) IDENTITY_NUMBER_ZERO to fc_types.h,
instead of 0 initializing city and unit ids.
Added access functions city_tile() and unit_tile(), renaming conflicting
variable names.
Index: utility/shared.c
===
--- utility/shared.c(revision 14385)
+++ utility/shared.c(working copy)
@@ -47,6 +47,7 @@
#include fcintl.h
#include log.h
#include mem.h
+#include rand.h
#include support.h
#include shared.h
@@ -79,6 +80,10 @@
static char *grouping = NULL;
static char *grouping_sep = NULL;
+static const char base64url[] =
+ ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_;
+
+
/***
Take a string containing multiple lines and create a copy where
each line is padded to the length of the longest line and centered.
@@ -390,22 +395,15 @@
/
bool is_safe_filename(const char *name)
{
- int i;
+ int i = 0;
/* must not be NULL or empty */
if (!name || *name == '\0') {
return FALSE;
}
- /* Accept only alphanumerics and '-', '_', '.' The exception is if
- * part of PARENT_DIR_OPERATOR is one of these, which is prohibited */
- for (i = 0; name[i]; i++) {
-if (!((name[i] = 'z' name[i] = 'a')
- || (name[i] = 'Z' name[i] = 'A')
- || (name[i] = '9' name[i] = '0')
- || name[i] == '-'
- || name[i] == '_'
- || name[i] == '.')) {
+ for (; '\0' != name[i]; i++) {
+if ('.' != name[i] NULL == strchr(base64url, name[i])) {
return FALSE;
}
}
@@ -457,6 +455,45 @@
return TRUE;
}
+/*
+ Check for valid base64url.
+*/
+bool is_base64url(const char *s)
+{
+ size_t i = 0;
+
+ /* must not be NULL or empty */
+ if (NULL == s || '\0' == *s) {
+return FALSE;
+ }
+
+ for (; '\0' != s[i]; i++) {
+if (NULL == strchr(base64url, s[i])) {
+ return FALSE;
+}
+ }
+ return TRUE;
+}
+
+/*
+ generate a random string meeting criteria such as is_ascii_name(),
+ is_base64url(), and is_safe_filename().
+*/
+void randomize_base64url_string(char *s, size_t n)
+{
+ size_t i = 0;
+
+ /* must not be NULL or too short */
+ if (NULL == s || 1 n) {
+return;
+ }
+
+ for (; i (n - 1); i++) {
+s[i] = base64url[myrand(sizeof(base64url) - 1)];
+ }
+ s[i] = '\0';
+}
+
/***
Produce a statically allocated textual representation of the given
year.
Index: utility/shared.h
===
--- utility/shared.h(revision 14385)
+++ utility/shared.h(working copy)
@@ -25,8 +25,9 @@
#endif
#endif
+/* Changing these will break network compatability! */
#define MAX_LEN_ADDR 256 /* see also MAXHOSTNAMELEN and RFC 1123 2.1 */
-#define MAX_LEN_PATH 4095
+#define MAX_LEN_PATH4095
/* Use FC_INFINITY to denote that a certain event will never occur or
another unreachable condition. */
@@ -144,8 +145,11 @@
const char *int_to_text(unsigned int number);
bool is_ascii_name(const char *name);
+bool is_base64url(const char *s);
bool is_safe_filename(const char *name);
+void randomize_base64url_string(char *s, size_t n);
const char *textyear(int year);
+
int compare_strings(const void *first, const void *second);
int compare_strings_ptrs(const void *first, const void *second);
Index: server/score.c
===
--- server/score.c (revision 14385)
+++ server/score.c (working copy)
@@ -399,7 +399,7 @@
return;
}
- fprintf(fp, P3\n# version:2\n# gameid: %s\n, game.id);
+ fprintf(fp, P3\n# version:2\n# gameid: %s\n, server.game_identifier);
fprintf(fp, # An intermediate map from saved Freeciv game %s%+05d\n,
game.save_name, game.info.year);
Index: server/srv_main.c
===
--- server/srv_main.c (revision 14385)
+++ server/srv_main.c (working copy)
@@ -1096,7 +1096,7 @@
}
/**
- Truncation of unsigned short wraps at 65K, skipping
Re: [Freeciv-Dev] (PR#40084) identity_number, server.game_identifier, city_tile(), unit_tile()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40084 Forgot to mention that it fixes the bug already reported in PR#40080, increasing the ai-stats.diplomat_reservations bit vector from 32767 (wrong) to 65536. Better not to overflow, even as a better solution may be developed in the future Committed trunk revision 14386. Committed S2_2 revision 14387. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40079) BUG! city unit ids not reserved, counter not saved/restored
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40079
While poking at the memory problems, tried moving city ids to avoid the
possible overrun, and was checking how ids were allocated. Discovered that
the running game never marks the used id numbers! And never checks they've
run out of id numbers!
Problem goes back as far as 2.0 (didn't look any farther back).
When savegames are loaded, the id used bits *are* reserved. But the counter
starts again at 100, so reloaded games will not have the same new ids after
that point in the game.
For Leave and Load games, the id counter and id used bits were not reset, so
the new ids start where the previous game finished.
Worse, because two different games could be loaded, the various saved bits
from the previous games are merged, further confounding the issues.
Although this may not be a problem in most games, for large or long games
some units' numbers may be duplicated. And it makes direct comparisons of
saved games absolutely impossible (unless both start at the beginning).
Redid the entire scheme (with symbols)! Set the 0th bit in the used array,
instead of testing against 0 every call. A tiny improvement in efficiency.
I'm now using server_game_init() properly, so the rulesets are no longer
loaded twice for --file games. Another small improvement in efficiency
Left in my debugging swaps of name and/or id for city and unit.
Other minor cleanup.
Index: server/srv_main.c
===
--- server/srv_main.c (revision 14380)
+++ server/srv_main.c (working copy)
@@ -129,10 +129,8 @@
*/
bool force_end_of_sniff;
-/* this counter creates all the id numbers used */
-/* use get_next_id_number() */
-static unsigned short global_id_counter=100;
-static unsigned char used_ids[8192]={0};
+#define IDENTITY_NUMBER_SIZE (1+MAX_UINT16)
+static unsigned char identity_numbers_used[IDENTITY_NUMBER_SIZE/8]={0};
/* server initialized flag */
static bool has_been_srv_init = FALSE;
@@ -196,7 +194,7 @@
init_character_encodings(FC_DEFAULT_DATA_ENCODING, FALSE);
/* Initialize callbacks. */
- game.callbacks.unit_deallocate = dealloc_id;
+ game.callbacks.unit_deallocate = identity_number_release;
/* done */
return;
@@ -1076,37 +1074,43 @@
/**
...
**/
-void dealloc_id(int id)
+void identity_number_release(int id)
{
- used_ids[id/8]= 0xff ^ (1(id%8));
+ identity_numbers_used[id/8] = 0xff ^ (1(id%8));
}
/**
...
**/
-static bool is_id_allocated(int id)
+void identity_number_reserve(int id)
{
- return TEST_BIT(used_ids[id / 8], id % 8);
+ identity_numbers_used[id/8] |= (1(id%8));
}
/**
...
**/
-void alloc_id(int id)
+static bool identity_number_is_used(int id)
{
- used_ids[id/8]|= (1(id%8));
+ return TEST_BIT(identity_numbers_used[id/8], id%8);
}
/**
-...
+ Truncation of unsigned short wraps at 65K, skipping VISION_SITE_NONE (0)
+ Setup in server_game_init()
**/
+int identity_number(void)
+{
+ int retries = 0;
-int get_next_id_number(void)
-{
- while (is_id_allocated(++global_id_counter) || global_id_counter == 0) {
-/* nothing */
+ while (identity_number_is_used(++server.identity_number)) {
+/* try again */
+if (++retries = IDENTITY_NUMBER_SIZE) {
+ die(exhausted city and unit numbers!);
+}
}
- return global_id_counter;
+ identity_number_reserve(server.identity_number);
+ return server.identity_number;
}
/**
@@ -1934,9 +1938,12 @@
#endif /* HAVE_AUTH */
/* load a saved game */
- if (srvarg.load_filename[0] != '\0') {
-(void) load_command(NULL, srvarg.load_filename, FALSE);
- }
+ if ('\0' == srvarg.load_filename[0]
+ || !load_command(NULL, srvarg.load_filename, FALSE)) {
+/* Rulesets are loaded on game initialization, but may be changed later
+ * if /load or /rulesetdir is done. */
+load_rulesets();
+ }
if(!(srvarg.metaserver_no_send)) {
freelog(LOG_NORMAL, _(Sending info to metaserver [%s]),
@@ -2131,13 +2138,14 @@
**/
void server_game_init(void)
{
- game_init();
-
+ /* was redundantly in game_load() */
server.nbarbarians = 0;
+ server.identity_number = IDENTITY_NUMBER_START;
- /* Rulesets are loaded on game initialization, but may be changed
Re: [Freeciv-Dev] (PR#40079) BUG! city unit ids not reserved, counter not saved/restored
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40079 Committed trunk revision 14381. Committed S2_2 revision 14382. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40075) S2_2 memory error in map_get_player_city
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40075
William Allen Simpson wrote:
==12774== Address 0x5391ca0 is 0 bytes inside a block of size 76 free'd
==12774==at 0x402365C: free (vg_replace_malloc.c:323)
==12774==by 0x80E6669: reality_check_city (citytools.c:1792)
playtile-site = NULL;
free(pdcity);
This goes back to the very reason that the border code was re-written!
(PR#39830) border expansion acquires destroyed city
In PR#39830, I'd tweaked the test without changing the logic (because the
problem was elsewhere):
-if (!pcity || (pcity pcity-id != pdcity-id)) {
+if (!pcity || pcity-id != pdcity-identity) {
Note that the second pcity was logically redundant.
But I missed another problem with the test, a problem that goes back years!
It may explain many bug reports! And the corrupted savegames!
dlsend_packet_city_remove(pplayer-connections, pdcity-identity);
playtile-site = NULL;
free(pdcity);
For the first clause (!pcity), sending city_remove should be OK.
But the second clause (pcity-id != pdcity-identity) is absolutely wrong.
There's still a city there, we shouldn't be freeing the dummy city, nor
telling the client to remove its city. Have to look further, but it seems
that we need to *update* the city, instead.
Now I have to look at all 10 calls to reality_check_city()
Also, determine how the city id might change. Building another city in the
same place?
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40069) end_turn() = map_calculate_borders() has bad site-location
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40069 In this report, tile0=0x2b. In PR#40068, tile0=0x4, and the client crashed, too. Let's keep this separate, to deal with the border recalculation issue. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40070) common/effects.c get_city_bonus() with NULL city
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40070 In this case, the traceback clearly shows the NULL city: city_owner (pcity=0x0) at city.c:654 That's properly failing checks added by PR#39998 and PR#39895 respectively: assert(NULL != pcity NULL != pcity-owner); But for the record, and as a reminder to myself, this assert should probably be split into two lines for better traceback indication. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40068) client gtk_progress_set_percentage() assertion
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40068 Jason Short wrote: Saving and loading doesn't generally give identical AI behavior so if this is caused by AI units moving or something then you won't get the same behavior from the savegame most likely. I've argued about this before (PR#39365), and it damn well SHOULD! I've spent a fair amount of time getting 2.1 to be reproducible, by checking and saving all myrand() states. Time to do again for 2.2! (Necessary, but tedious. Another ticket for another time.) Meanwhile, this report has two problems listed. Let's dedicate it to solving the client issue (leaving the server issue for the next ticket): (civclient:17913): Gtk-CRITICAL **: gtk_progress_set_percentage: assertion `percentage = 0 percentage = 1.0' failed 2: Verbindung zum Server verloren! The civclient.c line number is bogus, there are currently 807 lines. The log message is merely: #: client/clinet.c:143 client/clinet.c:144 msgid Lost connection to server! msgstr Verbindung zum Server verloren! So, how is gtk_progress_set_percentage() called? There's no direct reference anywhere in the code, and nothing seems relevant in clinet.c at all! It's probably unrelated. Anybody out there understand the GTK2 client enough to track this down? ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40068) client gtk_progress_set_percentage() assertion
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40068 Christian Knoke wrote: These 2 messages needn't necessarly be connected. I just copied the outbut on the terminal after the crash. Yes, and that's excellent bug reporting practice! That tells us that the client asserted sometime before the server died. Apparently, the assert() in GTK2 doesn't stop the client, unlike the assert() in the server But in any case, this client GTK2 message is something I've never seen, and we should figure out what happened. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40071) i18n: review LOG_FATAL and freeciv.org
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40071 Committed trunk revision 14376. Committed S2_2 revision 14377, with update-po to add TRANS comments. Reminder, ./autogen.sh before make! ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40073) BUG: 2.2-test AI building_want random interval not saved/restored
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40073
There's a problem with reproducing reports for 2.2 savegames. Over the past
9 months, I've spent considerable time improving the reproducibility of 2.1
(PR#39365, PR#39572, etc.)
Searching for myrand() differences between 2.1 and 2.2, found only 1 more
variable, added by PR#12682. It is used as both an interval *and* a weight
multiplier for calculating AI wants.
Renamed it to ai.building_wait, and save/restore it. (Yes, that's punny.)
Renamed the related AI recalc variables to have the same prefix_* as the
corresponding *_want counterparts (matching the names in the savegames).
Moved the pseudo-random equation from tilespec into rand, making it easier
to find, trace, and use elsewhere.
Other trivial cleanup.
Index: utility/rand.c
===
--- utility/rand.c (revision 14377)
+++ utility/rand.c (working copy)
@@ -78,8 +78,8 @@
directly representable in type RANDOM_TYPE, so we do instead:
divisor = MAX_UINT32/size
*/
-RANDOM_TYPE myrand_debug(RANDOM_TYPE size, const char *called_as, int line,
- const char *file)
+RANDOM_TYPE myrand_debug(RANDOM_TYPE size, const char *called_as,
+int line, const char *file)
{
RANDOM_TYPE new_rand, divisor, max;
int bailout = 0;
@@ -110,8 +110,8 @@
rand_state.v[rand_state.x] = new_rand;
if (++bailout 1) {
- freelog(LOG_ERROR, %s(%lu) = %lu bailout at line %d of %s,
- called_as, (unsigned long)size, (unsigned long)new_rand, line,
file);
+ freelog(LOG_ERROR, %s(%lu) = %lu bailout at %s:%d,
+ called_as, (unsigned long)size, (unsigned long)new_rand, file,
line);
new_rand = 0;
break;
}
@@ -124,8 +124,8 @@
new_rand = 0;
}
- freelog(LOG_RAND, %s(%lu) = %lu at line %d of %s,
- called_as, (unsigned long)size, (unsigned long)new_rand, line,
file);
+ freelog(LOG_RAND, %s(%lu) = %lu at %s:%d,
+ called_as, (unsigned long)size, (unsigned long)new_rand, file,
line);
return new_rand;
}
@@ -248,3 +248,31 @@
/* restore state: */
set_myrand_state(saved_state);
}
+
+/*
+ Local pseudo-random function for repeatedly reaching the same result,
+ instead of myrand(). Primarily needed for tiles.
+
+ Use an invariant equation for seed.
+ Result is 0 to (size - 1).
+*/
+RANDOM_TYPE myrandomly_debug(RANDOM_TYPE seed, RANDOM_TYPE size,
+const char *called_as, int line, const char *file)
+{
+ RANDOM_TYPE result;
+
+#define LARGE_PRIME (10007)
+#define SMALL_PRIME (1009)
+
+ /* Check for overflow and underflow */
+ assert((int)(seed * LARGE_PRIME) 0);
+ assert(size SMALL_PRIME);
+ assert(size 0);
+ result = ((seed * LARGE_PRIME) % SMALL_PRIME) % size;
+
+ freelog(LOG_RAND, %s(%lu,%lu) = %lu at %s:%d,
+ called_as, (unsigned long)seed, (unsigned long)size,
+ (unsigned long)result, file, line);
+
+ return result;
+}
Index: utility/rand.h
===
--- utility/rand.h (revision 14377)
+++ utility/rand.h (working copy)
@@ -26,8 +26,8 @@
bool is_init;/* initially 0 for static storage */
} RANDOM_STATE;
-#define myrand(vsize) myrand_debug((vsize), myrand, \
-__LINE__, __FILE__)
+#define myrand(_size) \
+ myrand_debug((_size), myrand, __LINE__, __FILE__)
RANDOM_TYPE myrand_debug(RANDOM_TYPE size, const char *called_as,
int line, const char *file);
@@ -40,4 +40,12 @@
void test_random1(int n);
+/*===*/
+
+#define myrandomly(_seed, _size) \
+ myrandomly_debug((_seed), (_size), myrandomly, __LINE__, __FILE__)
+
+RANDOM_TYPE myrandomly_debug(RANDOM_TYPE seed, RANDOM_TYPE size,
+const char *called_as, int line, const char *file);
+
#endif /* FC__RAND_H */
Index: server/stdinhand.c
===
--- server/stdinhand.c (revision 14377)
+++ server/stdinhand.c (working copy)
@@ -2357,7 +2357,6 @@
} else {
pcity-debug = TRUE;
CITY_LOG(LOG_TEST, pcity, debugged);
- pcity-ai.next_recalc = 0; /* force recalc of city next turn */
}
} else if (ntokens 0 strcmp(arg[0], units) == 0) {
int x, y;
Index: server/savegame.c
===
--- server/savegame.c (revision 14377)
+++ server/savegame.c (working copy)
@@ -2533,21 +2533,30 @@
worklist_load(file, pcity-worklist, player%d.c%d, plrno, i);
/* FIXME: remove this when the urgency is properly recalculated. */
-
Re: [Freeciv-Dev] (PR#40072) map displays wrong nation for enemy city
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40072 Thanks, looking ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40025) Happiness plurals
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40025 Christian Knoke wrote: This is in there since some time of 2.1. Something has been changed intendedly, I don't remember what it was. Wasn't paying much attention at the time, but it made the empire happiness code actually work! As it was intended! The default settings still aren't as stringent as commercial civ, where the effects kick in at 10 I have noticed this earlier and complained about. It is impossible now to grow large empires (with happiness), and it may be very hard to at least manage them. Well, you can always change the settings for a new ruleset: data/default/effects.ruleset: Empire_Size_Base Empire_Size_Step That's what Joan was asking for: better documentation. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40073) BUG: 2.2-test AI building_want random interval not saved/restored
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40073 Committed trunk revision 14378. Committed S2_2 revision 14379. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40069) end_turn() = map_calculate_borders() has bad site-location
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40069
I've poked and prodded this, and cannot get it to crash again. My guess is
that it's an array overrun of some kind from elsewhere, as the -location is
the first entry in the common/vision.h struct vision_site{}. The values in
the reports might be consistent with a string on a little-endian machine.
Jason, could you valgrind the PR#40068 savegame on a little-endian machine
and see whether you can detect anything?
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40068) server crash
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40068 Jason Short wrote: Looks to me like this means dsite-location is invalid. I have no familiarity with this code however. That's (my) very new border code. I'll take a look at it. What I don't understand is why it's not reproducible? Usually, that means an uninitialized variable, but all these structures are zeroed with calloc(), so that's unlikely ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#10400) untranslated LOG_NORMAL messages
URL: http://bugs.freeciv.org/Ticket/Display.html?id=10400 This has been waiting since 2004 Oct -- really shouldn't let better be the enemy of good. There are many more LOG_FATAL and LOG_ERROR than LOG_NORMAL. Will do others later, as they are likely reductions in translation rather than more translation Committed S2_2 revision 14370, with update-po to add TRANS comments. Committed trunk revision 14371. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40063) RFE: increase historian levels to prime number
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40063
Added +1 to ensure only top rank is rated Supreme (unless tied).
Added clamp to final entry.
Committed S2_2 revision 14364.
Committed trunk revision 14365.
Index: server/report.c
===
--- server/report.c (revision 14363)
+++ server/report.c (working copy)
@@ -149,39 +149,52 @@
char key;
} coltable[] = {{'q'}, {'r'}, {'b'}}; /* Corresponds to dem_flag enum */
-/**
-...
-**/
-static int secompare(const void *a, const void *b)
-{
- return (((const struct player_score_entry *)b)-value -
- ((const struct player_score_entry *)a)-value);
-}
-
-static const char *greatness[] = {
- /* TRANS: 1: The ranking Poles */
+/* prime number of entries makes for better scaling */
+static const char *ranking[] = {
+ /* TRANS: #: The ranking Poles */
+ N_(%2d: The Supreme %s),
+ /* TRANS: #: The ranking Poles */
N_(%2d: The Magnificent %s),
- /* TRANS: 2: The ranking Poles */
+ /* TRANS: #: The ranking Poles */
N_(%2d: The Great %s),
- /* TRANS: 3: The ranking Poles */
+ /* TRANS: #: The ranking Poles */
N_(%2d: The Glorious %s),
- /* TRANS: 4: The ranking Poles */
+ /* TRANS: #: The ranking Poles */
N_(%2d: The Excellent %s),
- /* TRANS: 5: The ranking Poles */
+ /* TRANS: #: The ranking Poles */
+ N_(%2d: The Eminent %s),
+ /* TRANS: #: The ranking Poles */
+ N_(%2d: The Distinguished %s),
+ /* TRANS: #: The ranking Poles */
N_(%2d: The Average %s),
- /* TRANS: 6: The ranking Poles */
+ /* TRANS: #: The ranking Poles */
N_(%2d: The Mediocre %s),
- /* TRANS: 7: The ranking Poles */
+ /* TRANS: #: The ranking Poles */
+ N_(%2d: The Ordinary %s),
+ /* TRANS: #: The ranking Poles */
N_(%2d: The Pathetic %s),
- /* TRANS: 8: The ranking Poles */
+ /* TRANS: #: The ranking Poles */
N_(%2d: The Useless %s),
- /* TRANS: 9: The ranking Poles */
+ /* TRANS: #: The ranking Poles */
+ N_(%2d: The Valueless %s),
+ /* TRANS: #: The ranking Poles */
N_(%2d: The Worthless %s),
+ /* TRANS: #: The ranking Poles */
+ N_(%2d: The Wretched %s),
};
/**
...
**/
+static int secompare(const void *a, const void *b)
+{
+ return (((const struct player_score_entry *)b)-value -
+ ((const struct player_score_entry *)a)-value);
+}
+
+/**
+...
+**/
static void historian_generic(enum historian_type which_news)
{
int i, j = 0, rank = 0;
@@ -219,18 +232,17 @@
qsort(size, j, sizeof(size[0]), secompare);
buffer[0] = '\0';
for (i = 0; i j; i++) {
-if (i == 0 || size[i].value size[i - 1].value) {
- if (i = sizeof(greatness)) {
-rank = sizeof(greatness) - 1;
- } else if (j = sizeof(greatness)) {
-rank = i;
- } else {
-rank = (i * sizeof(greatness)) / j;
- }
+if (i 0 size[i].value size[i - 1].value) {
+ /* since i j, only top entry reigns Supreme */
+ rank = ((i * sizeof(ranking)) / j) + 1;
}
+if (rank = sizeof(ranking)) {
+ /* clamp to final entry */
+ rank = sizeof(ranking) - 1;
+}
cat_snprintf(buffer, sizeof(buffer),
-_(greatness[rank]),
-rank + 1,
+_(ranking[rank]),
+i + 1,
nation_plural_for_player(size[i].player));
mystrlcat(buffer, \n, sizeof(buffer));
}
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40064) historian report passes NULL format to cat_snprintf()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40064 Jason Short wrote: Looks like this is cause by 40063. That would be highly surprising, as PR#40063 had not been committed yet! Anyway, I've just committed it minutes ago, and I'll do some testing now with the savegame provided Thanks! ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40064) historian report passes NULL format to cat_snprintf()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40064
My trivial patch was only meant to change an existing array from sentence
fragments for easier translation. Then, it ballooned from fixed size to
variable, and fancier, and
Used sizeof() instead of ARRAY_SIZE(). Fixed.
Excellent report with savegame! Thanks!
Committed S2_2 revision 14366.
Committed trunk revision 14367.
Index: server/report.c
===
--- server/report.c (revision 14365)
+++ server/report.c (working copy)
@@ -234,11 +234,11 @@
for (i = 0; i j; i++) {
if (i 0 size[i].value size[i - 1].value) {
/* since i j, only top entry reigns Supreme */
- rank = ((i * sizeof(ranking)) / j) + 1;
+ rank = ((i * ARRAY_SIZE(ranking)) / j) + 1;
}
-if (rank = sizeof(ranking)) {
+if (rank = ARRAY_SIZE(ranking)) {
/* clamp to final entry */
- rank = sizeof(ranking) - 1;
+ rank = ARRAY_SIZE(ranking) - 1;
}
cat_snprintf(buffer, sizeof(buffer),
_(ranking[rank]),
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40062) server/stdinhand.c duplicates server/command.c text
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40062 I looked at cmd_reply_prefix(), and chose not to use it for the Usage: lines. It really isn't multi-lingual friendly. Saved for another ticket. I looked at command_named(). It should be re-written to be more like find_player_by_name_prefix(). Saved for another ticket. Committed trunk revision 14368. Committed S2_2 revision 14369. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40066) i18n: commands and settings text should work for LtR and RtL, and wordwrap
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40066 In PR#40043, the disfunctional wordwrap was removed for server/stdinhand.c (stdout) and others. In PR#40062, the Usage: could sometimes use some wordwrap and prettifying. Ideally, the server/commands.c and server/settings.c text should not have their own \n, and should rely on the cmd_reply() and cmd_reply_prefix() mechanisms to handle lines. In civmanual, any \n should to be changed to br for html, but should primarily rely on the browser for wordwrap. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40058) mystrerror() and local encoding
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40058 Jason Short wrote: Patch is fine but the comment is a bit misleading - generally the returned value is passed off to GTK which wants it in UTF-8 (aka the internal encoding). When used in the server this may not be the case. Changed comment to: + The string is converted as necessary from the local_encoding + to internal_encoding, for inclusion in translations. May be + subsequently converted back to local_encoding for display. Even in the server, it will *always* be passed to either freelog() or some other *my* variant of printf(). ALL OF THEM need to use internal encoding, not some other encoding. Otherwise, we'd have a lot more bug reports. Anything else -- bare printf() to stderror -- is a bug. Unfortunately, there are ~170 bare printf remaining, mostly in gui-sdl, gui-ftwl, and lua. Oh well, another bug for another ticket (each). Anyway, since it's confirmed: Committed S2_1 revision 14342. Committed S2_2 revision 14343. Committed trunk revision 14344. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40057) strerror-mystrerror
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40057 Jason Dorje Short wrote: If you are asking about encodings, gui-sdl uses the call inside freelog which expects the string in the internal encoding (utf-8). OK. Can we remove gui-mui? Already done (2.2 and beyond). ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40036) 2.1.2 savegame data loss on rapid saves
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40036 Jason Dorje Short wrote: Unfortunately I don't seem to get this behavior anymore, though people keep reporting its symptoms. Every time I kill my client the server saves the game properly, without aitoggling the human. That supposedly was fixed, although I cannot find the PR# or the commit common/game.h:297: #define GAME_DEFAULT_AUTO_AI_TOGGLE FALSE Martin Rubinstein wrote: # a) How do I set autosave to each turn. (It is hidden in the easy distribution) It's documented several ways, have you tried the extensive Help menus? The easiest is typing /saveturns 1 in the Chat command line. For the 2.1 GTK2 gui, you'll find it under menu Game - Server Options - Internal - saveturns (Turns per auto-save) # b) Is there any easy way of saving a list of further options, for use at each load. There are several ways, have you tried Game - Save Options? And while you are looking at Game - Server Options, you might check the value of Network - autotoggle (Whether AI-status toggles with connection) We're speculating that you accidentally set it to On? ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40006) Placeholder strings like %d for gold amount isn't replaced
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40006 As I mentioned in PR#40007, I cannot even get them to display, other than for the OK, Yes, No, and Cancel buttons. Jason says ... the GTK library is using japanese correctly, but freeciv is still using the untranslated strings. I have trouble with this myself and aside from making sure you have gettext installed and linked My configure claims that I have gettext, and certainly the 'make update-po' and such seem to run fine. LANG=zh_CN ./civ Encodings: Data=UTF-8, Local=ISO-8859-1, Internal=UTF-8 2: No real audio plugin present. 2: Proceeding with sound support disabled 2: For sound support, install SDL_mixer 2: http://www.libsdl.org/projects/SDL_mixer/index.html LANG=zh_CN.UTF-8 ./civ Encodings: Data=UTF-8, Local=UTF-8, Internal=UTF-8 2: No real audio plugin present. 2: Proceeding with sound support disabled 2: For sound support, install SDL_mixer 2: http://www.libsdl.org/projects/SDL_mixer/index.html 2: Old attributes detected and removed. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40006) Placeholder strings like %d for gold amount isn't replaced
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40006 Christian Prochaska wrote: Did you make install before starting the client? Nope, should not be needed. I'm running the client (with ./civ) in the build directory itself. All the default paths seem to be working: ./data, etc. The game runs. Only the local translations don't ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40060) civserver core dump on debug command
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40060 Christian Knoke wrote: debug Speicherzugriffsfehler (core dumped) Unable to reproduce. Works here for both 2.1 and 2.2. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40062) server/stdinhand.c duplicates server/command.c text
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40062
Originally reported in PR#40061 for the debug command, investigation showed
that many of the commands had duplicate command usage synopses. Moreover,
the server/commands.c version was often out-of-date.
Merge all usage synopses into server/commands.c, and add missing TRANS
comments. Use proper access functions to check validity.
Index: server/commands.c
===
--- server/commands.c (revision 14344)
+++ server/commands.c (working copy)
@@ -21,9 +21,18 @@
#include commands.h
+struct command {
+ const char *name; /* name - will be matched by unique prefix */
+ enum cmdlevel_id level; /* access level required to use the command */
+ const char *synopsis; /* one or few-line summary of usage */
+ const char *short_help; /* one line (about 70 chars) description */
+ const char *extra_help; /* extra help information; will be line-wrapped */
+};
+
/* Commands must match the values in enum command_id. */
-const struct command commands[] = {
+static struct command commands[] = {
{start,ALLOW_INFO,
+ /* no translatable parameters */
start,
N_(Start the game, or restart after loading a savegame.),
N_(This command starts the game. When starting a new game,
@@ -55,6 +64,7 @@
},
{list, ALLOW_INFO,
+ /* no translatable parameters */
list\n
list players\n
list teams\n
@@ -66,6 +76,7 @@
and defaults to 'players' if absent.)
},
{quit, ALLOW_HACK,
+ /* no translatable parameters */
quit,
N_(Quit the game and shutdown the server.), NULL
},
@@ -99,12 +110,14 @@
or options with that prefix.)
},
{wall, ALLOW_HACK,
+ /* TRANS: translate text between only */
N_(wall message),
N_(Send message to all connections.),
N_(For each connected client, pops up a window showing the message
entered.)
},
{vote, ALLOW_INFO,
+ /* TRANS: translate text between [] only */
N_(vote yes|no [vote number]),
N_(Cast a vote.),
/* xgettext:no-c-format */
@@ -119,17 +132,21 @@
case if nobody votes against it.)
},
{debug,ALLOW_CTRL,
- N_(debug [ player player | city x y | units x y | unit id
- | tech player | timing | info]),
+ /* no translatable parameters */
+ debug [ diplomacy | ferries | player player | tech player
+| city x y | units x y | unit id
+| timing | info ],
N_(Turn on or off AI debugging of given entity.),
N_(Print AI debug information about given entity and turn continous
debugging output for this entity on or off.),
},
{set, ALLOW_CTRL,
+ /* TRANS: translate text between only */
N_(set option-name value),
N_(Set server option.), NULL
},
{team, ALLOW_CTRL,
+ /* TRANS: translate text between only */
N_(team player [team]),
N_(Change, add or remove a player's team affiliation.),
N_(Sets a player as member of a team. If no team specified, the
@@ -139,6 +156,7 @@
with averaged individual scores.)
},
{rulesetdir, ALLOW_CTRL,
+ /* TRANS: translate text between only */
N_(rulesetdir directory),
N_(Choose new ruleset directory or modpack.),
N_(Choose new ruleset directory or modpack. Calling this\n
@@ -160,6 +178,7 @@
N_(Set metaserver patches line.), NULL
},
{metaconnection, ALLOW_HACK,
+ /* no translatable parameters */
metaconnection u|up\n
metaconnection d|down\n
metaconnection ?,
@@ -213,8 +232,8 @@
been started.)
},
{away, ALLOW_INFO,
- N_(away\n
- away),
+ /* no translatable parameters */
+ away,
N_(Set yourself in away mode. The AI will watch your back.),
N_(The AI will govern your nation but do minimal changes.),
},
@@ -302,7 +321,9 @@
Note that this command now takes connection names, not player names.
)
},
- {first, ALLOW_INFO, first,
+ {first, ALLOW_INFO,
+ /* no translatable parameters */
+ first,
N_(If there is none, become the game organizer with increased
permissions.),
NULL,
},
@@ -315,13 +336,13 @@
concert with the option \timeout\. Defaults are 0 0 0 1)
},
{endgame, ALLOW_HACK,
- /* TRANS: translate text between only */
- N_(endgame),
+ /* no translatable parameters */
+ endgame,
N_(End the game immediately in a draw.), NULL,
},
{surrender,ALLOW_INFO,
- /* TRANS: translate text between only */
- N_(surrender),
+ /* no translatable parameters */
+ surrender,
N_(Concede the game.),
N_(This tells everyone else that you concede the game, and if all
but one player (or one team) have conceded the game in this way
@@ -365,12 +386,71 @@
N_(Write current settings as server commands to file.), NULL
},
{rfcstyle, ALLOW_HACK,
+ /* no translatable parameters */
rfcstyle,
N_(Switch server output between 'RFC-style' and normal
Re: [Freeciv-Dev] (PR#40059) BUG: i18n: Publishing year on historian reports
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40059
Based on discussion on the -i18n list, it seems that removing the
possessive names is highly desirable. And, that swapping the fields is
common. Also, there's a list of sentence fragments immediately following
the header. Changed to allow translators more flexibility.
Committed S2_2 revision 14346.
Committed trunk revision 14347.
Index: server/report.c
===
--- server/report.c (revision 14346)
+++ server/report.c (working copy)
@@ -53,22 +53,35 @@
#define HISTORIAN_LAST HISTORIAN_LARGEST
static const char *historian_message[]={
-N_(%s report on the RICHEST Civilizations in the World %s.),
-N_(%s report on the most ADVANCED Civilizations in the World %s.),
-N_(%s report on the most MILITARIZED Civilizations in the World %s.),
-N_(%s report on the HAPPIEST Civilizations in the World %s.),
-N_(%s report on the LARGEST Civilizations in the World %s.)
+/* TRANS: year name reports ... */
+N_(%s %s reports on the RICHEST Civilizations in the World.),
+/* TRANS: year name reports ... */
+N_(%s %s reports on the most ADVANCED Civilizations in the World.),
+/* TRANS: year name reports ... */
+N_(%s %s reports on the most MILITARIZED Civilizations in the World.),
+/* TRANS: year name reports ... */
+N_(%s %s reports on the HAPPIEST Civilizations in the World.),
+/* TRANS: year name reports ... */
+N_(%s %s reports on the LARGEST Civilizations in the World.)
};
static const char *historian_name[]={
-N_(Herodotus'),
-N_(Thucydides'),
-N_(Pliny the Elder's),
-N_(Livy's),
-N_(Toynbee's),
-N_(Gibbon's),
-N_(Ssu-ma Ch'ien's),
-N_(Pan Ku's)
+/* TRANS: [year] name [reports ...] */
+N_(Herodotus),
+/* TRANS: [year] name [reports ...] */
+N_(Thucydides),
+/* TRANS: [year] name [reports ...] */
+N_(Pliny the Elder),
+/* TRANS: [year] name [reports ...] */
+N_(Livy),
+/* TRANS: [year] name [reports ...] */
+N_(Toynbee),
+/* TRANS: [year] name [reports ...] */
+N_(Gibbon),
+/* TRANS: [year] name [reports ...] */
+N_(Ssu-ma Ch'ien),
+/* TRANS: [year] name [reports ...] */
+N_(Pan Ku)
};
static const char scorelog_magic[] = #FREECIV SCORELOG2 ;
@@ -145,13 +158,25 @@
((const struct player_score_entry *)a)-value);
}
-static const char *greatness[MAX_NUM_PLAYERS] = {
- N_(Magnificent), N_(Glorious), N_(Great), N_(Decent),
- N_(Mediocre), N_(Hilarious), N_(Worthless), N_(Pathetic),
- N_(Useless), Useless, Useless, Useless, Useless, Useless,
- Useless, Useless, Useless, Useless, Useless, Useless,
- Useless, Useless, Useless, Useless, Useless, Useless,
- Useless, Useless, Useless, Useless
+static const char *greatness[] = {
+ /* TRANS: 1: The ranking Poles */
+ N_(%2d: The Magnificent %s),
+ /* TRANS: 2: The ranking Poles */
+ N_(%2d: The Great %s),
+ /* TRANS: 3: The ranking Poles */
+ N_(%2d: The Glorious %s),
+ /* TRANS: 4: The ranking Poles */
+ N_(%2d: The Excellent %s),
+ /* TRANS: 5: The ranking Poles */
+ N_(%2d: The Average %s),
+ /* TRANS: 6: The ranking Poles */
+ N_(%2d: The Mediocre %s),
+ /* TRANS: 7: The ranking Poles */
+ N_(%2d: The Pathetic %s),
+ /* TRANS: 8: The ranking Poles */
+ N_(%2d: The Useless %s),
+ /* TRANS: 9: The ranking Poles */
+ N_(%2d: The Worthless %s),
};
/**
@@ -195,17 +220,23 @@
buffer[0] = '\0';
for (i = 0; i j; i++) {
if (i == 0 || size[i].value size[i - 1].value) {
- rank = i;
+ if (i = sizeof(greatness)) {
+rank = sizeof(greatness) - 1;
+ } else if (j = sizeof(greatness)) {
+rank = i;
+ } else {
+rank = (i * sizeof(greatness)) / j;
+ }
}
cat_snprintf(buffer, sizeof(buffer),
-_(%2d: The %s %s\n),
+_(greatness[rank]),
rank + 1,
-_(greatness[rank]),
nation_plural_for_player(size[i].player));
+mystrlcat(buffer, \n, sizeof(buffer));
}
my_snprintf(title, sizeof(title), _(historian_message[which_news]),
- _(historian_name[myrand(ARRAY_SIZE(historian_name))]),
- textyear(game.info.year));
+ textyear(game.info.year),
+ _(historian_name[myrand(ARRAY_SIZE(historian_name))]));
page_conn_etype(game.est_connections, _(Historian Publishes!),
title, buffer, E_BROADCAST_REPORT);
}
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39620) [Bug] gtk2: Cannot sell buildings
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39620 Marko, the requisite time has passed -- are you committing this, or am I? ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40057) strerror-mystrerror
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40057 Jason, the requisite time has passed -- are you committing this, or am I? ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39620) [Bug] gtk2: Cannot sell buildings
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39620
Marko Lindqvist wrote:
Includes some generic cleanup-style changes I made while debugging
problem. Actual fix is citydlg.c:1621. It was storing improvement
pointer instead of number.
Original report referring to TRUNK only is older than S2_2 branch.
Thanks, good catch! The generic interface somehow didn't cause a compile
error. Tough to find. Your original report didn't include a savegame,
and I'm glad you finally found the problem.
However, I strongly disagree with your solution. I've been trying to
remove all cid_* calls, and concomitant GINT_TO_POINTER() scattered all
over the code. We've had an awful lot of reports about this behaving
badly across compiler versions!
The value passed is supposed to be a pointer to the improvement, and the
mistake is the (ancient code) treating it as an int (now Impr_type_id).
Here's my (lightly tested) alternative:
Index: client/gui-gtk-2.0/citydlg.c
===
--- client/gui-gtk-2.0/citydlg.c(revision 14338)
+++ client/gui-gtk-2.0/citydlg.c(working copy)
@@ -253,7 +253,7 @@
static void buy_callback(GtkWidget * w, gpointer data);
static void change_production_callback(GtkWidget* w, struct city_dialog*);
-static void sell_callback(Impr_type_id id, gpointer data);
+static void sell_callback(struct impr_type *pimprove, gpointer data);
static void sell_callback_response(GtkWidget *w, gint response, gpointer data);
static void impr_callback(GtkTreeView *view, GtkTreePath *path,
@@ -1618,7 +1618,7 @@
gtk_list_store_append(store, it);
gtk_list_store_set(store, it,
- 0, target.value,
+ 0, target.value.building,
1, sprite_get_pixbuf(sprite),
2, items[item].descr,
3, upkeep,
@@ -2484,11 +2484,11 @@
/
...
*/
-static void sell_callback(Impr_type_id id, gpointer data)
+static void sell_callback(struct impr_type *pimprove, gpointer data)
{
- struct impr_type *pimprove = improvement_by_number(id);
- struct city_dialog *pdialog = (struct city_dialog *) data;
GtkWidget *shl;
+ struct city_dialog *pdialog = (struct city_dialog *) data;
+ pdialog-sell_id = improvement_number(pimprove);
if (!can_client_issue_orders()) {
return;
@@ -2502,8 +2502,6 @@
return;
}
- pdialog-sell_id = id;
-
shl = gtk_message_dialog_new(NULL,
GTK_DIALOG_DESTROY_WITH_PARENT,
GTK_MESSAGE_QUESTION,
@@ -2547,7 +2545,7 @@
GtkTreeModel *model;
GtkTreeIter it;
GdkModifierType mask;
- int id;
+ struct impr_type *pimprove;
model = gtk_tree_view_get_model(view);
@@ -2555,13 +2553,12 @@
return;
}
- gtk_tree_model_get(model, it, 0, id, -1);
+ gtk_tree_model_get(model, it, 0, pimprove, -1);
gdk_window_get_pointer(NULL, NULL, NULL, mask);
if (!(mask GDK_CONTROL_MASK)) {
-sell_callback(id, data);
+sell_callback(pimprove, data);
} else {
-struct impr_type *pimprove = improvement_by_number(id);
if (is_great_wonder(pimprove)) {
popup_help_dialog_typed(improvement_name_translation(pimprove),
HELP_WONDER);
} else {
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39620) [Bug] gtk2: Cannot sell buildings
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39620
Marko Lindqvist wrote:
That's why I don't like GTK's generic APIs. You don't have type checking with
them.
Yeah, and I'm not a gui expert enough to catch them. Here's a slightly better
version of the patch, telling the interface a pointer is passed. Presumably,
the code actually worked because ints and pointers are the same size.
Index: client/gui-gtk-2.0/citydlg.c
===
--- client/gui-gtk-2.0/citydlg.c(revision 14338)
+++ client/gui-gtk-2.0/citydlg.c(working copy)
@@ -253,7 +253,7 @@
static void buy_callback(GtkWidget * w, gpointer data);
static void change_production_callback(GtkWidget* w, struct city_dialog*);
-static void sell_callback(Impr_type_id id, gpointer data);
+static void sell_callback(struct impr_type *pimprove, gpointer data);
static void sell_callback_response(GtkWidget *w, gint response, gpointer data);
static void impr_callback(GtkTreeView *view, GtkTreePath *path,
@@ -759,7 +759,7 @@
gtk_box_pack_start(GTK_BOX(top), vbox, TRUE, TRUE, 0);
/* improvements */
- store = gtk_list_store_new(4, G_TYPE_INT, GDK_TYPE_PIXBUF, G_TYPE_STRING,
+ store = gtk_list_store_new(4, G_TYPE_POINTER, GDK_TYPE_PIXBUF, G_TYPE_STRING,
G_TYPE_INT);
view = gtk_tree_view_new_with_model(GTK_TREE_MODEL(store));
@@ -1618,7 +1618,7 @@
gtk_list_store_append(store, it);
gtk_list_store_set(store, it,
- 0, target.value,
+ 0, target.value.building,
1, sprite_get_pixbuf(sprite),
2, items[item].descr,
3, upkeep,
@@ -2484,11 +2484,11 @@
/
...
*/
-static void sell_callback(Impr_type_id id, gpointer data)
+static void sell_callback(struct impr_type *pimprove, gpointer data)
{
- struct impr_type *pimprove = improvement_by_number(id);
- struct city_dialog *pdialog = (struct city_dialog *) data;
GtkWidget *shl;
+ struct city_dialog *pdialog = (struct city_dialog *) data;
+ pdialog-sell_id = improvement_number(pimprove);
if (!can_client_issue_orders()) {
return;
@@ -2502,8 +2502,6 @@
return;
}
- pdialog-sell_id = id;
-
shl = gtk_message_dialog_new(NULL,
GTK_DIALOG_DESTROY_WITH_PARENT,
GTK_MESSAGE_QUESTION,
@@ -2547,7 +2545,7 @@
GtkTreeModel *model;
GtkTreeIter it;
GdkModifierType mask;
- int id;
+ struct impr_type *pimprove;
model = gtk_tree_view_get_model(view);
@@ -2555,13 +2553,12 @@
return;
}
- gtk_tree_model_get(model, it, 0, id, -1);
+ gtk_tree_model_get(model, it, 0, pimprove, -1);
gdk_window_get_pointer(NULL, NULL, NULL, mask);
if (!(mask GDK_CONTROL_MASK)) {
-sell_callback(id, data);
+sell_callback(pimprove, data);
} else {
-struct impr_type *pimprove = improvement_by_number(id);
if (is_great_wonder(pimprove)) {
popup_help_dialog_typed(improvement_name_translation(pimprove),
HELP_WONDER);
} else {
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40028) gtk/pango invalid UTF-8 warning, fix documentation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 Madeline Book wrote: Ha! So that is how it posts duplicate messages. After I submit my reply if I refresh the resulting page it posts the message again. Bleh RT tries hard to be disliked... Firefox tells me that I cannot refresh with post data. I always click on the #ticket number to get back again Anyway, I've posted a simpler patch on the new ticket, it should show up in your mailbox soon. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40059) BUG: i18n: Publishing year on historian reports
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40059
PR#34192 added a trailing year. Not only does that make no sense as English
grammar, the patch didn't update all (any) of the translations, and thus
creates a msgstr error report.
The existing reports use a possessive for the names of the historians. They
are listed to be translated, so a really good translator can probably handle
the pairing of the messages nearby the historian names. But a TRANS comment
would certainly be useful.
Absent any discussion, here's my proposed patch:
Index: server/report.c
===
--- server/report.c (revision 14336)
+++ server/report.c (working copy)
@@ -53,21 +53,34 @@
#define HISTORIAN_LAST HISTORIAN_LARGEST
static const char *historian_message[]={
-N_(%s report on the RICHEST Civilizations in the World %s.),
-N_(%s report on the most ADVANCED Civilizations in the World %s.),
-N_(%s report on the most MILITARIZED Civilizations in the World %s.),
-N_(%s report on the HAPPIEST Civilizations in the World %s.),
-N_(%s report on the LARGEST Civilizations in the World %s.)
+/* TRANS: possessive year report ... */
+N_(%s %s report on the RICHEST Civilizations in the World.),
+/* TRANS: possessive year report ... */
+N_(%s %s report on the most ADVANCED Civilizations in the World.),
+/* TRANS: possessive year report ... */
+N_(%s %s report on the most MILITARIZED Civilizations in the World.),
+/* TRANS: possessive year report ... */
+N_(%s %s report on the HAPPIEST Civilizations in the World.),
+/* TRANS: possessive year report ... */
+N_(%s %s report on the LARGEST Civilizations in the World.)
};
static const char *historian_name[]={
+/* TRANS: possessive [year report ...] */
N_(Herodotus'),
+/* TRANS: possessive [year report ...] */
N_(Thucydides'),
+/* TRANS: possessive [year report ...] */
N_(Pliny the Elder's),
+/* TRANS: possessive [year report ...] */
N_(Livy's),
+/* TRANS: possessive [year report ...] */
N_(Toynbee's),
+/* TRANS: possessive [year report ...] */
N_(Gibbon's),
+/* TRANS: possessive [year report ...] */
N_(Ssu-ma Ch'ien's),
+/* TRANS: possessive [year report ...] */
N_(Pan Ku's)
};
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40028) gtk/pango invalid UTF-8 warning, fix documentation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 I put the workaround into the doc/README -- and just (re-)discovered that's the *only* method described at: http://freeciv.wikia.com/wiki/Install-MacOSX#Localization_Support There's a different procedure at: http://freeciv.wikia.com/wiki/Install-Windows#Other_Languages There's nothing about running with LANG at: http://freeciv.wikia.com/wiki/Install Somebody needs to update these ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40020) Segfault in server aiunit.c ai_manage_units = plrhand.c maybe_make_contact()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40020 Jason Short wrote: make_contact kills the unit because of the broken treaty and bouncing (strange in itself but whatever). Then since maybe_make_contact doesn't use a proper iterator it breaks the loop. Attached patch should fix it for 2.1 and most likely 2.2/trunk. That looks like it! Of course, it *was* a proper iterator right up until somebody added code to wipe an unrelated unit of another player that happens to be in a stack with a player that canceled a treaty In this case, the Indian player's last settler. Causing the Indian player to die! Good thing this is probably a rare event. So, let's consider this a temporary patch until bounce_unit() -- or its caller -- is fixed. What a terrible fragile logic mess! ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40054) Percentages in helpdata.c
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40054 Joan Creus wrote: I've noticed that, in the last remodelation of helpdata.c, for trunk and 2.2, three strings which contained 50%% have been changed to 50%. But two of them are still flagged as c-format, That's very odd. The other message doesn't have any problem: #: client/helpdata.c:1045 msgid * Must end turn in a city or next to land, or has a 50% risk of being lost at sea.\n The xgettext heuristic is guessing wrong for two cases. ... and msgfmt doesn't like them (The format specifications of argument 1 in msgid and msgstr are not the same, and msgstr is not a valid conversion specifier). I don't get this warning, but it sounds like a good idea! There are quite a few of these problems in *all* the po files. (PR#39970) The problematic strings are: * May be disbanded in a city to recover 50% of the production cost.\n * May fortify, granting a 50% defensive bonus.\n How do you remove the c-format? Some in data/helpdata.txt and various rulesets have: #, no-c-format They have: /* xgettext:no-c-format */ I'll add it immediately! Details at: http://www.gnu.org/software/gettext/manual/html_node/c_002dformat-Flag.html#c_002dformat-Flag ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40032) server/plrhand.c civil war message plural
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40032 Committed as part of trunk revision 14337. Committed as part of S2_2 revision 14338. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40054) Percentages in client/helpdata.c
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40054 Committed as part of trunk revision 14337. Committed as part of S2_2 revision 14338. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40032) server/plrhand.c civil war message plural
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40032
Probably should attach the patch as applied to trunk for posterity!
(Also in 2.2, an update-po and vigorous line-by-line po massaging.)
Index: server/citytools.c
===
--- server/citytools.c (revision 14336)
+++ server/citytools.c (working copy)
@@ -730,7 +730,6 @@
Create a palace in a random city. Used when the capital was conquered.
**/
static void build_free_small_wonders(struct player *pplayer,
-const char *const old_capital_name,
bv_imprs *had_small_wonders)
{
int size = city_list_size(pplayer-cities);
@@ -742,12 +741,11 @@
improvement_iterate(pimprove) {
if (BV_ISSET(*had_small_wonders, improvement_index(pimprove))) {
- struct city *pnew_city;
+ /* FIXME: instead, find central city */
+ struct city *pnew_city = city_list_get(pplayer-cities, myrand(size));
assert(find_city_from_small_wonder(pplayer, pimprove) == NULL);
- pnew_city = city_list_get(pplayer-cities, myrand(size));
-
city_add_improvement(pnew_city, pimprove);
pplayer-small_wonders[improvement_index(pimprove)] = pnew_city-id;
@@ -757,9 +755,10 @@
*/
send_player_cities(pplayer);
- notify_player(pplayer, pnew_city-tile, E_CITY_LOST,
- _(You lost %s. A new %s was built in %s.),
- old_capital_name,
+ notify_player(pplayer, pnew_city-tile, E_IMP_BUILD,
+ /* FIXME: should already be notified about city loss? */
+ /* TRANS: building ... city */
+ _(A replacement %s was built in %s.),
improvement_name_translation(pimprove),
city_name(pnew_city));
/*
@@ -938,7 +937,7 @@
/* Build a new palace for free if the player lost her capital and
savepalace is on. */
if (game.info.savepalace) {
-build_free_small_wonders(pgiver, city_name(pcity), had_small_wonders);
+build_free_small_wonders(pgiver, had_small_wonders);
}
/* Remove the sight points from the giver...and refresh the city's
@@ -1096,7 +1095,6 @@
struct player *pplayer = city_owner(pcity);
struct tile *ptile = pcity-tile;
bv_imprs had_small_wonders;
- char *cityname = mystrdup(city_name(pcity));
struct vision *old_vision;
int id = pcity-id; /* We need this even after memory has been freed */
@@ -1214,11 +1212,9 @@
/* Build a new palace for free if the player lost her capital and
savepalace is on. */
if (game.info.savepalace) {
-build_free_small_wonders(pplayer, cityname, had_small_wonders);
+build_free_small_wonders(pplayer, had_small_wonders);
}
- free(cityname);
-
sync_cities();
}
Index: server/plrhand.c
===
--- server/plrhand.c(revision 14336)
+++ server/plrhand.c(working copy)
@@ -1666,8 +1666,9 @@
if (player_count() = MAX_NUM_PLAYERS) {
/* No space to make additional player */
-freelog(LOG_NORMAL, _(Could not throw %s into civil war - too many
-players), player_name(pplayer));
+freelog(LOG_NORMAL,
+_(Could not throw %s into civil war - too many players),
+nation_plural_for_player(pplayer));
return;
}
@@ -1682,14 +1683,18 @@
/* Now split the empire */
freelog(LOG_VERBOSE,
- %s nation is thrust into civil war, created AI player %s,
+ %s civil war; created AI %s,
nation_rule_name(nation_of_player(pplayer)),
- player_name(cplayer));
+ nation_rule_name(nation_of_player(cplayer)));
notify_player(pplayer, NULL, E_CIVIL_WAR,
- _(Your nation is thrust into civil war,
- %s is declared the leader of the rebel states.),
- player_name(cplayer));
+_(Your nation is thrust into civil war.));
+ notify_player(pplayer, NULL, E_FIRST_CONTACT,
+/* TRANS: leader ... the Poles. */
+_(%s is the rebellious leader of the %s.),
+player_name(cplayer),
+nation_plural_for_player(cplayer));
+
i = city_list_size(pplayer-cities)/2; /* number to flip */
j = city_list_size(pplayer-cities); /* number left to process */
city_list_iterate(pplayer-cities, pcity) {
@@ -1703,35 +1708,35 @@
resolved stack conflicts for each city we would teleport the first
of the units we met since the other would have another owner */
transfer_city(cplayer, pcity, -1, FALSE, FALSE, FALSE);
- freelog(LOG_VERBOSE, %s declares allegiance to %s,
+ freelog(LOG_VERBOSE, %s declares allegiance to the %s.,
city_name(pcity),
- player_name(cplayer));
+
Re: [Freeciv-Dev] (PR#40028) gtk/pango invalid UTF-8 warning, fix documentation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 Jason Short wrote: Here is a quick and partial fix. I assume that strerror() is one of the most common offending functions, so I quickly went through and converted all mystrerror users in client/ and server/ directories to use the newly-written L_(). Now, I find this irritating on a number of levels. 1) This ticket was already patched and resolved. Another new ticket would be better. I'll open one. 2) Madeline reported the problem was in mystrerror(), so that's a good target, but the solution would be to patch mystrerror(), not repeatedly patch every invocation This is *all* users! 3) By definition, mystrerror() can only be called once, so there's no technical reason to use a dynamic buffer. 4) I really hate L_() hiding a simple single function. There's no excuse. This makes the code harder to read and harder to grep. 5) The parameter order doesn't match either cat_snprintf() or strlcat(), or any standard C invocation. By convention, the destination buffer is always the first parameter. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40058) mystrerror() and local encoding
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40058 Followup of PR#40028. See earlier patch and objections there. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40028) gtk/pango invalid UTF-8 warning, fix documentation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 Jason Dorje Short wrote: Good, except, all rulesets must be (and to my knowledge are) in utf-8. Perhaps, but that's not what the data currently said, and I'd prefer to document reality rather than hope. Maybe there should be a concerted effort to scan every data file and ensure they are all UTF-8? ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40028) gtk/pango invalid utf8 warning for LANG=fr_FR
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 Jason Short wrote: Madeline Book wrote: Oh wow, it really was trivial! But certainly not obvious for the likes of me :(. Once I ran the client with LANG= fr_FR.UTF-8 the libc messages displayed correctly and gtk/ pango was very happy. Nor to me. Leaving open until this is documented on our web pages and in the distributed files. This is a bug in freeciv. Background: when I wrote the charset code, I divided freeciv into 3 charsets. ... And where the heck is this documented? So in summary, if any bug reporters can track where the offending strings are coming from, fixing each incident is not too hard. In the meantime this should be listed as a known bug. I'm really tired of this laissez fair attitude toward bug fixing. It may be a known bug, but that means there should be an open ticket, probably with a tracking ticket to gather the related incidents. It is a serious bug. And the .UTF-8 workaround needs to be clearly documented! Everywhere! ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40048) i18n: XAW wordwrap messages
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40048 Followup to PR#40043. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40049) Losing message is not shown
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40049 Joan Creus wrote: I have checked both trunk and 2.1, and both seem to have this behaviour. I have even tried an old saved game (from 2.0) with the same results. Is it on purpose? Certainly not! Sounds like a long term bug. Send the savegame? server/unithand.c:1022 notify_player(unit_owner(plooser), def_tile, E_UNIT_LOST_ATT, /* TRANS: ... Cannon ... the Polish Destroyer. */ _(Your attacking %s failed against the %s %s!), unit_name_translation(plooser), nation_adjective_for_player(unit_owner(pwinner)), unit_name_translation(pwinner)); wipe_unit(plooser); NB: The opposite of looser isn't winner, it's tighter. :-) ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40032) server/plrhand.c civil war message plural
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40032 Christian Knoke wrote: Egor Vyscrebentsov wrote on Jan 25, 05:37 (-0800): On Fri, 25 Jan 2008 William Allen Simpson wrote: ru_RU specific: Gender problem can be avoided by usage nation_plural at the first place. After all the recent work to add adjectives, you want to eliminate them? Just take into account that you can't get right translation with adjs for several language using gettext... I can confirm that this is also a prob in german with the latest changes. I'm not sure how to adress it. I've started a discussion over on -i18n, and hopefully we'll get more feedback. There are currently: 119 occurrences of nation_adjective_* in 45 files. 91 occurrences of nation_plural_* in 21 files. So, it would be a *lot* of work to make the changes from adjective to plural, and I'm not sure that it would even be possible City names aren't translated, so I'm at a loss as to how we'd standardize. In this report, let's concentrate on this set of messages, and see what we can learn to extrapolate to all messages. This is the best time (preparing a new beta) to address the whole issue. Let's discuss with the rest of the -i18n community. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#10400) untranslated LOG_NORMAL messages
URL: http://bugs.freeciv.org/Ticket/Display.html?id=10400
Went through *all* the 2.2 sources. Found quite a few that are debugging
messages, not normal user messages. New symbols LOG_TEST (same level as
LOG_NORMAL) and LOG_PACKET (same level as LOG_VERBOSE).
Actually results in very few new messages to translate:
Index: utility/inputfile.c
===
--- utility/inputfile.c (revision 14283)
+++ utility/inputfile.c (working copy)
@@ -592,7 +592,7 @@
func = tok_tab[type].func;
if (!func) {
-freelog(LOG_NORMAL, token type %d (%s) not supported yet, type, name);
+freelog(LOG_ERROR, token type %d (%s) not supported yet, type, name);
c = NULL;
} else {
if (!have_line(inf))
Index: utility/ioz.c
===
--- utility/ioz.c (revision 14283)
+++ utility/ioz.c (working copy)
@@ -107,13 +107,13 @@
#ifndef HAVE_LIBZ
if (method == FZ_ZLIB) {
- freelog(LOG_NORMAL, Not compiled with zlib support, reverting to
plain.);
+ freelog(LOG_ERROR, Not compiled with zlib support, reverting to
plain.);
method = FZ_PLAIN;
}
#endif
#ifndef HAVE_LIBBZ2
if (method == FZ_BZIP2) {
- freelog(LOG_NORMAL, Not compiled with bzib2 support, reverting to
plain.);
+ freelog(LOG_ERROR, Not compiled with bzib2 support, reverting to
plain.);
method = FZ_PLAIN;
}
#endif
Index: utility/ftwl/widget_window.c
===
--- utility/ftwl/widget_window.c(revision 14283)
+++ utility/ftwl/widget_window.c(working copy)
@@ -389,14 +389,12 @@
be_copy_osda_to_screen(whole_osda, NULL);
if (dump_screen) {
-static int counter = -1;
+static int counter = 0;
char b[100];
-counter++;
+my_snprintf(b, sizeof(b), fc_%05d.ppm, counter++);
+freelog(LOG_NORMAL, _(Making screenshot %s), b);
-freelog(LOG_NORMAL, flush to screen %d,counter);
-
-sprintf(b, screen-%04d.ppm, counter);
be_write_osda_to_file(whole_osda, b);
}
}
Index: utility/ftwl/text_renderer.c
===
--- utility/ftwl/text_renderer.c(revision 14283)
+++ utility/ftwl/text_renderer.c(working copy)
@@ -190,7 +190,7 @@
ft_kerning_default, delta);
pen.x += delta.x 6;
if (0 delta.x)
- freelog(LOG_NORMAL, kerning between %c and %c is %ld in '%s'\n,
+ freelog(LOG_VERBOSE, kerning between %c and %c is %ld in '%s'\n,
text[i - 1], c, delta.x 6, text);
}
Index: utility/ftwl/be_sdl_pixels_32.c
===
--- utility/ftwl/be_sdl_pixels_32.c (revision 14283)
+++ utility/ftwl/be_sdl_pixels_32.c (working copy)
@@ -67,12 +67,12 @@
}
atexit(SDL_Quit);
- freelog(LOG_NORMAL, Using Video Output: %s,
+ freelog(LOG_NORMAL, _(Using Video Output: %s),
SDL_VideoDriverName(device, sizeof(device)));
{
const SDL_VideoInfo *info = SDL_GetVideoInfo();
-freelog(LOG_NORMAL, Video memory of driver: %dkb, info-video_mem);
-freelog(LOG_NORMAL, Preferred depth: %d bits per pixel,
+freelog(LOG_VERBOSE, Video memory of driver: %dkb, info-video_mem);
+freelog(LOG_VERBOSE, Preferred depth: %d bits per pixel,
info-vfmt-BitsPerPixel);
}
@@ -104,12 +104,12 @@
exit(1);
}
- freelog(LOG_NORMAL, Got a screen with size (%dx%d) and %d bits per pixel,
+ freelog(LOG_VERBOSE, Got a screen with size (%dx%d) and %d bits per pixel,
screen-w, screen-h, screen-format-BitsPerPixel);
- freelog(LOG_NORMAL, format: red=0x%x green=0x%x blue=0x%x mask=0x%x,
+ freelog(LOG_VERBOSE, format: red=0x%x green=0x%x blue=0x%x mask=0x%x,
screen-format-Rmask, screen-format-Gmask,
screen-format-Bmask, screen-format-Amask);
- freelog(LOG_NORMAL, format: bits-per-pixel=%d bytes-per-pixel=%d,
+ freelog(LOG_VERBOSE, format: bits-per-pixel=%d bytes-per-pixel=%d,
screen-format-BitsPerPixel, screen-format-BytesPerPixel);
SDL_EnableUNICODE(1);
SDL_EnableKeyRepeat(SDL_DEFAULT_REPEAT_DELAY, SDL_DEFAULT_REPEAT_INTERVAL);
Index: utility/ftwl/be_sdl.c
===
--- utility/ftwl/be_sdl.c (revision 14283)
+++ utility/ftwl/be_sdl.c (working copy)
@@ -119,7 +119,7 @@
Uint16 unicode = sdl_event-key.keysym.unicode;
if (unicode == 0) {
- freelog(LOG_NORMAL, unicode == 0);
+ freelog(LOG_TEST, unicode == 0);
return FALSE;
}
if ((unicode 0xFF80) != 0) {
@@ -140,7 +140,7 @@
exit(EXIT_SUCCESS);
default:
-// freelog(LOG_NORMAL, ignored event %d\n, sdl_event-type);
+// freelog(LOG_TEST, ignored event %d\n, sdl_event-type);
return FALSE;
}
Re: [Freeciv-Dev] (PR#40043) i18n: wordwrap_string() broken for UTF-8 translations
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40043 Egor Vyscrebentsov wrote: Maybe I can't say what I want. Trying again: strlen couldn't be used for receiving number of alphabetic characters in the string. It can be used for receiving number of 7-bit signed integer. And my point is that usage strlen to get number of alphabetic characters IS and WILL BE an issue with UTF-8. Let's say we are in violent agreement. This is not and never has been correct usage of strlen(). The current code is wrong. The purpose of wordwrap_string function is to make human-readable break in string line, isn't it? (I do not say here if it right or not to make such breaks.) And I'm saying that it is wrong (not right). I'm comfortable making value judgments based on empirical evidence. And, as currently implemented, is impossible for languages that don't use blank spaces as dividers between words. It is the responsibility of the developers to make the human-readable line breaks in the canonical form message. It is the responsibility of the translators to make the human-readable line breaks in other places based on their language. As to whether there are other places with similar bad code, that may be true, but it doesn't mitigate the fact that this needs to be fixed. We'll fix the others as they are discovered. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40043) i18n: wordwrap_string() broken for UTF-8 translations
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40043
This removes wordwrap_string() in command line handling, adding a TRANS
comment tell translators the width.
It was amusing to discover that civmanual used it for html. That's silly.
By definition, html always handles localized wrapping itself!
Still to do, specific clients
Index: server/stdinhand.c
===
--- server/stdinhand.c (revision 14283)
+++ server/stdinhand.c (working copy)
@@ -29,7 +29,6 @@
#endif
#endif
-#include astring.h
#include fciconv.h
#include fcintl.h
#include log.h
@@ -1518,22 +1517,23 @@
if (op-short_help) {
cmd_reply(help_cmd, caller, C_COMMENT,
- %s %s - %s, _(Option:), op-name, _(op-short_help));
+ /* TRANS: untranslated name - translated short help */
+ Option: %s - %s,
+ op-name,
+ _(op-short_help));
} else {
cmd_reply(help_cmd, caller, C_COMMENT,
- %s %s, _(Option:), op-name);
+ /* TRANS: untranslated name */
+ Option: %s,
+ op-name);
}
if(op-extra_help strcmp(op-extra_help,)!=0) {
-static struct astring abuf = ASTRING_INIT;
const char *help = _(op-extra_help);
-astr_minsize(abuf, strlen(help)+1);
-strcpy(abuf.str, help);
-wordwrap_string(abuf.str, 76);
cmd_reply(help_cmd, caller, C_COMMENT, _(Description:));
cmd_reply_prefix(help_cmd, caller, C_COMMENT,
- , %s, abuf.str);
+ , %s, help);
}
cmd_reply(help_cmd, caller, C_COMMENT,
_(Status: %s), (setting_is_changeable(id)
@@ -3832,6 +3832,7 @@
{
/* This is formated like extra_help entries for settings and commands: */
const char *help =
+/* TRANS: line break width 68 */
_(Welcome - this is the introductory help text for the Freeciv
server.\n\n
Two important server concepts are Commands and Options.\n
Commands, such as 'help', are used to interact with the server.\n
@@ -3847,12 +3848,7 @@
save - to save the current game\n
quit - to exit);
- static struct astring abuf = ASTRING_INIT;
-
- astr_minsize(abuf, strlen(help)+1);
- strcpy(abuf.str, help);
- wordwrap_string(abuf.str, 78);
- cmd_reply(help_cmd, caller, C_COMMENT, abuf.str);
+ cmd_reply(help_cmd, caller, C_COMMENT, help);
}
/**
@@ -3867,10 +3863,15 @@
if (cmd-short_help) {
cmd_reply(help_cmd, caller, C_COMMENT,
- %s %s - %s, _(Command:), cmd-name, _(cmd-short_help));
+ /* TRANS: untranslated name - translated short help */
+ Command: %s - %s,
+ cmd-name,
+ _(cmd-short_help));
} else {
cmd_reply(help_cmd, caller, C_COMMENT,
- %s %s, _(Command:), cmd-name);
+ /* TRANS: untranslated name */
+ Command: %s,
+ cmd-name);
}
if (cmd-synopsis) {
/* line up the synopsis lines: */
@@ -3885,15 +3886,11 @@
cmd_reply(help_cmd, caller, C_COMMENT,
_(Level: %s), cmdlevel_name(cmd-level));
if (cmd-extra_help) {
-static struct astring abuf = ASTRING_INIT;
const char *help = _(cmd-extra_help);
-astr_minsize(abuf, strlen(help)+1);
-strcpy(abuf.str, help);
-wordwrap_string(abuf.str, 76);
cmd_reply(help_cmd, caller, C_COMMENT, _(Description:));
cmd_reply_prefix(help_cmd, caller, C_COMMENT, ,
- %s, abuf.str);
+ %s, help);
}
}
Index: manual/civmanual.c
===
--- manual/civmanual.c (revision 14283)
+++ manual/civmanual.c (working copy)
@@ -21,7 +21,6 @@
#include stdlib.h
#include string.h
-#include astring.h
#include fciconv.h
#include fcintl.h
#include log.h
@@ -128,17 +127,13 @@
fprintf(doc, _(h1Freeciv %s server options/h1\n\n),
VERSION_STRING);
for (i = 0; settings[i].name; i++) {
struct settings_s *op = settings[i];
-static struct astring abuf = ASTRING_INIT;
const char *help = _(op-extra_help);
-astr_minsize(abuf, strlen(help) + 10);
-strcpy(abuf.str, help);
-wordwrap_string(abuf.str, 76);
fprintf(doc, SEPARATOR);
fprintf(doc, %s%s - %s%s\n\n, SECTION_BEGIN, op-name,
_(op-short_help), SECTION_END);
if (strlen(op-extra_help) 0) {
- fprintf(doc, pre%s/pre\n\n, abuf.str);
+ fprintf(doc, pre%s/pre\n\n, help);
}
fprintf(doc, p class=\misc\);
fprintf(doc, _(Level: %s.br), _(sset_level_names[op-slevel]));
@@ -202,14 +197,10 @@
fprintf(doc, _(p class=\level\Level: %s/p\n\n),
cmdlevel_name(cmd-level));
if (cmd-extra_help) {
- static struct astring abuf = ASTRING_INIT;
const
[Freeciv-Dev] (PR#40044) i18n: split data/helptext.txt keyboard/mouse orders
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40044 Reported in PR#40032. Egor Vyscrebentsov [EMAIL PROTECTED] writes: PS. I'm also for splitting into several messages a keyboard/mouse orders helptext from data/helptext.txt. It will a) make it much easier for translator to see changes; b) be a step towards user-defined control. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40043) i18n: wordwrap_string() broken for UTF-8 translations
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40043 Reported in PR#40032. Egor Vyscrebentsov [EMAIL PROTECTED] writes: (For ru_RU.UTF-8 part of lines cover only half of screen width - 2 bytes/character. Easiest example - '/help' [show_help_intro]. wordwrap_string() looks to me like the root of problem in this case.) ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40043) Re: (PR#40045) wordwrap_string(), strlen(), and multibytes encodings
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40043 Egor Vyscrebentsov wrote: At least in utility/shared.c:wordwrap_string() strlen() is used to determine number of characters in the string. This gives wrong result on multibytes encoding. The issue is not strlen(). It is properly used to determine the length of a string. The issue is that strlen() has nothing to do with how much room there is for display in a particular window! This abuse of strlen() would have been considered a stupid bug as far back as the techtronix green screen in the '70s!!! Or the DECwriters we all used for C programming back then. Let alone modern i18n The messages should be wrapped in the clients by their respective GUI code, such as gtk-text-set-word-wrap, XawtextWrapWord, etc. (easy Google of examples) I propose removing wordwrap_string() everywhere. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40041) BUG: server sending intelligence researching A_UNSET?
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40041 This is probably only fixable 2.2-only, with the new advance/techology iterators (PR#39525) able to vary the starting range, and better use of access functions to find the error. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40043) i18n: wordwrap_string() broken for UTF-8 translations
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40043 Egor Vyscrebentsov wrote: On Thu, 24 Jan 2008 William Allen Simpson wrote: The issue is not strlen(). It is properly used to determine the length of a string. The issue is that strlen() has nothing to do with how much room there is for display in a particular window! From what I can see everyday, you're wrong. strlen() will return 70, while there are only, say, 40 multibyte characters. (heavy sigh) You are confusing strings with characters. They are not the same Perhaps because of the unfortunately named char type, a 7-bit signed integer. They have *never* been the same. Over my life, I've programmed for 5-bit (Baudot code), 6-bit (CDC), 7-bit, 8-bit, 12-bit, and now multi-byte characters. For characters, we don't talk about lengths, we talk about widths, either in pixels or points. strlen() may be used to get size of string, but not length. Perhaps there must be some Russian differences in the meaning of the words size and length. They are synonymous in English: man 3 strlen strlen - find length of string size_t strlen(const char *s); Please note that I have mentioned server console. What should we use there? Nothing. No (translated or otherwise) message to the console needs to be wrapped by the program. That is handled by the console driver, as always, since the days of paper tape! It's truly sad that so much knowledge in the computer science field has been lost because so many of our students only know M$windows. :-( I propose removing wordwrap_string() everywhere. I disagree. However, I agree that we should not use this function for GUI clients. But it seems to me that you say about other problem than me. I see them as all part of the same problem. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
