From af3fd90ac333bc0192393fea97995ef1b6ee24ba Mon Sep 17 00:00:00 2001
From: Louis Moureaux <m_louis30@yahoo.com>
Date: Mon, 26 Apr 2021 03:12:02 +0200
Subject: [PATCH] Migrate the LAYER_BACKGROUND code to its class

This commit migrates the code to draw the LAYER_BACKGROUND sprites to the
layer_background class. The feature that allowed to hide the terrain was not
migrated; see #449.

Closes #449.
See #430.
---
 client/control.cpp          |  18 -----
 client/control.h            |   2 -
 client/layer.h              |  17 +++++
 client/layer_background.cpp |  69 ++++++++++++++++-
 client/layer_background.h   |  19 ++++-
 client/options.cpp          |   5 --
 client/options.h            |   1 -
 client/packhand.cpp         |   2 -
 client/tilespec.cpp         | 144 ++++++++----------------------------
 client/tilespec.h           |   3 +-
 10 files changed, 130 insertions(+), 150 deletions(-)

diff --git a/client/control.cpp b/client/control.cpp
index f0047e3b3b..6e4298b2eb 100644
--- a/client/control.cpp
+++ b/client/control.cpp
@@ -2365,19 +2365,6 @@ void request_toggle_city_trade_routes()
   update_map_canvas_visible();
 }
 
-/**
-   Toggle display of terrain
- */
-void request_toggle_terrain()
-{
-  if (!can_client_change_view()) {
-    return;
-  }
-
-  gui_options.draw_terrain ^= 1;
-  update_map_canvas_visible();
-}
-
 /**
    Toggle display of coastline
  */
@@ -3692,11 +3679,6 @@ void key_city_productions_toggle() { request_toggle_city_productions(); }
  */
 void key_city_trade_routes_toggle() { request_toggle_city_trade_routes(); }
 
-/**
-   Handle user 'toggle terrain display' input
- */
-void key_terrain_toggle() { request_toggle_terrain(); }
-
 /**
    Handle user 'toggle coastline display' input
  */
diff --git a/client/control.h b/client/control.h
index f7f5ea91f0..1f699ab4b1 100644
--- a/client/control.h
+++ b/client/control.h
@@ -139,7 +139,6 @@ void request_toggle_city_growth();
 void request_toggle_city_productions();
 void request_toggle_city_buycost();
 void request_toggle_city_trade_routes();
-void request_toggle_terrain();
 void request_toggle_coastline();
 void request_toggle_roads_rails();
 void request_toggle_irrigation();
@@ -194,7 +193,6 @@ void key_city_growth_toggle();
 void key_city_productions_toggle();
 void key_city_buycost_toggle();
 void key_city_trade_routes_toggle();
-void key_terrain_toggle();
 void key_coastline_toggle();
 void key_roads_rails_toggle();
 void key_irrigation_toggle();
diff --git a/client/layer.h b/client/layer.h
index 9d4df048b2..ed0450ebc7 100644
--- a/client/layer.h
+++ b/client/layer.h
@@ -16,6 +16,7 @@
 class QPixmap;
 
 struct city;
+struct player;
 struct tileset;
 struct unit;
 struct unit_type;
@@ -137,6 +138,22 @@ class layer {
                     const tile_corner *pcorner, const unit *punit,
                     const city *pcity, const unit_type *putype) const;
 
+  /**
+   * Initializes data specific to one player. This allows to cache tiles
+   * depending on the actual players in a game.
+   *
+   * \see free_player
+   */
+  virtual void initialize_player(const player *player) { Q_UNUSED(player); }
+
+  /**
+   * Frees data initialized by initialize_player. Note that this takes the
+   * player ID and not a pointer to the player.
+   *
+   * \see initialize_player
+   */
+  virtual void free_player(int player_id) { Q_UNUSED(player_id); }
+
   mapview_layer type() const { return m_layer; }
 
 protected:
diff --git a/client/layer_background.cpp b/client/layer_background.cpp
index 938b713f43..ebb87d9a00 100644
--- a/client/layer_background.cpp
+++ b/client/layer_background.cpp
@@ -13,16 +13,81 @@
 
 #include "layer_background.h"
 
+#include "client_main.h"
+#include "colors_common.h"
+#include "control.h" // unit_is_in_focus
+#include "game.h"
+#include "sprite_g.h"
 #include "tilespec.h"
 
 namespace freeciv {
 
+layer_background::layer_background(struct tileset *ts)
+    : freeciv::layer(ts, LAYER_BACKGROUND)
+{
+}
+
 std::vector<drawn_sprite> layer_background::fill_sprite_array(
     const tile *ptile, const tile_edge *pedge, const tile_corner *pcorner,
     const unit *punit, const city *pcity, const unit_type *putype) const
 {
-  return ::fill_sprite_array(tileset(), LAYER_BACKGROUND, ptile, pedge,
-                             pcorner, punit, pcity, putype);
+  // Set up background color.
+  player *owner = nullptr;
+  if (gui_options.solid_color_behind_units) {
+    /* Unit drawing is disabled when the view options are turned off,
+     * but only where we're drawing on the mapview. */
+    bool do_draw_unit =
+        (punit
+         && (gui_options.draw_units || !ptile
+             || (gui_options.draw_focus_unit && unit_is_in_focus(punit))));
+    if (do_draw_unit) {
+      owner = unit_owner(punit);
+    } else if (pcity && gui_options.draw_cities) {
+      owner = city_owner(pcity);
+    }
+  }
+  if (owner) {
+    return {drawn_sprite(tileset(),
+                         m_player_background[player_index(owner)].get())};
+  } else {
+    return {};
+  }
+}
+
+void layer_background::initialize_player(const player *player)
+{
+  // Get the color. In pregame, players have no color so we just use red.
+  const auto color = player_has_color(tileset(), player)
+                         ? *get_player_color(tileset(), player)
+                         : Qt::red;
+
+  auto sprite = create_player_sprite(color);
+
+  const auto id = player_index(player);
+  m_player_background.at(id) = std::unique_ptr<QPixmap>(
+      crop_sprite(sprite.get(), 0, 0, tileset_tile_width(tileset()),
+                  tileset_tile_height(tileset()), get_mask_sprite(tileset()),
+                  0, 0, tileset_scale(tileset()), false));
+}
+
+void layer_background::free_player(int player_id)
+{
+  m_player_background.at(player_id) = nullptr;
+}
+
+/**
+ * Create a sprite with the given color.
+ */
+std::unique_ptr<QPixmap>
+layer_background::create_player_sprite(const QColor &pcolor) const
+{
+  if (tileset_scale(tileset()) == 1.0f) {
+    return std::unique_ptr<QPixmap>(create_sprite(128, 64, &pcolor));
+  } else {
+    return std::unique_ptr<QPixmap>(
+        create_sprite(tileset_full_tile_width(tileset()),
+                      tileset_full_tile_height(tileset()), &pcolor));
+  }
 }
 
 } // namespace freeciv
diff --git a/client/layer_background.h b/client/layer_background.h
index 7dbce136a5..ce6ff77700 100644
--- a/client/layer_background.h
+++ b/client/layer_background.h
@@ -12,20 +12,31 @@
       \____/        ********************************************************/
 #pragma once
 
+#include "fc_types.h"
 #include "layer.h"
 
+#include <QPixmap>
+
 namespace freeciv {
 
 class layer_background : public layer {
 public:
-  layer_background(struct tileset *ts) : freeciv::layer(ts, LAYER_BACKGROUND)
-  {
-  }
+  explicit layer_background(struct tileset *ts);
 
   std::vector<drawn_sprite>
   fill_sprite_array(const tile *ptile, const tile_edge *pedge,
                     const tile_corner *pcorner, const unit *punit,
-                    const city *pcity, const unit_type *putype) const;
+                    const city *pcity,
+                    const unit_type *putype) const override;
+
+  void initialize_player(const player *player) override;
+  void free_player(int player_id) override;
+
+private:
+  std::unique_ptr<QPixmap> create_player_sprite(const QColor &pcolor) const;
+
+  std::array<std::unique_ptr<QPixmap>, MAX_NUM_PLAYER_SLOTS>
+      m_player_background;
 };
 
 } // namespace freeciv
diff --git a/client/options.cpp b/client/options.cpp
index 86a237a9b1..92aa67dd58 100644
--- a/client/options.cpp
+++ b/client/options.cpp
@@ -152,7 +152,6 @@ struct client_options gui_options = {
     true,  //.draw_city_productions =
     false, //.draw_city_buycost =
     false, //.draw_city_trade_routes =
-    true,  //.draw_terrain =
     false, //.draw_coastline =
     true,  //.draw_roads_rails =
     true,  //.draw_irrigation =
@@ -1636,10 +1635,6 @@ static struct client_option client_options[] = {
                        "between cities which have trade routes."),
                     COC_GRAPHICS, GUI_STUB, false,
                     view_option_changed_callback),
-    GEN_BOOL_OPTION(draw_terrain, N_("Draw the terrain"),
-                    N_("Setting this option will draw the terrain."),
-                    COC_GRAPHICS, GUI_STUB, true,
-                    view_option_changed_callback),
     GEN_BOOL_OPTION(
         draw_coastline, N_("Draw the coast line"),
         N_("Setting this option will draw a line to separate the "
diff --git a/client/options.h b/client/options.h
index 4810df6a0f..cc54269297 100644
--- a/client/options.h
+++ b/client/options.h
@@ -125,7 +125,6 @@ struct client_options {
   bool draw_city_productions;
   bool draw_city_buycost;
   bool draw_city_trade_routes;
-  bool draw_terrain;
   bool draw_coastline;
   bool draw_roads_rails;
   bool draw_irrigation;
diff --git a/client/packhand.cpp b/client/packhand.cpp
index ee4d77d9e6..2941f3ce44 100644
--- a/client/packhand.cpp
+++ b/client/packhand.cpp
@@ -4557,8 +4557,6 @@ void handle_ruleset_game(const struct packet_ruleset_game *packet)
   game.plr_bg_color =
       rgbcolor_new(packet->background_red, packet->background_green,
                    packet->background_blue);
-
-  tileset_background_init(tileset);
 }
 
 /**
diff --git a/client/tilespec.cpp b/client/tilespec.cpp
index cd79875ee8..01a484ccd6 100644
--- a/client/tilespec.cpp
+++ b/client/tilespec.cpp
@@ -344,14 +344,9 @@ struct named_sprites {
   struct {
     struct sprite_vector overlays;
   } colors;
-  struct {
-    QPixmap *color;   // Generic background color
-    QPixmap *graphic; // Generic background graphic
-  } background;
   struct {
     QPixmap *grid_borders[EDGE_COUNT][2];
     QPixmap *color;
-    QPixmap *background;
   } player[MAX_NUM_PLAYER_SLOTS];
 
   struct drawing_data *drawing[MAX_NUM_ITEMS];
@@ -1251,9 +1246,6 @@ bool tilespec_reread(const char *new_tileset_name,
   tileset_load_tiles(tileset);
 
   if (game_fully_initialized) {
-    if (game.client.ruleset_ready) {
-      tileset_background_init(tileset);
-    } // else we'll get round to it on PACKET_RULESET_GAME
     players_iterate(pplayer) { tileset_player_init(tileset, pplayer); }
     players_iterate_end;
     boot_help_texts(); // "About Current Tileset"
@@ -2579,25 +2571,6 @@ static QPixmap *load_sprite(struct tileset *t, const QString &tag_name,
   return ss->sprite;
 }
 
-/**
-   Create a sprite with the given color and tag.
- */
-static QPixmap *create_plr_sprite(QColor *pcolor)
-{
-  QPixmap *sprite;
-
-  fc_assert_ret_val(pcolor != NULL, NULL);
-
-  if (tileset->scale == 1.0f) {
-    sprite = create_sprite(128, 64, pcolor);
-  } else {
-    sprite = create_sprite(tileset->full_tile_width,
-                           tileset->full_tile_height, pcolor);
-  }
-
-  return sprite;
-}
-
 /**
    Unloads the sprite. Decrease the reference counter. If the last
    reference is removed the sprite is freed.
@@ -5239,11 +5212,6 @@ static void fill_grid_sprite_array(const struct tileset *t,
     if (mapdeco_is_highlight_set(pedge->tile[0])
         || mapdeco_is_highlight_set(pedge->tile[1])) {
       sprs.emplace_back(t, t->sprites.grid.selected[pedge->type]);
-    } else if (!gui_options.draw_terrain && gui_options.draw_coastline
-               && pedge->tile[0] && pedge->tile[1] && known[0] && known[1]
-               && (is_ocean_tile(pedge->tile[0])
-                   ^ is_ocean_tile(pedge->tile[1]))) {
-      sprs.emplace_back(t, t->sprites.grid.coastline[pedge->type]);
     } else {
       if (gui_options.draw_map_grid) {
         if (worked[0] || worked[1]) {
@@ -5460,7 +5428,6 @@ fill_sprite_array(struct tileset *t, enum mapview_layer layer,
   bv_extras textras;
   struct terrain *tterrain_near[8];
   struct terrain *pterrain = NULL;
-  struct player *owner = NULL;
   /* Unit drawing is disabled when the view options are turned off,
    * but only where we're drawing on the mapview. */
   bool do_draw_unit =
@@ -5468,8 +5435,7 @@ fill_sprite_array(struct tileset *t, enum mapview_layer layer,
        && (gui_options.draw_units || !ptile
            || (gui_options.draw_focus_unit && unit_is_in_focus(punit))));
   bool solid_bg = (gui_options.solid_color_behind_units
-                   && (do_draw_unit || (pcity && gui_options.draw_cities)
-                       || (ptile && !gui_options.draw_terrain)));
+                   && (do_draw_unit || (pcity && gui_options.draw_cities)));
 
   const city *citymode = is_any_city_dialog_open();
 
@@ -5496,42 +5462,29 @@ fill_sprite_array(struct tileset *t, enum mapview_layer layer,
 
   switch (layer) {
   case LAYER_BACKGROUND:
-    // Set up background color.
-    if (gui_options.solid_color_behind_units) {
-      if (do_draw_unit) {
-        owner = unit_owner(punit);
-      } else if (pcity && gui_options.draw_cities) {
-        owner = city_owner(pcity);
-      }
-    }
-    if (owner) {
-      sprs.emplace_back(t,
-                        t->sprites.player[player_index(owner)].background);
-    } else if (ptile && !gui_options.draw_terrain) {
-      sprs.emplace_back(t, t->sprites.background.graphic);
-    }
+    fc_assert_ret_val(false, {});
     break;
 
   case LAYER_TERRAIN1:
-    if (NULL != pterrain && gui_options.draw_terrain && !solid_bg) {
+    if (NULL != pterrain && !solid_bg) {
       fill_terrain_sprite_layer(t, sprs, 0, ptile, pterrain, tterrain_near);
     }
     break;
 
   case LAYER_DARKNESS:
-    if (NULL != pterrain && gui_options.draw_terrain && !solid_bg) {
+    if (NULL != pterrain && !solid_bg) {
       fill_terrain_sprite_darkness(t, sprs, ptile, tterrain_near);
     }
     break;
 
   case LAYER_TERRAIN2:
-    if (NULL != pterrain && gui_options.draw_terrain && !solid_bg) {
+    if (NULL != pterrain && !solid_bg) {
       fill_terrain_sprite_layer(t, sprs, 1, ptile, pterrain, tterrain_near);
     }
     break;
 
   case LAYER_TERRAIN3:
-    if (NULL != pterrain && gui_options.draw_terrain && !solid_bg) {
+    if (NULL != pterrain && !solid_bg) {
       fc_assert(MAX_NUM_LAYERS == 3);
       fill_terrain_sprite_layer(t, sprs, 2, ptile, pterrain, tterrain_near);
     }
@@ -5539,8 +5492,7 @@ fill_sprite_array(struct tileset *t, enum mapview_layer layer,
 
   case LAYER_WATER:
     if (NULL != pterrain) {
-      if (gui_options.draw_terrain && !solid_bg
-          && terrain_type_terrain_class(pterrain) == TC_OCEAN) {
+      if (!solid_bg && terrain_type_terrain_class(pterrain) == TC_OCEAN) {
         for (dir = 0; dir < t->num_cardinal_tileset_dirs; dir++) {
           int didx = t->cardinal_tileset_dirs[dir];
 
@@ -5559,7 +5511,7 @@ fill_sprite_array(struct tileset *t, enum mapview_layer layer,
 
       fill_irrigation_sprite_array(t, sprs, textras, textras_near, pcity);
 
-      if (gui_options.draw_terrain && !solid_bg) {
+      if (!solid_bg) {
         extra_type_list_iterate(t->style_lists[ESTYLE_RIVER], priver)
         {
           int idx = extra_index(priver);
@@ -6262,8 +6214,6 @@ void tileset_free_tiles(struct tileset *t)
   sprite_vector_free(&t->sprites.nation_flag);
   sprite_vector_free(&t->sprites.nation_shield);
   sprite_vector_free(&t->sprites.citybar.occupancy);
-
-  tileset_background_free(t);
 }
 
 /**
@@ -6434,6 +6384,14 @@ QPixmap *get_event_sprite(const struct tileset *t, enum event_type event)
   return t->sprites.events[event];
 }
 
+/**
+ * Return tile mask sprite
+ */
+QPixmap *get_mask_sprite(const struct tileset *t)
+{
+  return t->sprites.mask.tile;
+}
+
 /**
    Return a thumbs-up/thumbs-down sprite to show treaty approval or
    disapproval.
@@ -6606,9 +6564,6 @@ void tileset_init(struct tileset *t)
 
   t->sprites.city.occupied = NULL;
 
-  t->sprites.background.color = NULL;
-  t->sprites.background.graphic = NULL;
-
   player_slots_iterate(pslot)
   {
     int edge, j, id = player_slot_index(pslot);
@@ -6620,7 +6575,6 @@ void tileset_init(struct tileset *t)
     }
 
     t->sprites.player[id].color = NULL;
-    t->sprites.player[id].background = NULL;
   }
   player_slots_iterate_end;
 
@@ -6848,22 +6802,18 @@ void tileset_player_init(struct tileset *t, struct player *pplayer)
   // Free all data before recreating it.
   tileset_player_free(t, plrid);
 
-  if (player_has_color(t, pplayer)) {
-    t->sprites.player[plrid].color = color =
-        create_plr_sprite(get_player_color(t, pplayer));
-  } else {
-    /* XXX: if player hasn't been assigned a color, perhaps there's no
-     * point proceeding with an arbitrary color; this should only happen
-     * in pregame. Probably blank sprites would be better. */
-
-    fc_assert_ret(t->sprites.background.color != NULL);
-
-    color = t->sprites.background.color;
+  for (auto &&layer : t->layers) {
+    layer->initialize_player(pplayer);
   }
 
-  t->sprites.player[plrid].background =
-      crop_sprite(color, 0, 0, t->normal_tile_width, t->normal_tile_height,
-                  t->sprites.mask.tile, 0, 0, t->scale, false);
+  QColor c = Qt::black;
+  if (player_has_color(t, pplayer)) {
+    c = *get_player_color(t, pplayer);
+  }
+  color = tileset_scale(t) == 1.0f
+              ? create_sprite(128, 64, &c)
+              : create_sprite(tileset_full_tile_width(t),
+                              tileset_full_tile_height(t), &c);
 
   for (i = 0; i < EDGE_COUNT; i++) {
     for (j = 0; j < 2; j++) {
@@ -6891,14 +6841,14 @@ static void tileset_player_free(struct tileset *t, int plrid)
   fc_assert_ret(plrid >= 0);
   fc_assert_ret(plrid < ARRAY_SIZE(t->sprites.player));
 
+  for (auto &&layer : t->layers) {
+    layer->free_player(plrid);
+  }
+
   if (t->sprites.player[plrid].color) {
     free_sprite(t->sprites.player[plrid].color);
     t->sprites.player[plrid].color = NULL;
   }
-  if (t->sprites.player[plrid].background) {
-    free_sprite(t->sprites.player[plrid].background);
-    t->sprites.player[plrid].background = NULL;
-  }
 
   for (i = 0; i < EDGE_COUNT; i++) {
     for (j = 0; j < 2; j++) {
@@ -6910,40 +6860,6 @@ static void tileset_player_free(struct tileset *t, int plrid)
   }
 }
 
-/**
-   Setup tiles for the background.
- */
-void tileset_background_init(struct tileset *t)
-{
-  // Free all data before recreating it.
-  tileset_background_free(t);
-
-  // generate background color
-  t->sprites.background.color =
-      create_plr_sprite(ensure_color(game.plr_bg_color));
-
-  // Chop up and build the background graphics.
-  t->sprites.background.graphic = crop_sprite(
-      t->sprites.background.color, 0, 0, t->normal_tile_width,
-      t->normal_tile_height, t->sprites.mask.tile, 0, 0, t->scale, false);
-}
-
-/**
-   Free tiles for the background.
- */
-void tileset_background_free(struct tileset *t)
-{
-  if (t->sprites.background.color) {
-    free_sprite(t->sprites.background.color);
-    t->sprites.background.color = NULL;
-  }
-
-  if (t->sprites.background.graphic) {
-    free_sprite(t->sprites.background.graphic);
-    t->sprites.background.graphic = NULL;
-  }
-}
-
 /**
    Reset tileset data specific to ruleset.
  */
diff --git a/client/tilespec.h b/client/tilespec.h
index 4db7a02ed8..876ecb6e9c 100644
--- a/client/tilespec.h
+++ b/client/tilespec.h
@@ -119,8 +119,6 @@ void tileset_setup_nation_flag(struct tileset *t,
 void tileset_setup_city_tiles(struct tileset *t, int style);
 
 void tileset_player_init(struct tileset *t, struct player *pplayer);
-void tileset_background_init(struct tileset *t);
-void tileset_background_free(struct tileset *t);
 
 // Layer order
 const std::vector<std::unique_ptr<freeciv::layer>> &
@@ -241,6 +239,7 @@ std::vector<drawn_sprite>
 fill_basic_extra_sprite_array(const struct tileset *t,
                               const struct extra_type *pextra);
 QPixmap *get_event_sprite(const struct tileset *t, enum event_type event);
+QPixmap *get_mask_sprite(const struct tileset *t);
 
 QPixmap *tiles_lookup_sprite_tag_alt(struct tileset *t, QtMsgType level,
                                      const char *tag, const char *alt,