[Freeciv-Dev] [Patch] Turn done button / return key (PR#1665)

[Davide Pagnin <nightmare@xxxxxxxxxx>]

        Hi all!
Content-Type: multipart/mixed; boundary="=-K7ZMLasDcN1UOKotfS5+"


--=-K7ZMLasDcN1UOKotfS5+
Content-Type: text/plain
Content-Transfer-Encoding: 7bit

        Hi all!

This is one of the "infamous" 1.13.0 bugs.

I think that many player are misjudging 1.13.0 enhancements because of
some annoying bugs (perhaps 2 or 3), and this is unfair considering how
much work has been done for presenting 1.13.0.

But I don't want to start a flaming war, this is a proposed fix of the
problem you found on civclient.

Symptoms: When you press turn done, it don't stay greyed, and you need
to press it another time (known also as "twice click on turn done
problem")

Analysis: Navigating through the code and searching the reason for such
an annoying problem, I found which instruction is responsible to make
happening the problem.

In client/agents/agents.c you found:

  if (!agents_busy() && !game.player_ptr->turn_done &&
!client_is_observer()) {
    set_turn_done_button_state(TRUE);
  }

This call to set_turn_done_button_state is responsible to uncorrectly
re-enabling the turn-done button the first time.

Even if this is the guilty call, it is correct!
In fact the statement and the call are correct AND needs to remain the
same, so what is wrong?

At the time of the check, game.player_ptr->turn_done is 0, because we
have not received the INFO packet that will change the turn_done status.

In fact, the second time we press the turn done button, the
game.player_ptr->turn_done variable is already 1 and the wrong effect is
not called.

One possible solution (the one adopted in this patch) is to not disable
the turn_done button until we receive the confirmation from the server
that our status has changed, thus falling in the second case and not in
the first!

The relevant code:

In client/civclient.c

void send_turn_done(void)
{
...
  send_packet_generic_message(&aconnection, PACKET_TURN_DONE,
&gen_packet);
  set_turn_done_button_state(FALSE);
--^^^^^^^^^^
This call has to be eliminated because the game.player_ptr->turn_done is
not 1 and thus this will be reverted by the call in agents.c

In client/pachhand.c

void handle_player_info(struct packet_player_info *pinfo)
{
...
  pplayer->turn_done=pinfo->turn_done;

  if(pplayer==game.player_ptr && pplayer->turn_done)
    set_turn_done_button_state(FALSE);
--^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Inside the handle_player_info call, we need to check our turn_done
status has change, and in this case, put the turn done button off.

This patch is "EXPERIMENTAL" and needs the review of other developers
before being considered for inclusion.

I also think that perhaps side-effects related to proper handling of
is_observer() and agents_busy() function has to be considered, but I
haven't the knowledge nor the time to find out them.

Raimar, for sure will answer to this question.

        Ciao, Davide


--=-K7ZMLasDcN1UOKotfS5+
Content-Disposition: attachment; filename=turn_done_fix.patch
Content-Transfer-Encoding: quoted-printable
Content-Type: text/x-patch; name=turn_done_fix.patch; charset=ISO-8859-1

diff -urN -Xfreeciv/diff_ignore freeciv/client/civclient.c freeciv-work/cli=
ent/civclient.c
--- freeciv/client/civclient.c  Tue Jul 23 04:48:45 2002
+++ freeciv-work/client/civclient.c     Fri Aug  2 18:28:56 2002
@@ -485,7 +485,6 @@
   gen_packet.message[0] =3D '\0';
=20
   send_packet_generic_message(&aconnection, PACKET_TURN_DONE, &gen_packet)=
;
-  set_turn_done_button_state(FALSE);
 }
 /*************************************************************************=
*
 ...
diff -urN -Xfreeciv/diff_ignore freeciv/client/packhand.c freeciv-work/clie=
nt/packhand.c
--- freeciv/client/packhand.c   Mon Jul 29 15:13:38 2002
+++ freeciv-work/client/packhand.c      Fri Aug  2 18:17:38 2002
@@ -1206,6 +1206,8 @@
   }
=20
   pplayer->turn_done=3Dpinfo->turn_done;
+  if(pplayer=3D=3Dgame.player_ptr && pplayer->turn_done)
+    set_turn_done_button_state(FALSE);
   pplayer->nturns_idle=3Dpinfo->nturns_idle;
   pplayer->is_alive=3Dpinfo->is_alive;
  =20

--=-K7ZMLasDcN1UOKotfS5+--