avoid option in actions because mongo strips null values

client/src/client_state.rs

@@ -378,7 +378,7 @@ impl State {
             GameState::StatusPhase(state) => match state {
                 StatusPhaseState::CompleteObjectives => ActiveDialog::CompleteObjectives,
                 StatusPhaseState::FreeAdvance => ActiveDialog::FreeAdvance,
-                StatusPhaseState::RaseSize1City => ActiveDialog::RazeSize1City,
+                StatusPhaseState::RazeSize1City => ActiveDialog::RazeSize1City,
                 StatusPhaseState::ChangeGovernmentType => ActiveDialog::ChangeGovernmentType,
                 StatusPhaseState::DetermineFirstPlayer => ActiveDialog::DetermineFirstPlayer,
             },

client/src/combat_ui.rs

@@ -1,4 +1,4 @@
-use server::action::{Action, CombatAction};
+use server::action::{Action, CombatAction, PlayActionCard};
 use server::game::Game;
 use server::position::Position;
 use server::unit::Unit;
@@ -105,7 +105,9 @@ pub fn remove_casualties_dialog(
 pub fn play_action_card_dialog(player: &ShownPlayer) -> StateUpdate {
     active_dialog_window(player, "Play action card", |ui| {
         if ui.button(None, "None") {
-            return StateUpdate::Execute(Action::Combat(CombatAction::PlayActionCard(None)));
+            return StateUpdate::Execute(Action::Combat(CombatAction::PlayActionCard(
+                PlayActionCard::None,
+            )));
         }
         StateUpdate::None
     })

client/src/remote_client/bin/main.rs

@@ -19,10 +19,10 @@ extern "C" {
 #[wasm_bindgen]
 extern "C" {
     type Rect;
-    
+
     #[wasm_bindgen(method, getter)]
     fn width(this: &Rect) -> f32;
-    
+
     #[wasm_bindgen(method, getter)]
     fn height(this: &Rect) -> f32;
 }
@@ -107,10 +107,7 @@ impl RemoteClient {
             }
 
             let s = self.control.canvas_size();
-            self.state.screen_size = vec2(
-                s.width(),
-                s.height(),
-            );
+            self.state.screen_size = vec2(s.width(), s.height());
 
             let sync_result = self.update_state();
 

client/src/status_phase_ui.rs

@@ -6,7 +6,9 @@ use server::action::Action;
 use server::content::advances;
 use server::game::Game;
 use server::position::Position;
-use server::status_phase::{ChangeGovernmentType, StatusPhaseAction};
+use server::status_phase::{
+    ChangeGovernment, ChangeGovernmentType, RazeSize1City, StatusPhaseAction,
+};
 
 pub fn determine_first_player_dialog(game: &Game, player: &ShownPlayer) -> StateUpdate {
     active_dialog_window(
@@ -32,7 +34,9 @@ pub fn raze_city_confirm_dialog(game: &Game, player: &ShownPlayer, pos: Position
     if player.get(game).can_raze_city(pos) {
         StateUpdate::execute_with_confirm(
             vec![format!("Raze {pos} to get 1 gold")],
-            Action::StatusPhase(StatusPhaseAction::RaseSize1City(Some(pos))),
+            Action::StatusPhase(StatusPhaseAction::RazeSize1City(RazeSize1City::Position(
+                pos,
+            ))),
         )
     } else {
         StateUpdate::None
@@ -42,7 +46,9 @@ pub fn raze_city_confirm_dialog(game: &Game, player: &ShownPlayer, pos: Position
 pub fn raze_city_dialog(player: &ShownPlayer) -> StateUpdate {
     active_dialog_window(player, "Select a city to raze - or decline.", |ui| {
         if ui.button(None, "Decline") {
-            return StateUpdate::status_phase(StatusPhaseAction::RaseSize1City(None));
+            return StateUpdate::status_phase(StatusPhaseAction::RazeSize1City(
+                RazeSize1City::None,
+            ));
         }
         StateUpdate::None
     })
@@ -124,7 +130,9 @@ pub fn change_government_type_dialog(game: &Game, player: &ShownPlayer) -> State
         }
 
         if ui.button(None, "Decline") {
-            return StateUpdate::status_phase(StatusPhaseAction::ChangeGovernmentType(None));
+            return StateUpdate::status_phase(StatusPhaseAction::ChangeGovernmentType(
+                ChangeGovernmentType::KeepGovernment,
+            ));
         }
         StateUpdate::None
     })
@@ -142,12 +150,12 @@ pub fn choose_additional_advances_dialog(
         additional_advances,
         |a| StateUpdate::SetDialog(ActiveDialog::ChooseAdditionalAdvances(a)),
         |a| {
-            StateUpdate::status_phase(StatusPhaseAction::ChangeGovernmentType(Some(
-                ChangeGovernmentType {
+            StateUpdate::status_phase(StatusPhaseAction::ChangeGovernmentType(
+                ChangeGovernmentType::ChangeGovernment(ChangeGovernment {
                     new_government: a.government.clone(),
                     additional_advances: a.selected.clone(),
-                },
-            )))
+                }),
+            ))
         },
     )
 }

server/src/action.rs

@@ -75,7 +75,14 @@ impl Action {
 
 #[derive(Serialize, Deserialize, Clone)]
 pub enum CombatAction {
-    PlayActionCard(Option<String>),
+    PlayActionCard(PlayActionCard),
     RemoveCasualties(Vec<u32>),
     Retreat(bool),
 }
+
+// Can't use Option<String> because of mongo stips null values
+#[derive(Serialize, Deserialize, Clone)]
+pub enum PlayActionCard {
+    None,
+    Card(String),
+}

server/src/combat.rs

@@ -1,4 +1,4 @@
-use crate::action::CombatAction;
+use crate::action::{CombatAction, PlayActionCard};
 use crate::game::GameState::Playing;
 use crate::game::{Game, GameState};
 use crate::map::Terrain::Water;
@@ -130,7 +130,7 @@ pub fn execute_combat_action(game: &mut Game, action: CombatAction, mut c: Comba
     game.lock_undo();
     match action {
         CombatAction::PlayActionCard(card) => {
-            assert!(card.is_none());
+            assert!(matches!(card, PlayActionCard::None));
             //todo use card
             combat_loop(game, c);
             return;

server/src/game_api_wrapper.rs

@@ -1,8 +1,8 @@
 #![allow(clippy::pedantic)]
 
+use crate::{game::Game, game_api};
 use serde::{Deserialize, Serialize};
 use wasm_bindgen::prelude::*;
-use crate::{game::Game, game_api};
 extern crate console_error_panic_hook;
 
 #[derive(Serialize, Deserialize)]

server/src/log.rs

@@ -3,6 +3,8 @@
 use itertools::Itertools;
 use serde::{Deserialize, Serialize};
 
+use crate::action::PlayActionCard;
+use crate::status_phase::{ChangeGovernmentType, RazeSize1City};
 use crate::{
     action::{Action, CombatAction},
     game::Game,
@@ -161,28 +163,30 @@ fn format_status_phase_action_log_item(action: &StatusPhaseAction, game: &Game)
         StatusPhaseAction::FreeAdvance(advance) => {
             format!("{player_name} advanced {advance} for free")
         }
-        StatusPhaseAction::RaseSize1City(city) => {
+        StatusPhaseAction::RazeSize1City(city) => {
             format!(
                 "{player_name} {}",
                 match city {
-                    Some(city) => format!("razed the city at {city} and gained 1 gold"),
-                    None => String::from("did not rase a city"),
+                    RazeSize1City::Position(city) =>
+                        format!("razed the city at {city} and gained 1 gold"),
+                    RazeSize1City::None => String::from("did not rase a city"),
                 }
             )
         }
         StatusPhaseAction::ChangeGovernmentType(new_government) => {
             format!(
                 "{player_name} {}",
                 match new_government {
-                    Some(new_government_advance) => format!(
+                    ChangeGovernmentType::ChangeGovernment(new_government_advance) => format!(
                         "changed his government from {} to {} - additional advances: {}",
                         game.players[game.active_player()]
                             .government()
                             .expect("player should have a government before changing it"),
                         new_government_advance.new_government,
                         new_government_advance.additional_advances.join(", ")
                     ),
-                    None => String::from("did not change his government"),
+                    ChangeGovernmentType::KeepGovernment =>
+                        String::from("did not change his government"),
                 }
             )
         }
@@ -219,10 +223,10 @@ fn format_combat_action_log_item(action: &CombatAction, game: &Game) -> String {
     match action {
         CombatAction::PlayActionCard(card) => format!(
             "{player_name} {}",
-            card.as_ref()
-                .map_or(String::from("did not play a tactics card"), |card| format!(
-                    "played the {card} tactics card"
-                ))
+            match card {
+                PlayActionCard::Card(card) => format!("played the {card} tactics card"),
+                PlayActionCard::None => String::from("did not play a tactics card"),
+            }
         ),
         CombatAction::RemoveCasualties(casualties) => format!(
             "{player_name} removed {}",

server/src/status_phase.rs

@@ -11,17 +11,31 @@ use crate::{
 };
 
 #[derive(Serialize, Deserialize, Clone)]
-pub struct ChangeGovernmentType {
+pub struct ChangeGovernment {
     pub new_government: String,
     pub additional_advances: Vec<String>,
 }
 
+// Can't use Option<String> because of mongo stips null values
+#[derive(Serialize, Deserialize, Clone)]
+pub enum ChangeGovernmentType {
+    ChangeGovernment(ChangeGovernment),
+    KeepGovernment,
+}
+
+// Can't use Option<String> because of mongo stips null values
+#[derive(Serialize, Deserialize, Clone)]
+pub enum RazeSize1City {
+    None,
+    Position(Position),
+}
+
 #[derive(Serialize, Deserialize, Clone)]
 pub enum StatusPhaseAction {
     CompleteObjectives(Vec<String>),
     FreeAdvance(String),
-    RaseSize1City(Option<Position>),
-    ChangeGovernmentType(Option<ChangeGovernmentType>),
+    RazeSize1City(RazeSize1City),
+    ChangeGovernmentType(ChangeGovernmentType),
     DetermineFirstPlayer(usize),
 }
 
@@ -31,7 +45,7 @@ impl StatusPhaseAction {
         match self {
             StatusPhaseAction::CompleteObjectives(_) => StatusPhaseState::CompleteObjectives,
             StatusPhaseAction::FreeAdvance(_) => StatusPhaseState::FreeAdvance,
-            StatusPhaseAction::RaseSize1City(_) => StatusPhaseState::RaseSize1City,
+            StatusPhaseAction::RazeSize1City(_) => StatusPhaseState::RazeSize1City,
             StatusPhaseAction::ChangeGovernmentType(_) => StatusPhaseState::ChangeGovernmentType,
             StatusPhaseAction::DetermineFirstPlayer(_) => StatusPhaseState::DetermineFirstPlayer,
         }
@@ -54,8 +68,8 @@ impl StatusPhaseAction {
                 );
                 game.advance(advance, player_index);
             }
-            StatusPhaseAction::RaseSize1City(ref city) => {
-                if let Some(city) = *city {
+            StatusPhaseAction::RazeSize1City(ref city) => {
+                if let RazeSize1City::Position(city) = *city {
                     assert!(
                         game.players[player_index].can_raze_city(city),
                         "Illegal action"
@@ -65,7 +79,9 @@ impl StatusPhaseAction {
                 }
             }
             StatusPhaseAction::ChangeGovernmentType(ref new_government_advance) => {
-                if let Some(new_government) = new_government_advance {
+                if let ChangeGovernmentType::ChangeGovernment(new_government) =
+                    new_government_advance
+                {
                     change_government_type(game, player_index, new_government);
                 }
             }
@@ -80,11 +96,7 @@ impl StatusPhaseAction {
     }
 }
 
-fn change_government_type(
-    game: &mut Game,
-    player_index: usize,
-    new_government: &ChangeGovernmentType,
-) {
+fn change_government_type(game: &mut Game, player_index: usize, new_government: &ChangeGovernment) {
     let government = &new_government.new_government;
     if advances::get_leading_government_advance(government)
         .expect("government should exist")
@@ -177,7 +189,7 @@ fn skip_player(game: &Game, player_index: usize, state: &StatusPhaseState) -> bo
         StatusPhaseState::FreeAdvance => !advances::get_all()
             .into_iter()
             .any(|advance| player.can_advance_free(&advance.name)),
-        StatusPhaseState::RaseSize1City => !player.cities.iter().any(|city| city.size() == 1),
+        StatusPhaseState::RazeSize1City => !player.cities.iter().any(|city| city.size() == 1),
         StatusPhaseState::ChangeGovernmentType => {
             player.government().is_none()
                 || player.government().is_some_and(|government| {
@@ -195,7 +207,7 @@ pub enum StatusPhaseState {
     CompleteObjectives,
     FreeAdvance,
     //draw new cards (after free advance)
-    RaseSize1City,
+    RazeSize1City,
     ChangeGovernmentType,
     DetermineFirstPlayer,
 }
@@ -206,8 +218,8 @@ pub fn next_status_phase(phase: Option<StatusPhaseState>) -> StatusPhaseState {
     if let Some(phase) = phase {
         match phase {
             CompleteObjectives => FreeAdvance,
-            FreeAdvance => RaseSize1City,
-            RaseSize1City => ChangeGovernmentType,
+            FreeAdvance => RazeSize1City,
+            RazeSize1City => ChangeGovernmentType,
             ChangeGovernmentType => DetermineFirstPlayer,
             DetermineFirstPlayer => {
                 unreachable!("function should return early with this action")

server/tests/game_api_tests.rs

@@ -8,7 +8,7 @@ use std::{
 
 use server::action::CombatAction;
 use server::game::{CulturalInfluenceResolution, GameState};
-use server::status_phase::{ChangeGovernmentType, StatusPhaseAction};
+use server::status_phase::{ChangeGovernment, ChangeGovernmentType, RazeSize1City, StatusPhaseAction};
 use server::{
     action::Action,
     city::{City, MoodState::*},
@@ -627,7 +627,7 @@ fn test_construct_port() {
 fn test_wrong_status_phase_action() {
     test_action(
         "illegal_free_advance",
-        Action::StatusPhase(StatusPhaseAction::RaseSize1City(None)),
+        Action::StatusPhase(StatusPhaseAction::RazeSize1City(RazeSize1City::None)),
         0,
         false,
         true,
@@ -651,7 +651,7 @@ fn test_free_advance() {
 fn test_raze_city() {
     test_action(
         "raze_city",
-        Action::StatusPhase(StatusPhaseAction::RaseSize1City(Some(
+        Action::StatusPhase(StatusPhaseAction::RazeSize1City(RazeSize1City::Position(
             Position::from_offset("A1"),
         ))),
         0,
@@ -664,7 +664,7 @@ fn test_raze_city() {
 fn test_raze_city_decline() {
     test_action(
         "raze_city_decline",
-        Action::StatusPhase(StatusPhaseAction::RaseSize1City(None)),
+        Action::StatusPhase(StatusPhaseAction::RazeSize1City(RazeSize1City::None)),
         0,
         false,
         false,
@@ -686,8 +686,8 @@ fn test_determine_first_player() {
 fn test_change_government() {
     test_action(
         "change_government",
-        Action::StatusPhase(StatusPhaseAction::ChangeGovernmentType(Some(
-            ChangeGovernmentType {
+        Action::StatusPhase(StatusPhaseAction::ChangeGovernmentType(ChangeGovernmentType::ChangeGovernment(
+            ChangeGovernment {
                 new_government: String::from("Theocracy"),
                 additional_advances: vec![String::from("Theocracy 2")],
             },

server/tests/test_games/change_government.json

@@ -471,12 +471,12 @@
     },
     {
       "StatusPhase": {
-        "RaseSize1City": null
+        "RazeSize1City": "None"
       }
     },
     {
       "StatusPhase": {
-        "RaseSize1City": null
+        "RazeSize1City": "None"
       }
     }
   ],