unit cleanup

client/src/city_ui.rs

@@ -108,7 +108,7 @@ fn building_icons<'a>(rc: &'a RenderContext, city: &'a City) -> IconActionVec<'a
     let owner = rc.shown_player;
     Building::all()
         .iter()
-        .filter_map(|(b)| {
+        .filter_map(|b| {
             if city.can_construct(*b, owner, rc.game) {
                 Some(*b)
             } else {
@@ -218,11 +218,7 @@ pub fn city_labels(game: &Game, city: &City) -> Vec<String> {
                     if city.player_index == *o {
                         b.name().to_string()
                     } else {
-                        format!(
-                            "{} (owned by {})",
-                            b.name(),
-                            game.get_player(*o).get_name()
-                        )
+                        format!("{} (owned by {})", b.name(), game.get_player(*o).get_name())
                     }
                 })
             })
@@ -322,4 +318,3 @@ pub fn building_position(city: &City, center: Point, i: usize, building: Buildin
         hex_ui::rotate_around(center, 25.0, 90 * i)
     }
 }
-

client/src/influence_ui.rs

@@ -1,4 +1,4 @@
-use crate::city_ui::{ building_position, BUILDING_SIZE};
+use crate::city_ui::{building_position, BUILDING_SIZE};
 use crate::client_state::{CameraMode, StateUpdate};
 use crate::dialog_ui::{cancel_button_with_tooltip, ok_button, OkTooltip};
 use crate::hex_ui;

client/src/local_client/bin/main.rs

@@ -80,7 +80,7 @@ pub fn setup_local_game() -> Game {
             vec![unit_type],
             Position::from_offset(pos),
             None,
-            vec![],
+            &[],
         );
     };
 

server/src/game.rs

@@ -1092,7 +1092,7 @@ impl Game {
         units: Vec<UnitType>,
         city_position: Position,
         leader_name: Option<&String>,
-        replaced_units: Vec<u32>,
+        replaced_units: &[u32],
     ) {
         let mut replaced_leader = None;
         if let Some(leader_name) = leader_name {
@@ -1110,7 +1110,7 @@ impl Game {
             self.set_active_leader(leader_name.clone(), player_index);
         }
         let mut replaced_units_undo_context = Vec::new();
-        for unit in &replaced_units {
+        for unit in replaced_units {
             let player = &mut self.players[player_index];
             let u = player.remove_unit(*unit);
             if u.carrier_id.is_some_and(|c| replaced_units.contains(&c)) {

server/src/player.rs

@@ -265,7 +265,7 @@ impl Player {
             // carried units are added to carriers
             .filter(|unit| unit.carrier_id.is_none())
             .sorted_by_key(|unit| unit.id)
-            .map(|u| u.data(&self))
+            .map(|u| u.data(self))
             .collect();
         PlayerData {
             name: self.name.clone(),
@@ -978,8 +978,8 @@ impl Player {
         self.units.iter_mut().find(|unit| unit.id == id)
     }
 
-    pub fn remove_unit(&mut self, id: u32) -> Unit {
-        for id in carried_units(id, &self) {
+    pub(crate) fn remove_unit(&mut self, id: u32) -> Unit {
+        for id in carried_units(id, self) {
             self.remove_unit(id);
         }
 

server/src/playing_actions.rs

@@ -7,6 +7,7 @@ use crate::city::MoodState;
 use crate::collect::{collect, undo_collect};
 use crate::game::{CulturalInfluenceResolution, GameState};
 use crate::payment::PaymentModel;
+use crate::unit::Unit;
 use crate::{
     city::City,
     city_pieces::Building::{self, *},
@@ -16,7 +17,6 @@ use crate::{
     resource_pile::ResourcePile,
     unit::UnitType,
 };
-use crate::unit::Unit;
 
 #[derive(Serialize, Deserialize, PartialEq, Eq, Clone)]
 pub struct Construct {
@@ -132,8 +132,7 @@ impl PlayingAction {
                 game.advance(&advance, player_index);
             }
             FoundCity { settler } => {
-                let settler = game.players[player_index]
-                    .remove_unit(settler);
+                let settler = game.players[player_index].remove_unit(settler);
                 assert!(settler.can_found_city(game), "Illegal action");
                 let player = &mut game.players[player_index];
                 let city = City::new(player_index, settler.position);
@@ -204,7 +203,7 @@ impl PlayingAction {
                     r.units,
                     r.city_position,
                     r.leader_name.as_ref(),
-                    r.replaced_units,
+                    &r.replaced_units,
                 );
             }
             IncreaseHappiness(i) => {
@@ -331,9 +330,12 @@ impl PlayingAction {
                 };
                 let player = &mut game.players[player_index];
                 let units = Unit::from_data(player_index, settler);
-                player.units.push(units.into_iter().next().expect(
-                    "The player should have a unit after founding a city",
-                ));
+                player.units.push(
+                    units
+                        .into_iter()
+                        .next()
+                        .expect("The player should have a unit after founding a city"),
+                );
                 player
                     .cities
                     .pop()

server/src/unit.rs

@@ -13,6 +13,7 @@ use crate::game::CurrentMove;
 use crate::player::Player;
 use crate::{game::Game, map::Terrain::*, position::Position, resource_pile::ResourcePile, utils};
 
+#[derive(Clone)]
 pub struct Unit {
     pub player_index: usize,
     pub position: Position,
@@ -88,7 +89,7 @@ impl Unit {
     }
 
     #[must_use]
-    pub fn data(&self, player: &Player) -> UnitData {
+    pub(crate) fn data(&self, player: &Player) -> UnitData {
         UnitData {
             position: self.position,
             data: UnitBaseData {
@@ -99,7 +100,7 @@ impl Unit {
             carried_units: carried_units(self.id, player)
                 .iter()
                 .map(|id| {
-                    let unit = player.get_unit(*id).unwrap();
+                    let unit = player.get_unit(*id).expect("unit not found");
                     UnitBaseData {
                         unit_type: unit.unit_type.clone(),
                         movement_restrictions: unit.movement_restrictions.clone(),
@@ -411,6 +412,7 @@ pub enum MovementAction {
     Stop,
 }
 
+#[must_use]
 pub fn carried_units(carrier: u32, player: &Player) -> Vec<u32> {
     player
         .units