http://bugs.freeciv.org/Ticket/Display.html?id=39850 >
This code was added by "(PR#13773) Improved cursors", which has Jason's
comment:
"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).
client/control.c:63:
/* 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;
client/control.c:939:
if (!ptile) {
if (hover_tile) {
/* hover_tile is the tile which is currently under the mouse cursor. */
ptile = hover_tile;
} else {
return;
}
}
===
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:
client/gui-gtk-2.0/mapctrl.c:411:
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
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
