[Freeciv-Dev] [patch #1296] restructure basic notify functions

2009-09-30 Thread Matthias Pfafferodt

Follow-up Comment #8, patch #1296 (project freeciv):

 I think the reset of the coordinates (x,y) within the packet
 is needed, as the same packet struct could also be used for 
 offline events. 

One remark: if the packet should be used for another function, it should not
be changed at all. Does this mean a 'const' keyword would be needed? This
would again require to make a copy of the packet to change the coordinates.

 I do the same. I am working on Freeciv code for 3 years, and I 
 still ignore some things. 

Nice to know that I'm not alone ;-)

___

Reply to this item at:

  http://gna.org/patch/?1296

___
  Nachricht geschickt von/durch Gna!
  http://gna.org/


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


[Freeciv-Dev] [patch #1296] restructure basic notify functions

2009-09-29 Thread pepeto

Follow-up Comment #7, patch #1296 (project freeciv):

 if (ptile) {
 /* Is it useful? */
 packet-x = ptile-x;
 packet-y = ptile-y;
 }


 I think the reset of the coordinates (x,y) within the packet is
 needed, as the same packet struct could also be used for offline
 events.

Changed this comment to 'Set the coordinates back, to make the packet still
usable.'

 Why is this not simple strncpy()?


 I do most of my coding as learning-by-doing. Sorry if I did
 not choose the right function or if there are bad code
 sections. Here I did select the first function which did that
 I want. You did delete the block completely and I think its a
 better solution.

I do the same.  I am working on Freeciv code for 3 years, and I still ignore
some things.

I am sorry if sometimes, the comments looks a bit too affirmative, but
actually, there are advices.  Don't worry if people comment your patch, it
marks at least they have interest on it.

Actually the usage of strncpy() is not the best in my opinion in this case. 
sz_strlcpy() (defined in utility/support.h) alias auto-sized mystrlcpy()
should be faster because it stops when the string ends and unsure it finish
by a nul character.

In your previous patch, you should also have used only: 'packet_send =
*packet;', then all fields, including the string would have been copied.


(file #6829)
___

Additional Item Attachment:

File name: trunk_S2_2_restructure_notify_conn2.diff Size:7 KB


___

Reply to this item at:

  http://gna.org/patch/?1296

___
  Message posté via/par Gna!
  http://gna.org/


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


[Freeciv-Dev] [patch #1296] restructure basic notify functions

2009-09-16 Thread pepeto

Follow-up Comment #4, patch #1296 (project freeciv):

 + my_snprintf(packet_send.message, sizeof(packet_send.message), %s,
 + packet-message);


 Why is this not simple strncpy()?

Or sz_strlcpy(packet_send.message, packet-message)?

___

Reply to this item at:

  http://gna.org/patch/?1296

___
  Message posté via/par Gna!
  http://gna.org/


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


[Freeciv-Dev] [patch #1296] restructure basic notify functions

2009-09-13 Thread Matthias Pfafferodt

URL:
  http://gna.org/patch/?1296

 Summary: restructure basic notify functions
 Project: Freeciv
Submitted by: syntron
Submitted on: Sonntag 13.09.2009 um 18:09
Category: general
Priority: 5 - Normal
  Status: None
 Privacy: Public
 Assigned to: None
Originator Email: 
 Open/Closed: Open
 Discussion Lock: Any
 Planned Release: 2.3.0

___

Details:

this patch includes a cleanup of the notify_* functions.

* the new function fill_packet_chat_msg() is used to generate the struct
packet_chat_msg
* vnotify_conn() is replaced by notify_conn_packet()

As all the notify_* functions now have the message as struct packet_chat_msg
this can directly be used for offline events (patch #1280)



___

File Attachments:


---
Date: Sonntag 13.09.2009 um 18:09  Name:
0001-restructure-basic-notify-functions.diff  Size: 8kB   By: syntron

http://gna.org/patch/download.php?file_id=6671

___

Reply to this item at:

  http://gna.org/patch/?1296

___
  Nachricht geschickt von/durch Gna!
  http://gna.org/


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


[Freeciv-Dev] [patch #1296] restructure basic notify functions

2009-09-13 Thread pepeto

Follow-up Comment #1, patch #1296 (project freeciv):

It looks a good patch.  But I noticed an error in notify_conn_packet(): if
one connection is not allowed to receive the tile coordinates, then the
coordinates in the packets are set to -1 (which is right), but then, if the
next connection of the list is allowed to see that tile, the packet
coordinates are not computed back.

Also, I'm not sure about:
  if (packet-x != -1  packet-y != -1) {
ptile = map_pos_to_tile(packet-x, packet-y);
  } else {
assert(!is_normal_map_pos(-1, -1));
ptile = NULL;
  }

I would have done:
  if (is_normal_map_pos(packet-x, packet-y)) {
ptile = map_pos_to_tile(packet-x, packet-y);
  } else {
ptile = NULL;
  }
like in client side.


___

Reply to this item at:

  http://gna.org/patch/?1296

___
  Message posté via/par Gna!
  http://gna.org/


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