[Freeciv-Dev] (PR#39548) [Patch] Fix sernet.c compiler warning

2007-08-16 Thread Christian Prochaska

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

 [EMAIL PROTECTED] - So 12. Aug 2007, 16:50:47]:
 
  This fixes warning when neither SOCKET_ZERO_ISNT_STDIN nor
 HAVE_READLINE is defined.
 
 
  - ML
 

There are actually two locations where the handle_stdin_close() function
might get called. At the one location HAVE_LIBREADLINE is defined and at
the other location both SOCKET_ZERO_ISNT_STDIN and HAVE_LIBREADLINE are
undefined. Updated patches for S2_0, S2_1 and trunk are attached.
Index: server/sernet.c
===
--- server/sernet.c	(revision 13338)
+++ server/sernet.c	(working copy)
@@ -130,6 +130,10 @@
 
 static bool no_input = FALSE;
 
+/* Avoid compiler warning about defined, but unused function
+ * by defining it only when needed */
+#if defined(HAVE_LIBREADLINE) || \
+(!defined(SOCKET_ZERO_ISNT_STDIN)  !defined(HAVE_LIBREADLINE))  
 /*
   This happens if you type an EOF character with nothing on the current line.
 */
@@ -141,9 +145,11 @@
 #ifndef SOCKET_ZERO_ISNT_STDIN
   freelog(LOG_NORMAL, _(Server cannot read standard input. Ignoring input.));
   no_input = TRUE;
-#endif
+#endif /* SOCKET_ZERO_ISNT_STDIN */
 }
 
+#endif /* HAVE_LIBREADLINE || (!SOCKET_ZERO_ISNT_STDIN  !HAVE_LIBREADLINE) */
+
 #ifdef HAVE_LIBREADLINE
 //
 
Index: server/sernet.c
===
--- server/sernet.c	(revision 13338)
+++ server/sernet.c	(working copy)
@@ -133,7 +133,8 @@
 
 /* Avoid compiler warning about defined, but unused function
  * by defining it only when needed */
-#if !defined(SOCKET_ZERO_ISNT_STDIN)  !defined(HAVE_READLINE)
+#if defined(HAVE_LIBREADLINE) || \
+(!defined(SOCKET_ZERO_ISNT_STDIN)  !defined(HAVE_LIBREADLINE))  
 /*
   This happens if you type an EOF character with nothing on the current line.
 */
@@ -148,7 +149,7 @@
 #endif /* SOCKET_ZERO_ISNT_STDIN */
 }
 
-#endif /* !SOCKET_ZERO_ISNT_STDIN  !HAVE_READLINE */
+#endif /* HAVE_LIBREADLINE || (!SOCKET_ZERO_ISNT_STDIN  !HAVE_LIBREADLINE) */
 
 #ifdef HAVE_LIBREADLINE
 //
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#39462) goto command for few unit crash

2007-08-16 Thread Pepeto _

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

 [wsimpson - Jeu. Aoû. 16 01:18:04 2007]:
 Are there some old bug reports with games we can test?
This is a save game you can test this, with select the engineers with Y,
on the 2 islands. Then you can test goto, patrol and connect...


civgame-3980.sav.gz
Description: GNU Zip compressed data
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#39548) [Patch] Fix sernet.c compiler warning

2007-08-16 Thread Marko Lindqvist

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

On 16/08/07, Christian Prochaska [EMAIL PROTECTED] wrote:

  [EMAIL PROTECTED] - So 12. Aug 2007, 16:50:47]:
 
   This fixes warning when neither SOCKET_ZERO_ISNT_STDIN nor
  HAVE_READLINE is defined.
 

 There are actually two locations where the handle_stdin_close() function
 might get called. At the one location HAVE_LIBREADLINE is defined and at
 the other location both SOCKET_ZERO_ISNT_STDIN and HAVE_LIBREADLINE are
 undefined. Updated patches for S2_0, S2_1 and trunk are attached.

 Can someone commit (at least S2_1) immediately? Must go in before
beta6 (and we must test that everything compiles before releasing
beta6)


 - ML



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


[Freeciv-Dev] (PR#39462) goto command for few unit crash

2007-08-16 Thread Pepeto _

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

 [wsimpson - Jeu. Aoû. 16 01:18:04 2007]:
 Are there some old bug reports with games we can test?
This is a save game you can test this, with select the engineers with Y,
on the 2 islands. Then you can test goto, patrol and connect...


civgame-3980.sav.gz
Description: GNU Zip compressed data
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#39462) goto command for few unit crash

2007-08-16 Thread Pepeto _

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

This patch should work faster (when there are a lot of unit for
example). This one can be apply on 2.1 and trunk. Enjoy...
--- client/control.c.old	2007-08-15 20:28:54.0 +0200
+++ client/control.c	2007-08-16 10:49:38.0 +0200
@@ -2035,16 +2035,13 @@
 **/
 void do_unit_goto(struct tile *ptile)
 {
-  struct tile *dest_tile;
-
   if (hover_state != HOVER_GOTO  hover_state != HOVER_NUKE) {
 return;
   }
 
   draw_line(ptile);
-  dest_tile = get_line_dest();
-  if (ptile == dest_tile) {
-send_goto_route();
+  if (is_valid_goto_destination(ptile)) {
+send_goto_route(ptile);
   } else {
 create_event(ptile, E_BAD_COMMAND,
 		 _(Didn't find a route to the destination!));
@@ -2072,15 +2069,12 @@
 **/
 void do_unit_patrol_to(struct tile *ptile)
 {
-  struct tile *dest_tile;
-
   draw_line(ptile);
-  dest_tile = get_line_dest();
-  if (ptile == dest_tile
+  if (is_valid_goto_destination(ptile)
!is_non_allied_unit_tile(ptile, game.player_ptr)) {
-send_patrol_route();
+send_patrol_route(ptile);
   } else {
-create_event(dest_tile, E_BAD_COMMAND,
+create_event(ptile, E_BAD_COMMAND,
 		 _(Didn't find a route to the destination!));
   }
 
@@ -2093,12 +2087,9 @@
 void do_unit_connect(struct tile *ptile,
 		 enum unit_activity activity)
 {
-  struct tile *dest_tile;
-
   draw_line(ptile);
-  dest_tile = get_line_dest();
-  if (same_pos(dest_tile, ptile)) {
-send_connect_route(activity);
+  if (is_valid_goto_destination(ptile)) {
+send_connect_route(ptile, activity);
   } else {
 create_event(ptile, E_BAD_COMMAND,
 		 _(Didn't find a route to the destination!));
--- client/goto.c.old	2007-08-15 20:29:08.0 +0200
+++ client/goto.c	2007-08-16 11:13:15.0 +0200
@@ -190,8 +190,9 @@
 /** 
   Change the destination of the last part to the given position if a
   path can be found. If not the destination is set to the start.
+  Return TRUE if the new path is found.
 ***/
-static void update_last_part(struct goto_map *goto_map, struct tile *ptile)
+static bool update_last_part(struct goto_map *goto_map, struct tile *ptile)
 {
   struct part *p = goto_map-parts[goto_map-num_parts - 1];
   struct pf_path *new_path;
@@ -202,12 +203,10 @@
   TILE_XY(ptile), TILE_XY(p-start_tile), TILE_XY(p-end_tile));
   new_path = pf_get_path(p-map, ptile);
 
-  is_goto_valid = (new_path != NULL);
-
   if (!new_path) {
 freelog(PATH_LOG_LEVEL,   no path found);
 reset_last_part(goto_map);
-return;
+return FALSE;
   }
 
   freelog(PATH_LOG_LEVEL,   path found:);
@@ -277,6 +276,8 @@
   /* Refresh tiles so turn information is shown. */
   refresh_tile_mapcanvas(old_tile, FALSE, FALSE);
   refresh_tile_mapcanvas(ptile, FALSE, FALSE);
+
+  return TRUE;
 }
 
 /** 
@@ -885,8 +886,11 @@
 {
   assert(is_active);
 
+  is_goto_valid = FALSE;
   goto_map_iterate(goto_maps, goto_map, punit) {
-update_last_part(goto_map, dest_tile);
+if (update_last_part(goto_map, dest_tile)) {
+  is_goto_valid = TRUE;
+}
   } goto_map_iterate_end;
 
   /* Update goto data in info label. */
@@ -1021,15 +1025,21 @@
   Send the current patrol route (i.e., the one generated via HOVER_STATE)
   to the server.
 **/
-void send_patrol_route(void)
+void send_patrol_route(struct tile *ptile)
 {
   assert(is_active);
   goto_map_iterate(goto_maps, goto_map, punit) {
 int i;
+struct part *last_part = goto_map-parts[goto_map-num_parts - 1];
 struct pf_path *path = NULL, *return_path;
 struct pf_parameter parameter = goto_map-template;
 struct pf_map *map;
 
+if (last_part-end_tile != ptile) {
+  /* Cannot move there */
+  continue;
+}
+
 parameter.start_tile = goto_map-parts[goto_map-num_parts - 1].end_tile;
 parameter.moves_left_initially
   = goto_map-parts[goto_map-num_parts - 1].end_moves_left;
@@ -1059,15 +1069,21 @@
   Send the current connect route (i.e., the one generated via HOVER_STATE)
   to the server.
 **/
-void send_connect_route(enum unit_activity activity)
+void send_connect_route(struct tile *ptile, enum unit_activity activity)
 {
   assert(is_active);
   goto_map_iterate(goto_maps, goto_map, punit) {
+struct part *last_part = goto_map-parts[goto_map-num_parts - 1];
 struct pf_path *path = NULL;
 int i;
 struct packet_unit_orders p;
 struct tile *old_tile;
 
+if (last_part-end_tile != ptile) {
+  /* Cannot move there */
+  continue;
+ 

Re: [Freeciv-Dev] 2.1.0-beta5 release created

2007-08-16 Thread Marko Lindqvist
On 16/08/07, Daniel Markstedt [EMAIL PROTECTED] wrote:

 I suggest we hold off announcing or creating builds for beta5, and
 make beta6 as soon as this crashbug has been resoved.

 Addition to #39548 is release-critical too.


 - ML

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


Re: [Freeciv-Dev] (PR#39397) 2.1.0 Beta missing graphics for trident Ocean and tech Nichts

2007-08-16 Thread Christian Knoke

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

Du schriebst am 15. Aug um 17:35 Uhr:
 Christian Knoke wrote:
 Currently trunk runs and compiles ok, and beta4 svn doesn't compile
 because of lt.po, so I can't test.
 
 We're still awaiting your confirmation that this bug is fixed for you
 in S2_1?
 
 And are we ever going to have a graphic for tech None (Nichts)?

Oh sorry. Yes, these problems are solved.

Christian

-- 
Christian Knoke* * *http://cknoke.de
* * * * * * * * *  Ceterum censeo Microsoft esse dividendum.



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


Re: [Freeciv-Dev] 2.1.0-beta5 release created

2007-08-16 Thread Marko Lindqvist
On 16/08/07, William Allen Simpson [EMAIL PROTECTED] wrote:
 Daniel Markstedt wrote:
  I suggest we hold off announcing or creating builds for beta5, and
  make beta6 as soon as this crashbug has been resoved.
 
 Should be able to svn merge the changes into beta5.  Just wait a few
 days for folks to test everything.

 I'm against having different versions with same version number around.

 However, if we take this road, I'm *not* going to announce to
translators that we are about to release new beta. (Or should Daniel
make the announcement anyway) Anyway, do not commit any string changes
to S2_1 for now (or anything non-critical in that matter).


 - ML

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


Re: [Freeciv-Dev] 2.1.0-beta5 release created

2007-08-16 Thread Daniel Markstedt
On 8/16/07, Marko Lindqvist [EMAIL PROTECTED] wrote:
 On 16/08/07, William Allen Simpson [EMAIL PROTECTED] wrote:
  Daniel Markstedt wrote:
   I suggest we hold off announcing or creating builds for beta5, and
   make beta6 as soon as this crashbug has been resoved.
  
  Should be able to svn merge the changes into beta5.  Just wait a few
  days for folks to test everything.

  I'm against having different versions with same version number around.

  However, if we take this road, I'm *not* going to announce to
 translators that we are about to release new beta. (Or should Daniel
 make the announcement anyway) Anyway, do not commit any string changes
 to S2_1 for now (or anything non-critical in that matter).


  - ML


I would also prefer to just forget about beta5 and make beta6. Shall
we say I make the release late night Sunday 19th GMT (early Monday
morning for me.)

I'll announce it to the i18n list.

 ~Daniel

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


[Freeciv-Dev] 2.1.0-beta5 release created

2007-08-16 Thread Marko Lindqvist
On 16/08/07, Daniel Markstedt [EMAIL PROTECTED] wrote:

  beta6. Shall
 we say I make the release late night Sunday 19th GMT (early Monday
 morning for me.)

 Ok


 - ML

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


Re: [Freeciv-Dev] (PR#39536) Stable branch version number

2007-08-16 Thread Marko Lindqvist

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

On 11/08/07, Marko Lindqvist [EMAIL PROTECTED] wrote:

  I think we should set real release number for stable branch only for
 the time it takes to tag. That is:
 1) Set release version number to branch
 2) Tag
 3) Set branch version number to branch.

  Another possibility would be to tag with branch version number, and
 then to correct version number in tag (and now svn purists will kill
 me...)

 We have to decide which method to use when releasing beta6. I prefer
first one, but if Daniel decides to use second, I'm not complaining.
 This ticket can be closed when procedure is documented.


 - ML



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


Re: [Freeciv-Dev] (PR#39573) civserver crash when loading tutorial.sav

2007-08-16 Thread Marko Lindqvist

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

On 16/08/07, Daniel Markstedt [EMAIL PROTECTED] wrote:

 Detta är servern för Freeciv version 2.1.0-beta4 (betaversion).

 Not *the* beta4, but quite recent svn version of S2_1 (before version
number bump to beta4+), I presume.

 In that case crash is same as PR#39570, already fixed.


 - ML



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


[Freeciv-Dev] (PR#39574) BUG: trying to load graphics in too many places

2007-08-16 Thread William Allen Simpson

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

Reported in the past (for example, PR#39397), the client tries to load
graphics for a never displayed None -- or as the older bug(s)
improperly displayed, Nichts (fixed by PR#39389).

The iterator at client/tilespec.c tilespec_reread() is now smart enough
to start at A_FIRST. (PR#39525)

But the client/packhand.c handle_ruleset_tech() also tries to load the
graphic.  Why are the graphics loaded in multiple places?

Moreover, this seems a symptom of a larger problem.  It would be far
more efficient to only load graphics as the set changes, rather than
piecemeal as individual packets arrive.



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


Re: [Freeciv-Dev] (PR#39574) BUG: trying to load graphics in too many places

2007-08-16 Thread Marko Lindqvist

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

On 16/08/07, William Allen Simpson [EMAIL PROTECTED] wrote:

 Moreover, this seems a symptom of a larger problem.  It would be far
 more efficient to only load graphics as the set changes, rather than
 piecemeal as individual packets arrive.

 I fixed some memory management problems in this some months ago, and
if I remember correctly (can't check code now):
 When tileset changes, it *is* loaded in one place.
 However, initial tileset (after ruleset change) is loaded piece by
piece as information about what graphics are actually needed. becomes
available. Still, it should be possible to load tileset only after we
know all the required tags (or is there some problme I can't remember.
I looked that a bit, but for some reason did nothing. Should check if
I created some ticket related to this at the time)


 - ML



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


Re: [Freeciv-Dev] (PR#39536) Stable branch version number

2007-08-16 Thread William Allen Simpson

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

Marko Lindqvist wrote:
  This ticket can be closed when procedure is documented.
 
For svn purists the latter is preferable.

Remember, unlike CVS, in subversion tags are the same as branches.  There
is no implementation difference, zero copy, and all the usual subversion
advantages.  From the manual:

   In Subversion, there's no difference between a tag and a branch.
   Both are just ordinary directories that are created by copying.

That's why setting the version in the tagged branch is the right thing to
do.  It's a property of the snapshot, not of the S2_1 branch.

And why svn merge of recent S2_1 changes into beta5 is OK.  I really hate
the idea of skipping beta5.  Folks will have to explain what happened.
Beta5 has not yet been released to the public.  Yes, there are a few
versions floating around among developers -- but who cares?



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


[Freeciv-Dev] (PR#39575) Redefinition of packets crash

2007-08-16 Thread Pepeto _

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

at revision 13068, 13069 and 13070, ATTRIBUTE_CHUNK_SIZE had been changed.
But, ATTRIBUTE_CHUNK_SIZE is used to define packets.

PACKET_PLAYER_ATTRIBUTE_CHUNK=47; pre-send, sc,cs,handle-via-packet
  UINT32 offset, total_length, chunk_length;
  /* to keep memory management simple don't allocate dynamic memory */
  MEMORY data[ATTRIBUTE_CHUNK_SIZE:chunk_length];
end

This cause nowadays lot of server's crash in metaserver. The half of the
players have ATTRIBUTE_CHUNK_SIZE = 1400, the other ATTRIBUTE_CHUNK_SIZE
= 2*1024.

IT'S VERY DANGEROUS TO CHANGE PACKET DEFINITIONS WITHOUT REDEFINE
CAPABILITY STRING.

Please make something fast. Thank you.

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


Re: [Freeciv-Dev] (PR#39462) goto command for few unit crash

2007-08-16 Thread William Allen Simpson

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

I like the more recent patch much less.  It doesn't get rid of the inane
static variables.

Oh well, it's up to you.  But settle on something that works well, so
that it can be adequately tested before Saturday.



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


Re: [Freeciv-Dev] (PR#39558) BUG: science tree not displaying

2007-08-16 Thread William Allen Simpson

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

Michael Kaufman wrote:
 sorry, yes inventions are technically trinary, but for purposes of passing
 to the client, they're binary, since update_research() is called upon
 receiving a player_info packet.
 
Wow, is that wrong!  I'll delete that terrible mistake as soon as I can
test it.  All research and diplomacy and advances are handled and
verified in the server.  The server version is canonical.  It's hard
enough to keep things synchronized, without clients recalculating!


 Do not get me started on attributes. The faster they die a horrible death,
 the better we will all feel (except maybe Raimar), 

I'll start a new report.


 I've heard the bit about premature optimization, but in this case, you're
 going backward.
 
Actually forward, fixing a horrible implementation flaw.  It may not be the
final efficient answer, as sending the whole list is useless -- there will
never be more than 1 or 2 changes per player per turn.

While I understand from comments in some of the documentation that once
upon a time more intelligence was in the clients, that didn't work.

Professionally, I've successfully implemented a distributed game client
system (no servers at all), where every client made every calculation
independently, and verified the reputation of other clients.  That
requires a serious up-front design.

And that's not what is done in this project.



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


Re: [Freeciv-Dev] (PR#39575) Redefinition of packets crash

2007-08-16 Thread William Allen Simpson

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

Pepeto _ wrote:
 IT'S VERY DANGEROUS TO CHANGE PACKET DEFINITIONS WITHOUT REDEFINE
 CAPABILITY STRING.
 
There is no documented packet specific capability string.

There is no released version with this new code.

IT'S VERY DANGEROUS TO RUN DEVELOPMENT CODE AGAINST RELEASED SERVERS.

You want to run development code, you also have to use servers from the
exact same commit.  BY DEFINITION!!!


 Please make something fast. Thank you.
 
That's entirely up to you.  I tried to get folks to officially release an
updated 2.0.10 (PR#39441), but no joy.



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


[Freeciv-Dev] (PR#39575) Redefinition of packets crash

2007-08-16 Thread Pepeto _

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

 [wsimpson - Jeu. Aoû. 16 13:48:36 2007]:
 
 Pepeto _ wrote:
 There is no released version with this new code.
 
 IT'S VERY DANGEROUS TO RUN DEVELOPMENT CODE AGAINST RELEASED SERVERS.
 
 You want to run development code, you also have to use servers from the
 exact same commit.  BY DEFINITION!!!
You are wrong, the 2.0 branch is supposed be stable. All the user with a
2.0.* client would be able to play on a 2.0.* server without
compatibility problems.

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


Re: [Freeciv-Dev] (PR#39575) Redefinition of packets crash

2007-08-16 Thread Marko Lindqvist

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

On 16/08/07, William Allen Simpson [EMAIL PROTECTED] wrote:

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

 Pepeto _ wrote:
  IT'S VERY DANGEROUS TO CHANGE PACKET DEFINITIONS WITHOUT REDEFINE
  CAPABILITY STRING.
 
 There is no documented packet specific capability string.

 version.in, common/capsomething.c

 IT'S VERY DANGEROUS TO RUN DEVELOPMENT CODE AGAINST RELEASED SERVERS.

 The very idea of capability string is to make sure that incompatible
server/client do not accept connections from each other.
 ...and if we are supposed to yell:
 IT'S VERY STUPID TO INSIST DOING SAME MISTAKE AFTER IT HAS BEEN
POINTED OUT TO YOU.



 - ML



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


Re: [Freeciv-Dev] (PR#39575) Redefinition of packets crash

2007-08-16 Thread Marko Lindqvist

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

On 16/08/07, Pepeto _ [EMAIL PROTECTED] wrote:

 at revision 13068, 13069 and 13070, ATTRIBUTE_CHUNK_SIZE had been changed.
 But, ATTRIBUTE_CHUNK_SIZE is used to define packets.

 Yes, this has to reverted from S2_1 and S2_0 immediately. I have no
access to my development machine now.
 And trunk needs new manditory capability.


 - ML



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


[Freeciv-Dev] (PR#39577)

2007-08-16 Thread Pepeto _

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

When you bribe a city in 2.0, the units are not transfered correctly.
Actually, you get all the homecities of the units.

It seems there were an error at revision 13057:
(PR#39431) Cannot add settlers to city

patch by Timothy Brownawell [EMAIL PROTECTED]

Previous S2_1 revision 13052, trunk revision 13053.

Also, another wipe_unit(vunit) removed from server/citytools.c, 
already removed from S2_1, but I cannot find the bug report.

But wipe_unit(vunit) in 2.0 is needed, because transfer_unit() makes
already a new unit. In 2.1, the same structure is used, so there is no
problem.

--- server/citytools.c.old	2007-08-16 17:32:00.0 +0200
+++ server/citytools.c	2007-08-16 17:32:19.0 +0200
@@ -619,6 +619,7 @@
   /* Don't transfer units already owned by new city-owner --wegge */ 
   if (unit_owner(vunit) == pvictim) {
 	transfer_unit(vunit, pcity, verbose);
+wipe_unit(vunit);
 	unit_list_unlink(units, vunit);
   } else if (!pplayers_allied(pplayer, unit_owner(vunit))) {
 /* the owner of vunit is allied to pvictim but not to pplayer */
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#39575) Redefinition of packets crash

2007-08-16 Thread William Allen Simpson

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

Marko Lindqvist wrote:
 On 16/08/07, William Allen Simpson [EMAIL PROTECTED] wrote:
 There is no documented packet specific capability string.
 
  version.in, common/capsomething.c
 
That is not a documented packet specific capability string.

It is neither:
  1) documented
nor
  2) packet specific

Traditionally, version.in is for releases, and should not be updated more
than once per release cycle.  It is not accessed except during configure.

You are misusing long understood industry terminology.

If there were packet specific capabilities, they would number in the
hundreds or thousands.  And would have been included in the packets.def
file, to be generated for every handler, and every individual packet.

For example, like tilespec capabilities, in each and every file.


 IT'S VERY DANGEROUS TO RUN DEVELOPMENT CODE AGAINST RELEASED SERVERS.
 
  The very idea of capability string is to make sure that incompatible
 server/client do not accept connections from each other.

Then every development version.in needs a capability that is *never* in
the released version.in.  Because nobody should willy-nilly be compiling
development code and running it as or against a public server!

It's really not my problem that this project cannot get releases out the
door in a timely fashion, so poor saps in the field try to hack something
together as their own pseudo-release.

That's the fault of certain persons that disappear for months at a time,
leaving the makefiles, configuration, and code in a sorry state.


  IT'S VERY STUPID TO INSIST DOING SAME MISTAKE AFTER IT HAS BEEN
 POINTED OUT TO YOU.
 
It's not a mistake.  And you only mentioned such a thing in a very recent
posting.  The code of which he complains is from some time ago.




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


Re: [Freeciv-Dev] (PR#39575) Redefinition of packets crash

2007-08-16 Thread William Allen Simpson

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

Pepeto _ wrote:
 You are wrong, the 2.0 branch is supposed be stable. All the user with a
 2.0.* client would be able to play on a 2.0.* server without
 compatibility problems.
 
You don't have a 2.0.* client.  That's an unreleased development client.

Moreover, should you insist that all 2.0.x clients work forever with all
2.0.x servers, then you'll just have to live with the crashes the bug
fixes prevent.  I for one vow to never fix another 2.0 bug.

After all, 2.0.9 crashes on the first turn!  A fine thing to preserve



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


[Freeciv-Dev] (PR#39575) Using development code with released servers

2007-08-16 Thread Pepeto _

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

 [wsimpson - Jeu. Aoû. 16 15:22:31 2007]:
 
 Pepeto _ wrote:
  You are wrong, the 2.0 branch is supposed be stable. All the user with a
  2.0.* client would be able to play on a 2.0.* server without
  compatibility problems.
  
 You don't have a 2.0.* client.  That's an unreleased development client.
 
 Moreover, should you insist that all 2.0.x clients work forever with all
 2.0.x servers, then you'll just have to live with the crashes the bug
 fixes prevent.  I for one vow to never fix another 2.0 bug.
Yes i insist: all 2.0.x clients should work forever with all 2.0.x
servers. This is a compatibility way.

 After all, 2.0.9 crashes on the first turn!  A fine thing to preserve
I never said I wanted 2.0.9 crash at the first turn. The thing to change
is the definition of ATTRIBUTE_CHUNK_SIZE, which wouldn't be changed.

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


Re: [Freeciv-Dev] (PR#39577)

2007-08-16 Thread William Allen Simpson

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

Pepeto _ wrote:
 But wipe_unit(vunit) in 2.0 is needed, because transfer_unit() makes
 already a new unit. In 2.1, the same structure is used, so there is no
 problem.
 
Then that's a bug in transfer_unit().  My fix prevents a reported crash in
the following line at unit_list_unlink, an invalid vunit.  You cannot wipe
the unit before unlinking:

   wipe_unit(vunit);
   unit_list_unlink(units, vunit);

Clearly, you never tested your proposed patch.  That makes all your other
patches suspect.

Moreover, the code in 2.0 is so bad, it's a waste of time.  Live with
2.0.9!



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


[Freeciv-Dev] (PR#39577)

2007-08-16 Thread Pepeto _

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

 [wsimpson - Jeu. Aoû. 16 15:33:37 2007]:
 
 Pepeto _ wrote:
  But wipe_unit(vunit) in 2.0 is needed, because transfer_unit() makes
  already a new unit. In 2.1, the same structure is used, so there is no
  problem.
  
 Then that's a bug in transfer_unit().  My fix prevents a reported crash in
 the following line at unit_list_unlink, an invalid vunit.  You cannot wipe
 the unit before unlinking:
 
wipe_unit(vunit);
unit_list_unlink(units, vunit);
 
 Clearly, you never tested your proposed patch.  That makes all your other
 patches suspect.
I tested my patch. It doesn't crash since you don't use vunit as a
structure but as a pointer. But, you are right, it's better to call
unit_list_unlink() before. See the attached patch...

 Moreover, the code in 2.0 is so bad, it's a waste of time.  Live with
 2.0.9!
No comment...
--- server/citytools.c.old	2007-08-16 17:32:00.0 +0200
+++ server/citytools.c	2007-08-16 19:16:26.0 +0200
@@ -620,6 +620,7 @@
   if (unit_owner(vunit) == pvictim) {
 	transfer_unit(vunit, pcity, verbose);
 	unit_list_unlink(units, vunit);
+wipe_unit(vunit);
   } else if (!pplayers_allied(pplayer, unit_owner(vunit))) {
 /* the owner of vunit is allied to pvictim but not to pplayer */
 bounce_unit(vunit, verbose);
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#39558) BUG: science tree not displaying

2007-08-16 Thread Michael Kaufman

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

 Michael Kaufman wrote:
  sorry, yes inventions are technically trinary, but for purposes of passing
  to the client, they're binary, since update_research() is called upon
  receiving a player_info packet.
  
 Wow, is that wrong!  I'll delete that terrible mistake as soon as I can
 test it.  All research and diplomacy and advances are handled and
 verified in the server.  The server version is canonical.  It's hard
 enough to keep things synchronized, without clients recalculating!

Not necessarily wrong [or not necessarily terrible]. as I recall
update_research() only updates TECH_REACHABLE, which has a rigid
definition [and really is only a cached value]: It doesn't matter whether 
the server tells the client or the client calculates it itself. The server 
still keeps the master record of inventions.

  I've heard the bit about premature optimization, but in this case, you're
  going backward.
  
 Actually forward, fixing a horrible implementation flaw.  It may not be the
 final efficient answer, as sending the whole list is useless -- there will
 never be more than 1 or 2 changes per player per turn.

If the list you're talking about is the inventions list, I don't think you 
get much benefit from say sending the number of changes/the invention
indices/and the states or just sending the whole list in a bit array.

The correct fix is to split the player_info packet into multiple packets.

M



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


[Freeciv-Dev] (PR#39579) BUG: too many rulesets sent to client

2007-08-16 Thread William Allen Simpson

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

A verbose log of packets shows that for a single client game, loading,
and then immediately quitting, the entire ruleset is sent 3 times!

Some rulesets are sent 5 times:
   PACKET_RULESET_NATION_GROUPS
   PACKET_RULESET_NATION

The packet design is seriously flawed.

In a client/server architecture, the client should request a ruleset, and
the server should send it.  One transaction at a time.  Freeze and thaw
hints (from the server) should not be needed (at least for this function).

Moreover, in a client initiated server, the control security HACK
request and response should occur at the beginning, allowing selection
of savegames before loading *any* rulesets.

Graphics related to rulesets should be loading after the appropriate
ruleset is loaded, not one at a time for each ruleset packet.



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


[Freeciv-Dev] (PR#39578) BUG: too many player_info packets, with wrong information, causing recalculation

2007-08-16 Thread William Allen Simpson

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

Just starting a saved game (with 3 players) has too many player_info packets.

The comments in client/packhand.c say:

   /* If the server sends out player information at the wrong time, it is
* likely to give us inconsistent player tech information, causing a
* sanity-check failure within this function.  Fixing this at the client
* end is very tricky; it's hard to figure out when to read the techs
* and when to ignore them.  The current solution is that the server should
* only send the player info out at appropriate times - e.g., while the
* game is running. */

Obviously, based on a load followed by immediate quit, the server isn't
meeting that restriction:

3: Player0 
inventions:
3: Player0 
inventions:
3: Player0 
inventions:
3: Player0 
inventions:
3: Player0 
inventions:2000
3: Player0 
inventions:2000
3: Player0 
inventions:2000
3: Player1 
inventions:2000
3: Player2 
inventions:2000
3: Player0 
inventions:20200220020010010020022012020001000210002012
3: Player1 
inventions:2000
3: Player2 
inventions:2000
3: Player0 
inventions:2000
3: Player1 
inventions:2000
3: Player2 
inventions:2000
3: Player0 
inventions:20200220020010010020022012020001000210002012
3: Player1 
inventions:2000
3: Player2 
inventions:2000
3: Player0 
inventions:20200220020010010020022012020001000210002012
3: Player1 
inventions:2000
3: Player2 
inventions:2000
3: Player0 
inventions:20200220020010010020022012020001000210002012
3: Player0 
inventions:20200220020010010020022012020001000210002012
3: Player1 
inventions:2000
3: Player2 
inventions:2000
3: Player0 
inventions:20200220020010010020022012020001000210002012

Note that the first are sent with no A_NONE at all!  There are comments in
various places as the A_NONE is repeatedly set again, trying to fix the
problems.

Note that player0 inventions are flapping, probably due to an internal
problem in the server, causing recalculation.



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


Re: [Freeciv-Dev] (PR#39575) Redefinition of packets crash

2007-08-16 Thread Per I. Mathisen

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

On Thu, 16 Aug 2007, William Allen Simpson wrote:
 If there were packet specific capabilities

There are. See doc/README.delta

   - Per



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


[Freeciv-Dev] (PR#39575) Using development code with released servers

2007-08-16 Thread Pepeto _

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

 [per - Jeu. Aoû. 16 22:10:35 2007]:
 
 On Thu, 16 Aug 2007, William Allen Simpson wrote:
  You don't have a 2.0.* client.  That's an unreleased development client.
 
 We have for a long time been running a fresh checkout of stable branches 
 for pubserver games, since this way we could get the most important bug 
 fixes out effective immediately. I do not think pubserver is working at 
 the moment, though.
 
- Per
 
You are right pubserver is not running. But warclient, which is based on
freeciv use this code. So any change make (notably network change) are
reported on the code.


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


Re: [Freeciv-Dev] (PR#39575) Redefinition of packets crash

2007-08-16 Thread Per I. Mathisen

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

On Thu, 16 Aug 2007, William Allen Simpson wrote:
 You don't have a 2.0.* client.  That's an unreleased development client.

We have for a long time been running a fresh checkout of stable branches 
for pubserver games, since this way we could get the most important bug 
fixes out effective immediately. I do not think pubserver is working at 
the moment, though.

   - Per



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


Re: [Freeciv-Dev] (PR#39576) RFE: rethinking CMA client attributes

2007-08-16 Thread Per I. Mathisen

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

On Thu, 16 Aug 2007, William Allen Simpson wrote:
 Problem: the attributes are sent and stored as serialized data, so the
 server has no way to check validity of format.  Many reports of bad
 lengths, and other validity problems.  The problems are not detected until
 sent back to the client after loading a savegame.

 So, the immediate solution is to validate the data in the server to catch
 the problem as it occurs, before it is saved.

 However, there is a question as to their usefulness at all (PR#39558)?

I agree with Mike: Attributes are the wrong solution to whatever problem 
they were meant to solve. I understand the reasoning behind it, a generic 
way to save client state that the server need not understand, but I 
disagree with it, as I do not believe that developing the client and 
server in separation is ever going to be desirable.

   - Per



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


[Freeciv-Dev] (PR#39462) goto command for few unit crash

2007-08-16 Thread Pepeto _

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

This is the final patch, which include a new get_line_dest() function
(now, it would really return the destination tile and not the first unit
destination which could be his own tile). I tried to optimize the code
in time.

Sorry, I know you don't like static variables, but I inserted a new one :)

--- client/control.c.old	2007-08-15 20:28:54.0 +0200
+++ client/control.c	2007-08-17 03:21:36.0 +0200
@@ -2035,15 +2035,12 @@
 **/
 void do_unit_goto(struct tile *ptile)
 {
-  struct tile *dest_tile;
-
   if (hover_state != HOVER_GOTO  hover_state != HOVER_NUKE) {
 return;
   }
 
   draw_line(ptile);
-  dest_tile = get_line_dest();
-  if (ptile == dest_tile) {
+  if (is_valid_goto_destination(ptile)) {
 send_goto_route();
   } else {
 create_event(ptile, E_BAD_COMMAND,
@@ -2072,15 +2069,12 @@
 **/
 void do_unit_patrol_to(struct tile *ptile)
 {
-  struct tile *dest_tile;
-
   draw_line(ptile);
-  dest_tile = get_line_dest();
-  if (ptile == dest_tile
+  if (is_valid_goto_destination(ptile)
!is_non_allied_unit_tile(ptile, game.player_ptr)) {
 send_patrol_route();
   } else {
-create_event(dest_tile, E_BAD_COMMAND,
+create_event(ptile, E_BAD_COMMAND,
 		 _(Didn't find a route to the destination!));
   }
 
@@ -2093,11 +2087,8 @@
 void do_unit_connect(struct tile *ptile,
 		 enum unit_activity activity)
 {
-  struct tile *dest_tile;
-
   draw_line(ptile);
-  dest_tile = get_line_dest();
-  if (same_pos(dest_tile, ptile)) {
+  if (is_valid_goto_destination(ptile)) {
 send_connect_route(activity);
   } else {
 create_event(ptile, E_BAD_COMMAND,
--- client/goto.c.old	2007-08-15 20:29:08.0 +0200
+++ client/goto.c	2007-08-17 03:16:18.0 +0200
@@ -112,6 +112,7 @@
 static bool is_init = FALSE;
 static bool is_goto_valid = FALSE;
 static int connect_initial;
+static struct tile *goto_tile = NULL;
 
 /
   Create a new goto map.
@@ -190,8 +191,9 @@
 /** 
   Change the destination of the last part to the given position if a
   path can be found. If not the destination is set to the start.
+  Return TRUE if the new path is found.
 ***/
-static void update_last_part(struct goto_map *goto_map, struct tile *ptile)
+static bool update_last_part(struct goto_map *goto_map, struct tile *ptile)
 {
   struct part *p = goto_map-parts[goto_map-num_parts - 1];
   struct pf_path *new_path;
@@ -202,12 +204,10 @@
   TILE_XY(ptile), TILE_XY(p-start_tile), TILE_XY(p-end_tile));
   new_path = pf_get_path(p-map, ptile);
 
-  is_goto_valid = (new_path != NULL);
-
   if (!new_path) {
 freelog(PATH_LOG_LEVEL,   no path found);
 reset_last_part(goto_map);
-return;
+return FALSE;
   }
 
   freelog(PATH_LOG_LEVEL,   path found:);
@@ -277,6 +277,8 @@
   /* Refresh tiles so turn information is shown. */
   refresh_tile_mapcanvas(old_tile, FALSE, FALSE);
   refresh_tile_mapcanvas(ptile, FALSE, FALSE);
+
+  return TRUE;
 }
 
 /** 
@@ -802,6 +804,7 @@
 goto_map_list_append(goto_maps, goto_map);
   } unit_list_iterate_end;
   is_active = TRUE;
+  is_goto_valid = TRUE;
 }
 
 /** 
@@ -821,6 +824,8 @@
   } goto_map_list_iterate_end;
   goto_map_list_unlink_all(goto_maps);
 
+  is_goto_valid = FALSE;
+  goto_tile = NULL;
   is_active = FALSE;
 }
 
@@ -837,14 +842,7 @@
 ***/
 struct tile *get_line_dest(void)
 {
-  if (is_active) {
-struct goto_map *goto_map = goto_map_list_get(goto_maps, 0);
-struct part *p = goto_map-parts[goto_map-num_parts - 1];
-
-return p-end_tile;
-  } else {
-return NULL;
-  }
+  return goto_tile;
 }
 
 /**
@@ -885,8 +883,12 @@
 {
   assert(is_active);
 
+  goto_tile = dest_tile;
+  is_goto_valid = FALSE;
   goto_map_iterate(goto_maps, goto_map, punit) {
-update_last_part(goto_map, dest_tile);
+if (update_last_part(goto_map, dest_tile)) {
+  is_goto_valid = TRUE;
+}
   } goto_map_iterate_end;
 
   /* Update goto data in info label. */
@@ -1026,10 +1028,16 @@
   assert(is_active);
   goto_map_iterate(goto_maps, goto_map, punit) {
 int i;
+struct part *last_part = goto_map-parts[goto_map-num_parts - 1];
 struct pf_path *path = NULL, *return_path;
 struct pf_parameter parameter = goto_map-template;
 struct pf_map *map;
 
+if (last_part-end_tile != goto_tile) {
+  /* Cannot move there */
+  continue;