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#40110) tile_info worked, enum known_type, city_can_work_tile(), and invisible cities
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40110 [wsimpson - Sun Feb 24 12:51:14 2008]: 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. Ah, thank you for that clarification. It does make much more sense that gameplay rules be controlled by rulesets rather than server settings (which has unfortunately been the standard practice for warserver up until now). 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. I would call it a ruleset omission rather than a bug. As far is I know diplomats and spies never had the Partial_Invis flag in the default ruleset, and players are highly cognizant of that fact. 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. I think it could be argued that this behaviour is a feature rather than a bug, but I grant that if the only way to fix crashes is to sacrifice it for the new behaviour, then it is alright to lose it. 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. I'm not sure if this last sentence refers to ZOC handling or the other arguments in the discussion. Since if it refers to ZOC it is a non sequitor, I will assume it is refering to the latter. ;) I would hope that civ3 emulation be controlled by rulesets (analogously to the civ1 and civ2 rulesets), and the core functionality of the civserver be flexible enough to satisfy the expectations of all users (multiplayer and singleplayer). However, none of this is relevant to this ticket, which is on the path to fixing crashing bugs I agree, but you did bundle a gameplay rule change into your bugfix, to which this discussion is relevant. Try to separate the two in the future. ___ 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 [wsimpson - Sun Feb 24 12:55:07 2008]: 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. I apologize for half-replying to this in the other ticket (PR#40110), I did not notice that you started a new ticket for it until after I had posted that reply. Anyway, for this rule change I would like to know your source (in what version of civ is that the case?), since in the default, civ1, and civ2 rulesets, diplomats and spies do not have Partial_Invis, and as far as I know they never did. Is this something in civ3? If so it would be nice to have this then in a civ3 ruleset (analogously to the civ1 and civ2 ruleset directories under data/) so that the expected behaviour (no stealth) is preserved in the default ruleset. ___ 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 [wsimpson - Mon Feb 25 01:11:20 2008]: Since Madeline's testing revealed no bugs in the code execution, I never tested the code (indeed looking back at my posts I never said so implicitly or explicitly), I merely commented on your own report of the one game rule change, and why it would not be, according to me, a good idea. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40113) civclient: player.c:293: player_number: Assertion `pplayer' failed.
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40113 After updating my working tree with latest S2_2 branch (r14422) I get an assertion failure in the client when trying to start a new game. Steps to reproduce: 1. Start a server in one console (e.g. run ./ser). 2. Start a client in another console (e.g. with ./civ -a). 3. Once the client connects press Pick Nation and select any nation from the list. 4. Now press Ok. civclient: player.c:293: player_number: Assertion `pplayer' failed. Backtrace: #0 0xe410 in __kernel_vsyscall () #1 0x40764e49 in raise () from /lib/tls/libc.so.6 #2 0x40766872 in abort () from /lib/tls/libc.so.6 #3 0x4075e718 in __assert_fail () from /lib/tls/libc.so.6 #4 0x080f5814 in player_number (pplayer=0x0) at player.c:293 #5 0x0811e031 in races_response (w=0x85aad08, response=0, data=0x0) at dialogs.c:1291 #6 0x40527035 in IA__g_cclosure_marshal_VOID__INT (closure=0x95dabd8, return_value=0x0, n_param_values=2, param_values=0xbfee32b0, invocation_hint=0xbfee3178, marshal_data=0x0) at gmarshal.c:216 #7 0x4050fd3b in IA__g_closure_invoke (closure=0x95dabd8, return_value=0x0, n_param_values=0, param_values=0x0, invocation_hint=0x0) at gclosure.c:490 ... (more gtk callback stuff all the way to ui_main) I will see if I cannot fix this (kind of hard to work on the editor if I cannot start a game ;)). ___ 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
[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 [wsimpson - Mon Feb 25 03:15:06 2008]: Madeline Book wrote: I never tested the code WTF! The whole purpose of posting patches for comments is that the patches are actively tested! 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. ___ 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
[Freeciv-Dev] (PR#40113) civclient: player.c:293: player_number: Assertion `pplayer' failed.
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40113 [book - Mon Feb 25 02:47:42 2008]: I will see if I cannot fix this (kind of hard to work on the editor if I cannot start a game ;)). So here is a quick patch that addresses the underlying cause as I understand it at the moment. From 537f83146dce55c7805aa8b0eaaee3a0cca4d977 Mon Sep 17 00:00:00 2001 From: Madeline Book [EMAIL PROTECTED] Date: Sun, 24 Feb 2008 23:47:38 -0500 Subject: [PATCH] (PR#40113) Fix assertion failure when picking nation. The civclient global variable client.playing was not being updated when the client received a packet of type PACKET_CONN_INFO (see handle_conn_info in client/packhand.c). This would result in client.playing being NULL under certain circumstances thus causing the assertion in question to fail. --- client/packhand.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/client/packhand.c b/client/packhand.c index 0eacef8..e3691d8 100644 --- a/client/packhand.c +++ b/client/packhand.c @@ -1955,6 +1955,7 @@ void handle_conn_info(struct packet_conn_info *pinfo) aconnection.observer = pconn-observer; aconnection.access_level = pconn-access_level; aconnection.player = pplayer; + client.playing = pplayer; } } update_players_dialog(); -- 1.5.3.8 ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40113) civclient: player.c:293: player_number: Assertion `pplayer' failed.
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40113 [wsimpson - Mon Feb 25 03:22:41 2008]: 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. I did say to run the client and server separately in my report (i.e. not via Start New Game). Once you can reproduce, try my patch, and possibly expand upon it (client.playing is supposed to supplant aconnection.player is it not?). ___ 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
Re: [Freeciv-Dev] (PR#40111) Diplomatic immunity
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40111 On Sun, Feb 24, 2008 at 6:46 PM, Madeline Book [EMAIL PROTECTED] wrote: This is a bug. These units should not show up until adjacent to cities, and should be completely invisible to passing units. 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. - Per ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
