<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39850 >

This code was added by "(PR#13773) Improved cursors", which has Jason's

   "I don't understand these changes, and I suspect they are wrong."

Always comforting....  The report doesn't say who committed it.


The comments about it are both wrong.  It's not in set_hover_state(), and
it's not the current tile (it's the tile from the previous call).

/* These should be set via set_hover_state() */
enum cursor_hover_state hover_state = HOVER_NONE;
struct tile *hover_tile = NULL;
enum cursor_action_state action_state = CURSOR_ACTION_DEFAULT;

   if (!ptile) {
     if (hover_tile) {
       /* hover_tile is the tile which is currently under the mouse cursor. */
       ptile = hover_tile;
     } else {


It makes no sense to me -- hover_tile is always set just after calling
handle_mouse_cursor() with a non-NULL tile.

client/gui-gtk-2.0/gui_main.c:652:    hover_tile = ptile;
client/gui-gtk-2.0/mapctrl.c:388:    hover_tile = ptile;
client/gui-sdl/gui_main.c:416:          hover_tile = ptile;
client/gui-win32/mapctrl.c:70:    hover_tile = ptile;


Only one case with a non-NULL tile doesn't set hover_tile, at:

     handle_mouse_cursor(canvas_pos_to_tile(canvas_x, canvas_y));
     /* update_unit_info_label is handled inside handle_mouse_cursor. */

The fact that's not called here is probably a bug!


The NULL calls are:

client/civclient.c:794:      handle_mouse_cursor(NULL);
client/control.c:899:      handle_mouse_cursor(NULL);
client/control.c:1079:      handle_mouse_cursor(NULL);
client/gui-gtk-2.0/mapview.c:181:    handle_mouse_cursor(NULL);


It's only used inside handle_mouse_cursor(), so I don't see why it
wouldn't have been set and cleared only in handle_mouse_cursor()....

My conclusion is hover_tile is useless and should be removed, or at the
very least restricted to inside handle_mouse_cursor().

And it shouldn't be named "handle_" as it's not a packet call.

Freeciv-dev mailing list

Reply via email to