[gnome-chess] Make ChessClock class harder to misuse

[Michael Catanzaro <mcatanzaro src gnome org>]

commit 8ee7af6608c13a352eda3c87baab538c03e36ced
Author: Michael Catanzaro <mike catanzaro gmail com>
Date:   Thu Aug 1 11:12:49 2013 -0500

    Make ChessClock class harder to misuse
    
    This class has never really been safe.  Pay off some technical debt.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=701578

 src/chess-clock.vala |  146 ++++++++++++++++++++++++-------------------------
 src/gnome-chess.vala |   24 +++++---
 2 files changed, 87 insertions(+), 83 deletions(-)
---
diff --git a/src/chess-clock.vala b/src/chess-clock.vala
index 6a37805..3fd4653 100644
--- a/src/chess-clock.vala
+++ b/src/chess-clock.vala
@@ -10,55 +10,46 @@
 
 public class ChessClock : Object
 {
-    private uint _white_duration;
-    public uint white_duration
+    private uint _white_initial_ms;
+    public uint white_initial_seconds
     {
-        get { return _white_duration; }
+        get { return ms_to_seconds (_white_initial_ms); }
     }
-    private uint _black_duration;
-    public uint black_duration
+
+    private uint _black_initial_ms;
+    public uint black_initial_seconds
     {
-        get { return _black_duration; }
+        get { return ms_to_seconds (_black_initial_ms); }
     }
 
-    private uint _white_used;
-    public uint white_used
+    private uint _white_ms_used = 0;
+    public uint white_seconds_used
     {
         get
         {
             if (timer == null)
                 return 0;
             else if (active_color == Color.WHITE)
-                return _white_used + (uint) (timer.elapsed () * 1000);
+                return ms_to_seconds (_white_ms_used + (uint) (timer.elapsed () * 1000));
             else
-                return _white_used;
+                return ms_to_seconds (_white_ms_used);
         }
     }
 
-    public uint white_used_in_seconds
-    {
-        get { return (white_used + 500) / 1000; }
-    }
-
-    private uint _black_used;
-    public uint black_used
+    private uint _black_ms_used = 0;
+    public uint black_seconds_used
     {
         get
         {
             if (timer == null)
                 return 0;
             else if (active_color == Color.WHITE)
-                return _black_used;
+                return ms_to_seconds (_black_ms_used);
             else
-                return _black_used + (uint) (timer.elapsed () * 1000);
+                return ms_to_seconds (_black_ms_used + (uint) (timer.elapsed () * 1000));
         }
     }
 
-    public uint black_used_in_seconds
-    {
-        get { return (black_used + 500) / 1000; }
-    }
-
     private Color _active_color = Color.WHITE;
     public Color active_color
     {
@@ -75,23 +66,21 @@ public class ChessClock : Object
     }
 
     private Timer? timer;
-    private uint expire_timeout = 0;
-    private uint tick_timeout = 0;
+    private uint expire_timeout_id = 0;
+    private uint tick_timeout_id = 0;
 
     public signal void tick ();
     public signal void expired ();
 
-    public ChessClock (uint white_duration, uint black_duration, uint white_used = 0, uint black_used = 0)
+    public ChessClock (uint white_initial_seconds, uint black_initial_seconds)
     {
-        _white_duration = white_duration * 1000;
-        _black_duration = black_duration * 1000;
-        _white_used = white_used;
-        _black_used = black_used;
+        _white_initial_ms = white_initial_seconds * 1000;
+        _black_initial_ms = black_initial_seconds * 1000;
     }
-    
+
     private bool is_started
     {
-        get { return expire_timeout != 0; }
+        get { return expire_timeout_id != 0; }
     }
     
     public void start ()
@@ -105,16 +94,11 @@ public class ChessClock : Object
             timer = new Timer ();
         }
         else
+        {
             timer.start ();
+        }
 
-        /* Notify when this timer has expired */
-        if (active_color == Color.WHITE)
-            expire_timeout = Timeout.add (white_duration - _white_used, timer_expired_cb);
-        else
-            expire_timeout = Timeout.add (black_duration - _black_used, timer_expired_cb);
-
-        /* Wake up each second */
-        tick_cb ();
+        watch_timer ();
     }
 
     private bool timer_expired_cb ()
@@ -126,17 +110,17 @@ public class ChessClock : Object
 
     private bool tick_cb ()
     {
-        if (tick_timeout != 0)
+        if (tick_timeout_id != 0)
             tick ();
 
         uint elapsed = (uint) (timer.elapsed () * 1000);
         uint used;
         if (active_color == Color.WHITE)
-            used = _white_used + elapsed;
+            used = _white_ms_used + elapsed;
         else
-            used = _black_used + elapsed;
+            used = _black_ms_used + elapsed;
         var next_tick_time = ((used / 1000) + 1) * 1000;
-        tick_timeout = Timeout.add (next_tick_time - used, tick_cb);
+        tick_timeout_id = Timeout.add (next_tick_time - used, tick_cb);
 
         return false;
     }
@@ -147,51 +131,65 @@ public class ChessClock : Object
             return;
 
         timer.stop ();
-        Source.remove (expire_timeout);
-        expire_timeout = 0;
-        Source.remove (tick_timeout);
-        tick_timeout = 0;
+        stop_checking_timer ();
 
         var elapsed = (uint) (timer.elapsed () * 1000);
         if (active_color == Color.WHITE)
         {
-            _white_used += elapsed;
-            if (_white_used > white_duration)
-                _white_used = white_duration;
+            _white_ms_used += elapsed;
+            if (_white_ms_used > _white_initial_ms)
+                _white_ms_used = _white_initial_ms;
         }
         else
         {
-            _black_used += elapsed;
-            if (_black_used > black_duration)
-                _black_used = black_duration;
+            _black_ms_used += elapsed;
+            if (_black_ms_used > _black_initial_ms)
+                _black_ms_used = _black_initial_ms;
         }
 
         timer.reset ();
     }
 
-    public void toggle_paused (bool paused)
+    public void pause ()
     {
         if (timer == null)
             return;
 
-        if (paused)
-        {
-            timer.stop ();
-            Source.remove (expire_timeout);
-            expire_timeout = 0;
-            Source.remove (tick_timeout);
-            tick_timeout = 0;
-        }
+        timer  continue ();
+        watch_timer ();
+    }
+
+    public void unpause ()
+    {
+        if (timer == null)
+            return;
+
+        timer.stop ();
+        stop_checking_timer ();
+    }
+
+    private void watch_timer ()
+    {
+        /* Notify when this timer has expired */
+        if (active_color == Color.WHITE)
+            expire_timeout_id = Timeout.add (_white_initial_ms - _white_ms_used, timer_expired_cb);
         else
-        {
-            timer  continue ();
-            if (active_color == Color.WHITE)
-                expire_timeout = Timeout.add (white_duration - _white_used,
-                                              timer_expired_cb);
-            else
-                expire_timeout = Timeout.add (black_duration - _black_used,
-                                              timer_expired_cb);
-            tick_cb ();
-        }
+            expire_timeout_id = Timeout.add (_black_initial_ms - _black_ms_used, timer_expired_cb);
+
+        /* Wake up each second */
+        tick_cb ();
+    }
+
+    private void stop_checking_timer ()
+    {
+        Source.remove (expire_timeout_id);
+        expire_timeout_id = 0;
+        Source.remove (tick_timeout_id);
+        tick_timeout_id = 0;
+    }
+
+    private uint ms_to_seconds (uint ms)
+    {
+        return (ms + 500) / 1000;
     }
 }
diff --git a/src/gnome-chess.vala b/src/gnome-chess.vala
index 079a20d..3af2869 100644
--- a/src/gnome-chess.vala
+++ b/src/gnome-chess.vala
@@ -1228,12 +1228,18 @@ public class Application : Gtk.Application
 
     public void set_paused_state (bool paused)
     {
-        if (paused != is_paused)
+        if (paused == is_paused)
+            return;
+
+        is_paused = paused;
+        game.is_paused = paused;
+
+        if (game.clock != null)
         {
-            is_paused = paused;
-            game.is_paused = paused;
-            if (game.clock != null)
-                game.clock.toggle_paused (paused);
+            if (paused)
+                game.clock.unpause ();
+            else
+                game.clock.pause ();
         }
     }
 
@@ -1269,9 +1275,9 @@ public class Application : Gtk.Application
 
         int used;
         if (color == Color.WHITE)
-            used = (int) (game.clock.white_duration / 1000 - game.clock.white_used_in_seconds);
+            used = (int) (game.clock.white_initial_seconds - game.clock.white_seconds_used);
         else
-            used = (int) (game.clock.black_duration / 1000 - game.clock.black_used_in_seconds);
+            used = (int) (game.clock.black_initial_seconds - game.clock.black_seconds_used);
 
         if (used >= 60)
             return "%d:%02d".printf (used / 60, used % 60);
@@ -1823,8 +1829,8 @@ public class Application : Gtk.Application
         {
             /* We currently only support simple timeouts */
             uint initial_time = int.parse (pgn_game.time_control);
-            uint white_used = game.clock.white_used_in_seconds;
-            uint black_used = game.clock.black_used_in_seconds;
+            uint white_used = game.clock.white_seconds_used;
+            uint black_used = game.clock.black_seconds_used;
 
             pgn_game.white_time_left = (initial_time - white_used).to_string ();
             pgn_game.black_time_left = (initial_time - black_used).to_string ();