[Freeciv-Dev] (PR#18010) goto segfault in pft_concat() from send_goto_route()
http://bugs.freeciv.org/Ticket/Display.html?id=18010 > > [wsimpson - Jeu. Aoû. 23 13:46:47 2007]: > > Pepeto _ wrote: > > 15) the first tile (the unit tile) is an invalid destination when the > > unit just entered in the goto state whereas, it's valid if you move > > first the cursor on an other tile. > > > Odd, pf_* code is sometimes returning a "valid" path of no moves? > Yes, it does. It returns a path of length 1, with 1 pf_position :) ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#18010) goto segfault in pft_concat() from send_goto_route()
http://bugs.freeciv.org/Ticket/Display.html?id=18010 > Pepeto _ wrote: > 15) the first tile (the unit tile) is an invalid destination when the > unit just entered in the goto state whereas, it's valid if you move > first the cursor on an other tile. > Odd, pf_* code is sometimes returning a "valid" path of no moves? > 16) Why the nukes ignore the dangerous position? Isn't this quite strange? > No idea. Perhaps the logic is that nothing is dangerous to a transcontinental missile!?!? Not related to this problem report > Anyway, very good job William. > Committed trunk revision 13382, S2_1 revision 13384. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#18010) goto segfault in pft_concat() from send_goto_route()
http://bugs.freeciv.org/Ticket/Display.html?id=18010 >
The working patch for trunk handles all the above issues. (The patch
for S2_1 differs in 2 lines, but we're waiting for a beta release.)
Index: client/control.c
===
--- client/control.c(revision 13376)
+++ client/control.c(working copy)
@@ -1036,18 +1036,18 @@
**/
void request_unit_connect(enum unit_activity activity)
{
- if (!can_units_do_connect(get_units_in_focus(), activity)) {
+ struct unit_list *punits = get_units_in_focus();
+
+ if (!can_units_do_connect(punits, activity)) {
return;
}
if (hover_state != HOVER_CONNECT || connect_activity != activity) {
-/* Enter or change the hover connect state. */
-set_hover_state(get_units_in_focus(), HOVER_CONNECT,
- activity, ORDER_LAST);
-update_unit_info_label(get_units_in_focus());
-
-enter_goto_state(get_units_in_focus());
+set_hover_state(punits, HOVER_CONNECT, activity, ORDER_LAST);
+enter_goto_state(punits);
create_line_at_mouse_pos();
+update_unit_info_label(punits);
+handle_mouse_cursor(NULL);
} else {
assert(goto_is_active());
goto_add_waypoint();
@@ -2078,20 +2078,16 @@
}
/**
- Finish the goto mode and let the unit which is stored in hover_unit move
+ Finish the goto mode and let the units stored in hover_units move
to a given location.
**/
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_draw_line(ptile)) {
send_goto_route();
} else {
create_event(ptile, E_BAD_COMMAND,
@@ -2120,15 +2116,11 @@
**/
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_draw_line(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!"));
}
@@ -2141,11 +2133,7 @@
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_draw_line(ptile)) {
send_connect_route(activity);
} else {
create_event(ptile, E_BAD_COMMAND,
@@ -2164,9 +2152,14 @@
cancel_tile_hiliting();
- if (hover_state == HOVER_GOTO || hover_state == HOVER_PATROL) {
+ switch (hover_state) {
+ case HOVER_GOTO:
+ case HOVER_PATROL:
+ case HOVER_CONNECT:
popped = goto_pop_waypoint();
- }
+ default:
+break;
+ };
if (hover_state != HOVER_NONE && !popped) {
set_hover_state(NULL, HOVER_NONE, ACTIVITY_LAST, ORDER_LAST);
Index: client/text.c
===
--- client/text.c (revision 13376)
+++ client/text.c (working copy)
@@ -699,8 +699,9 @@
if (count > 0 && unit_list_size(hover_units) > 0) {
int min, max;
-goto_get_turns(&min, &max);
-if (min == max) {
+if (!goto_get_turns(&min, &max)) {
+ astr_add_line(&str, _("Turns to target: %d"), min);
+} else if (min == max) {
astr_add_line(&str, _("Turns to target: %d"), max);
} else {
astr_add_line(&str, _("Turns to target: %d to %d"), min, max);
Index: client/goto.c
===
--- client/goto.c (revision 13376)
+++ client/goto.c (working copy)
@@ -39,9 +39,9 @@
#define PACKET_LOG_LEVELLOG_DEBUG
/*
- * The whole path is seperated by waypoints into parts. The number of parts is
- * number of waypoints + 1. Each part has its own starting position and
- * therefore requires it's own map.
+ * The whole path is separated by waypoints into parts. Each part has its
+ * own starting position and requires its own map. When the unit is unable
+ * to move, end_tile equals start_tile.
*/
struct part {
int start_moves_left, start_fuel_left;
@@ -62,14 +62,17 @@
* only one has number less than 4
* 2. There _can_ be more than one line drawn between two tiles, because of
* the waypoints. */
-static struct {
+struct goto_tiles {
unsigned char drawn[4];
-} *tiles;
+};
+static struct goto_tiles *tiles = NULL;
struct goto_map {
- int unit_id; /* The unit of the goto map */
struct part *parts;
int num_
Re: [Freeciv-Dev] (PR#18010) goto segfault in pft_concat() from send_goto_route()
http://bugs.freeciv.org/Ticket/Display.html?id=18010 > 11) request_unit_connect() doesn't update the cursor (to show invalid). 12) Escape to back out of waypoints doesn't work for HOVER_CONNECT (missing in test of hover_state). 13) The number of turns displayed in client/text.c (and its doppleganger in client/gui-sdl/mapview.c) is bad for invalid path. For now, will make it FC_INFINITE. 14) Another static variable buried in the code, this time for speed. Worse, passed as pointer to data structure(s), and then goes on through the loop re-writing the value over again. This will work only when all units in the group have exactly the same speed! ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#18010) goto segfault in pft_concat() from send_goto_route()
http://bugs.freeciv.org/Ticket/Display.html?id=18010 > I've tracked the problem to: r11378 | jdorje | 2005-12-23 -- PR#14365, battlegroups, and r11391 | jdorje | 2005-12-26 -- PR#14992, unitlists. The code is still almost identical in both S2_1 and trunk. There are quite a few problems with various degrees of seriousness. 1) The tiles array is uninitialized; its state is indicated by is_init. Would make more sense to have initially NULL. Would also make more sense to use fc_calloc, instead of whole_map_iterate() setting zero. (It seems horribly wasteful to have such a large array, one for every possible tile and direction, but that's for another time.) 2) Another semantic duplication: is_active appears to track goto_map_list_size(goto_maps) > 0, with duplicated assert(). 3) Each unit is saved in the map by id, and converted back to a pointer. Would make more sense to save the pointer in the first place. Is this a workaround for units that are destroyed during goto selection? 4) As already discovered, is_goto_valid seems to be based on the final unit in the goto_map_iterate() list. It doesn't work, as some units can be valid, and others invalid. 5) In addition, connect_initial is also based on the final unit in the goto_map_iterate() list. It seems that the value saved in each path is based on the previous unit(s). Makes no sense. 6) The parameter of is_valid_goto_destination(ptile) is unused. Clear evidence of partial implementation. 7) There are an awful lot of useless asserts. For example, just after goto_map_new(), where num_parts is set to 0, an assert checks that it's zero. More evidence of implementation problems in the past, and attempts at finding the solution. 8) There are "waypoints" that cannot possibly work for battlegroups that are not already in the same stack. Moreover, there is no method for communicating waypoints to the server. Such waypoints could become obsolete as roads are connected during the following turns. 9) The code seems to contemplate multiple paths drawn simultaneously, but doesn't work correctly. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
