Re: [Freeciv-Dev] (PR#40408) [Patch] Updating problem in trunk when capturing enemy cities

2008-08-04 Thread Nicolas R. Wadhwani

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40408 

Oh, my bad actually. I didn't notice there was a ticket already describing 
exactly the same bug as I was tackling with. ;) Your patch seems to be more 
thoroughly cleaning up some messy code while I was just looking for a 
quickdirty solution, as always. So no complaints from me, go ahaed and close 
the tickets (if you can delete mine I won't mind either ;) ), we should stick 
with your patch. I haven't tested it totally in depth, but it seems to be 
working quite fine.

Greetings,
Nico



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40408) [Patch] Updating problem in trunk when capturing enemy cities

2008-08-03 Thread Nicolas R. Wadhwani

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40408 

 [EMAIL PROTECTED] - So 27. Jul 2008, 17:10:31]:
 
 Depending on the order of packets being correct is, in general, very
 bad.  Each packet should be independent.

I agree on that so it is probably most sensible to let the apropiate
function processing the city_info packet check for the change of the
owner of the city instead of the tile. I also deleted a comment
suggesting that tile owner should be the same as city owner, which
clearly didn't seem to be the case without my previous patch.

Greetings,
Nico
diff -Nur -X.diff_ignore trunk/client/packhand.c changed/client/packhand.c
--- trunk/client/packhand.c	2008-07-27 21:46:43.0 +0200
+++ changed/client/packhand.c	2008-08-03 21:28:58.0 +0200
@@ -509,8 +509,7 @@
   pcity-original = powner;
 
   tile_set_owner(pcenter, powner);
-} else if (tile_owner(ptile) != powner) {
-  /* assumes the tile properly reflects city_owner() */
+} else if (city_owner(pcity) != powner) {
   client_remove_city(pcity);
   pcity = NULL;
   city_has_changed_owner = TRUE;
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40408) [Patch] Updating problem in trunk when capturing enemy cities

2008-08-03 Thread Madeline Book

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40408 

 [nicorwadh - Sun Aug 03 21:35:49 2008]:
 
  [EMAIL PROTECTED] - So 27. Jul 2008, 17:10:31]:
  
  Depending on the order of packets being correct is, in general, very
  bad.  Each packet should be independent.
 
 I agree on that so it is probably most sensible to let the apropiate
 function processing the city_info packet check for the change of the
 owner of the city instead of the tile. I also deleted a comment
 suggesting that tile owner should be the same as city owner, which
 clearly didn't seem to be the case without my previous patch.

Oops, I missed this ticket when I fixed this issue
for #40327. My patch there basically does the exact
same thing as yours (but fixes both occurences of
the misuse of tile_owner). Can you check that that
patch also fixes the problem in this ticket; we
may be able to close both issues.


--
誰が私のズボンを取ってしまった!

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40408) [Patch] Updating problem in trunk when capturing enemy cities

2008-07-27 Thread Jason Dorje Short

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40408 

On Sat, Jul 26, 2008 at 7:47 PM, Nicolas R. Wadhwani [EMAIL PROTECTED] wrote:

 URL: http://bugs.freeciv.org/Ticket/Display.html?id=40408 

 Hi,

 In current trunk version if you capture an enemy city it still looks like it
 would belong to the previous owner before you captured it. Only if you would
 quit the game and rejoin/reload the captured city would appear in your city
 list and become accesible for changing the worklist, worker placement etc.

Yep.

 This is because the handle_city_info function depends on noticing a change of
 owners between the current known owner of a tile and the new owner of the
 city sent by the server. However, this change of ownership is currently
 handled by handle_tile_info previously before the city info packet arrives.
 The result is that the borders and all are calculated correcty and updated
 immediately, but once the new packet for the change of the city ownership
 comes in the client assumes the owner remained the same because the tile
 already belongs to the player the server is sending as new owner.

The check simply needs to be fixed.  Whatever it is that's causing the
city to be displayed as enemy is what needs to be checked for.

 This problem could probably alternatively solved by changing the order in
 which the packets are send by the server, but this would be a bit more
 complicated and inhibits the risk of messing up something I just don't see
 now that crucialy depends on updating the tiles before the cities or I dunno
 what else.

Depending on the order of packets being correct is, in general, very
bad.  Each packet should be independent.

-jason



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev