disobedience/control.c: Handle state-change toggles better.

If you click a toggle icon, say the `pause' button, then: Gtk
immediately toggles the state of the icon, but doesn't change the
corresponding menu item.  Instead, a state change is reported through
the log connection which causes us to update the menu (and the icon,
though that's already in the right state so you don't notice).  (Of
course, this just works the other way around if you use the menu item or
keyboard accelerator instead.)

There are a couple of problems with this, with more or less the same
fix.

  * If the connection to the server is very slow, then there's a lag
    between clicking the icon, or pressing the accelerator, and the rest
    of the UI updating.  This is unfortunate.

  * If something goes wrong, then the icon is left in the wrong state,
    and nothing will correct it.

The fix is as follows:

  * When toggling the state of something, we immediately (a) adjust our
    internal idea of what the state /ought/ to be, and (b) force an
    update of the icon (and associated menu item).  This solves the
    problem of the associated UI element lagging.

  * On a server error, we immediately change the state back again and
    force another icon update.

  * That leaves the RTP connection option (which was my original
    motivation for all of this).  There's no server feedback for this
    control.  Instead, we periodically check whether the RTP player
    process is listening for connections.  An event is signalled if this
    state /changes/, and this event is used to set the icon and
    menu-item state.  So, again, if we force our internal idea of the
    RTP process state to match the icon, but the player doesn't start up
    properly, then the next time we check, we find that the player isn't
    responding, signal the event, and then we fix the icon state.

diff --git a/disobedience/control.c b/disobedience/control.c
index 40f347ada7b7c3200b2c56edf6bd0949db4bcbbb..9153f33b1f5dfc55b0f20148a9ecc5411e1ec87c 100644 (file)
--- a/disobedience/control.c
+++ b/disobedience/control.c
@@ -31,6 +31,24 @@ static void toggled_icon(GtkToggleToolButton *button,
 static void clicked_menu(GtkMenuItem *, gpointer userdata);
 static void toggled_menu(GtkCheckMenuItem *, gpointer userdata);
 
+static int enable_playing(disorder_eclient *c,
+                          disorder_eclient_no_response *completed,
+                          void *v);
+static int disable_playing(disorder_eclient *c,
+                           disorder_eclient_no_response *completed,
+                           void *v);
+static int enable_random(disorder_eclient *c,
+                         disorder_eclient_no_response *completed,
+                         void *v);
+static int disable_random(disorder_eclient *c,
+                          disorder_eclient_no_response *completed,
+                          void *v);
+static int pause_track(disorder_eclient *c,
+                       disorder_eclient_no_response *completed,
+                       void *v);
+static int resume_track(disorder_eclient *c,
+                        disorder_eclient_no_response *completed,
+                        void *v);
 static int enable_rtp(disorder_eclient *c,
                       disorder_eclient_no_response *completed,
                       void *v);
@@ -65,6 +83,20 @@ int suppress_actions = 1;
 /** @brief Toolbar widget */
 static GtkWidget *toolbar;
 
+/** @brief Old state to restore on error
+ *
+ * Here's the problem we're trying to solve.  Suppose the user clicks the
+ * `pause' button.  Immediately, Gtk toggles the button's state.  There's also
+ * a menu item which is @i not yet updated, so we hack @c last_state and call
+ * icon_changed() by hand.  We send a `pause' command to the server.  If all is
+ * well, then we're done: we'll get a state update in the server log which will
+ * match what we're already doing and everything is fine.  If there's an error,
+ * though, we must put @c last_state back the way we found it since there will
+ * be no change reported in the server log to correct the wrong state
+ * information.  Also, we must untoggle the icon and menu items.
+ */
+static unsigned long restore_state;
+
 /** @brief Definition of an icon
  *
  * We have two kinds of icon:
@@ -192,8 +224,8 @@ static struct icon icons[] = {
     .menuitem = "<GdisorderMain>/Control/Playing",
     .on = pause_resume_on,
     .sensitive = pause_resume_sensitive,
-    .action_go_on = disorder_eclient_pause,
-    .action_go_off = disorder_eclient_resume,
+    .action_go_on = pause_track,
+    .action_go_off = resume_track,
     .events = "pause-changed playing-changed rights-changed playing-track-changed",
     .menu_invert = TRUE,
   },
@@ -217,8 +249,8 @@ static struct icon icons[] = {
     .menuitem = "<GdisorderMain>/Control/Random play",
     .on = random_enabled,
     .sensitive = random_sensitive,
-    .action_go_on = disorder_eclient_random_enable,
-    .action_go_off = disorder_eclient_random_disable,
+    .action_go_on = enable_random,
+    .action_go_off = disable_random,
     .events = "random-changed rights-changed",
   },
   {
@@ -230,8 +262,8 @@ static struct icon icons[] = {
     .tip_off = "Enable play",
     .on = playing_enabled,
     .sensitive = playing_sensitive,
-    .action_go_on = disorder_eclient_enable,
-    .action_go_off = disorder_eclient_disable,
+    .action_go_on = enable_playing,
+    .action_go_off = disable_playing,
     .events = "enabled-changed rights-changed",
   },
   {
@@ -428,10 +460,12 @@ static void icon_changed(const char attribute((unused)) *event,
   --suppress_actions;
 }
 
-static void icon_action_completed(void attribute((unused)) *v,
-                                  const char *err) {
-  if(err)
+static void icon_action_completed(void *v, const char *err) {
+  if(err) {
+    last_state = restore_state;
     popup_protocol_error(0, err);
+    icon_changed(0, 0, v);
+  }
 }
 
 static void clicked_icon(GtkToolButton attribute((unused)) *button,
@@ -449,10 +483,12 @@ static void toggled_icon(GtkToggleToolButton attribute((unused)) *button,
 
   if(suppress_actions)
     return;
+  restore_state = last_state;
   if(icon->on())
     icon->action_go_off(client, icon_action_completed, 0);
   else
     icon->action_go_on(client, icon_action_completed, 0);
+  icon_changed(0, 0, user_data);
 }
 
 static void clicked_menu(GtkMenuItem attribute((unused)) *menuitem,
@@ -583,6 +619,48 @@ static double balance(double l, double r) {
     return 0;
 }
 
+/** @brief Called to enable jukebox playback */
+static int enable_playing(disorder_eclient *c,
+                          disorder_eclient_no_response *completed, void *v) {
+  last_state |= DISORDER_PLAYING;
+  return disorder_eclient_enable(c, completed, v);
+}
+
+/** @brief Called to disable jukebox playback */
+static int disable_playing(disorder_eclient *c,
+                           disorder_eclient_no_response *completed, void *v) {
+  last_state &= ~DISORDER_PLAYING;
+  return disorder_eclient_disable(c, completed, v);
+}
+
+/** @brief Called to enable random selection */
+static int enable_random(disorder_eclient *c,
+                         disorder_eclient_no_response *completed, void *v) {
+  last_state |= DISORDER_RANDOM_ENABLED;
+  return disorder_eclient_random_enable(c, completed, v);
+}
+
+/** @brief Called to disable random selection */
+static int disable_random(disorder_eclient *c,
+                          disorder_eclient_no_response *completed, void *v) {
+  last_state &= ~DISORDER_RANDOM_ENABLED;
+  return disorder_eclient_random_disable(c, completed, v);
+}
+
+/** @brief Called to pause the current track */
+static int pause_track(disorder_eclient *c,
+                       disorder_eclient_no_response *completed, void *v) {
+  last_state |= DISORDER_TRACK_PAUSED;
+  return disorder_eclient_pause(c, completed, v);
+}
+
+/** @brief Called to resume the current track */
+static int resume_track(disorder_eclient *c,
+                        disorder_eclient_no_response *completed, void *v) {
+  last_state &= ~DISORDER_TRACK_PAUSED;
+  return disorder_eclient_resume(c, completed, v);
+}
+
 /** @brief Called to enable RTP play
  *
  * Rather odd signature is to fit in with the other icons which all call @ref
@@ -592,6 +670,7 @@ static int enable_rtp(disorder_eclient attribute((unused)) *c,
                       disorder_eclient_no_response attribute((unused)) *completed,
                       void attribute((unused)) *v) {
   start_rtp();
+  rtp_is_running = 1;
   return 0;
 }
 
@@ -604,6 +683,7 @@ static int disable_rtp(disorder_eclient attribute((unused)) *c,
                        disorder_eclient_no_response attribute((unused)) *completed,
                        void attribute((unused)) *v) {
   stop_rtp();
+  rtp_is_running = 0;
   return 0;
 }