[iagno] Improve readability, again.

[Arnaud B. <arnaudb src gnome org>]

commit e8f0adcb4819b2e10198a98cfcdc0e54ce112915
Author: Arnaud Bonatti <arnaud bonatti gmail com>
Date:   Sun Apr 14 18:43:27 2019 +0200

    Improve readability, again.

 src/computer-player.vala | 37 +++++++++++++++++++++++--------------
 src/game.vala            |  7 +++++--
 src/iagno.vala           |  4 +++-
 src/test-iagno.vala      | 12 +++++-------
 4 files changed, 36 insertions(+), 24 deletions(-)
---
diff --git a/src/computer-player.vala b/src/computer-player.vala
index 412f2df..2a54812 100644
--- a/src/computer-player.vala
+++ b/src/computer-player.vala
@@ -221,8 +221,8 @@ private class ComputerPlayer : Object
         /* The -1 is because the search sometimes returns NEGATIVE_INFINITY. */
         int a = NEGATIVE_INFINITY - 1;
 
-        List<PossibleMove?> moves = new List<PossibleMove?> ();
-        get_possible_moves_sorted (g, ref moves);
+        List<PossibleMove?> moves;
+        get_possible_moves_sorted (g, out moves);
 
         /* Try each move using alpha-beta pruning to optimise finding the best branch */
         foreach (PossibleMove? move in moves)
@@ -261,8 +261,8 @@ private class ComputerPlayer : Object
 
         if (g.current_player_can_move)
         {
-            List<PossibleMove?> moves = new List<PossibleMove?> ();
-            get_possible_moves_sorted (g, ref moves);
+            List<PossibleMove?> moves;
+            get_possible_moves_sorted (g, out moves);
 
             /* Try each move using alpha-beta pruning to optimise finding the best branch */
             foreach (PossibleMove? move in moves)
@@ -293,26 +293,33 @@ private class ComputerPlayer : Object
         return a;
     }
 
-    private static void get_possible_moves_sorted (GameState g, ref List<PossibleMove?> moves)
+    private static void get_possible_moves_sorted (GameState g, out List<PossibleMove?> moves)
     {
-        for (uint8 x = 0; x < g.size; x++)
+        uint8 size = g.size;
+        moves = new List<PossibleMove?> ();
+
+        for (uint8 x = 0; x < size; x++)
         {
-            for (uint8 y = 0; y < g.size; y++)
+            for (uint8 y = 0; y < size; y++)
             {
                 uint8 n_tiles = g.test_placing_tile (x, y);
                 if (n_tiles == 0)
                     continue;
 
                 PossibleMove move = PossibleMove (x, y, n_tiles);
+                // the g_list_insert_sorted() documentation says: "if you are adding many new elements to
+                // a list, and the number of new elements is much larger than the length of the list, use
+                // g_list_prepend() to add the new items and sort the list afterwards with g_list_sort()"
+                // but the perfs tests on complete games disagree; so let's keep things like that for now
                 moves.insert_sorted (move, compare_move);
             }
         }
     }
 
     private static int compare_move (PossibleMove? a, PossibleMove? b)
+        requires (a != null)
+        requires (b != null)
     {
-        if (a == null || b == null)
-            assert_not_reached ();
         return ((!) b).n_tiles - ((!) a).n_tiles;
     }
 
@@ -338,13 +345,14 @@ private class ComputerPlayer : Object
 
     private static int eval_heuristic (GameState g)
     {
-        if (g.size != 8)     // TODO
+        uint8 size = g.size;
+        if (size != 8)     // TODO
             return 0;
 
         int count = 0;
-        for (uint8 x = 0; x < g.size; x++)
+        for (uint8 x = 0; x < size; x++)
         {
-            for (uint8 y = 0; y < g.size; y++)
+            for (uint8 y = 0; y < size; y++)
             {
                 int h = heuristic [x, y];
                 if (g.get_owner (x, y) != g.current_color)
@@ -358,9 +366,10 @@ private class ComputerPlayer : Object
     private static int around (GameState g)
     {
         int count = 0;
-        for (int8 x = 0; x < (int8) g.size; x++)
+        int8 size = (int8) g.size;
+        for (int8 x = 0; x < size; x++)
         {
-            for (int8 y = 0; y < (int8) g.size; y++)
+            for (int8 y = 0; y < size; y++)
             {
                 int a = 0;
                 a -= is_empty (g, x + 1, y    );
diff --git a/src/game.vala b/src/game.vala
index 683e635..5174dfa 100644
--- a/src/game.vala
+++ b/src/game.vala
@@ -441,12 +441,15 @@ private class Game : Object
         return true;
     }
 
-    internal void pass ()
-        requires (!current_player_can_move)
+    internal /* success */ bool pass ()
     {
+        if (current_player_can_move)
+            return false;
+
         current_state = new GameState.copy_and_pass (current_state);
         undo_stack.append (current_state);
         end_of_turn (/* undoing */ false, /* no_draw */ true);
+        return true;
     }
 
     private void end_of_turn (bool undoing, bool no_draw)
diff --git a/src/iagno.vala b/src/iagno.vala
index b143fd9..844dccd 100644
--- a/src/iagno.vala
+++ b/src/iagno.vala
@@ -473,7 +473,9 @@ private class Iagno : Gtk.Application
         play_sound (Sound.FLIP);
         view.update_scoreboard ();
 
-        game.pass ();
+        if (!game.pass ())
+            assert_not_reached ();
+
         if (game.current_color == Player.DARK)
         {
             /* Translators: during a game, notification to display when Light has no possible moves */
diff --git a/src/test-iagno.vala b/src/test-iagno.vala
index 4a3bc4b..8daa160 100644
--- a/src/test-iagno.vala
+++ b/src/test-iagno.vala
@@ -93,8 +93,7 @@ private class TestIagno : Object
         assert_true (game.number_of_moves == 0);
         assert_true (game.place_tile (7, 2));
         assert_true (game.number_of_moves == 1);
-        assert_true (!game.current_player_can_move);
-        game.pass ();
+        assert_true (game.pass ());
         assert_true (game.number_of_moves == 2);
         game.undo (2);
         assert_true (game.number_of_moves == 0);
@@ -131,8 +130,7 @@ private class TestIagno : Object
         assert_true (game.current_color == Player.DARK);
         assert_true (game.place_tile (1, 2));
         assert_true (game.current_color == Player.LIGHT);
-        assert_true (!game.current_player_can_move);
-        game.pass ();
+        assert_true (game.pass ());
         assert_true (game.current_color == Player.DARK);
     }
 
@@ -504,13 +502,13 @@ private class TestIagno : Object
         assert_true (game.place_tile (1, 0));
         assert_true (ai.force_moving (1, 6));
         assert_true (game.place_tile (2, 1));
-        game.pass ();
+        assert_true (game.pass ());
         assert_true (game.place_tile (0, 7));
-        game.pass ();
+        assert_true (game.pass ());
         assert_true (game.place_tile (0, 1));
         assert_true (ai.force_moving (1, 1));
         assert_true (game.place_tile (0, 0));
-        game.pass ();
+        assert_true (game.pass ());
         assert_true (game.place_tile (4, 7));
         assert_true (ai.force_moving (5, 7));
     }