I have been working on my first ever C# project and I'd love some feedback of the any and all aspects variety. I am building a game called Fleet Command. In the MainWindow I use a ContentControl to use various UserControls based on user interaction. The screen navigation is pretty simple so for the most part I am leaving it out. But for context I would like to give you this:
MainWindow.xaml
<Window x:Class="FleetCommand.MainWindow" xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:d="http://schemas.microsoft.com/expression/blend/2008" xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" mc:Ignorable="d" Title="Fleet Command" Height="700" Width="1200" Background="DarkSlateGray"> <Grid> <ContentControl x:Name="MainScreenContent"/> </Grid> </Window> and
MainWindow.xaml.cs
using System.Windows; using System.Windows.Controls; namespace FleetCommand { /// <summary> /// Struct to encapsulate initialization state from Custom game settings. /// Used to simplify passing many parameters to initialization function and minimize errors. /// </summary> public struct InitialState { public int NumberPlayerCharacters; public int NumberNonPlayerCharacters; public int StartingOil; public int StartingCash; public int StartingResearch; public Difficulty Difficulty; } public enum Difficulty { Easy, Normal, Hard } /// <summary> /// Interaction logic for MainWindow.xaml /// </summary> public partial class MainWindow : Window { public MainWindow() { InitializeComponent(); SetScreen(SplashScreen); } public void ReturnToSplash() => SetScreen(SplashScreen); public void OpenNewGameScreen() => SetScreen(NewGameScreen); public void OpenLoadGameScreen() { LoadGameScreen.Set(); SetScreen(LoadGameScreen); } public void OpenCustomScreen() => SetScreen(CustomGameScreen); public void OpenWorldMap() => GameScreen.OpenWorldMap(); public void OpenCity(string CityName) => GameScreen.OpenCity(CityName); public void StartCampaign() { GameData.StartGame(); SetScreen(GameScreen); } public void StartCustomGame(InitialState state) { GameData.StartGame(state); SetScreen(GameScreen); } public void StartLoadedGame(LoadFile FileSlot) { GameData.StartGame(FileSlot); SetScreen(GameScreen); } public void Reset() { CustomGameScreen.Reset(); GameScreen.Reset(); GameData.Reset(); } public void Save(LoadFile SaveSlot) => GameData.Save(SaveSlot); private void SetScreen(UserControl screen) => MainScreenContent.Content = screen; private static SplashScreen SplashScreen { get; } = new SplashScreen(); private static NewGameScreen NewGameScreen { get; } = new NewGameScreen(); private static LoadGameScreen LoadGameScreen { get; } = new LoadGameScreen(); private static CustomGameScreen CustomGameScreen { get; } = new CustomGameScreen(); private static GameScreen GameScreen { get; } = new GameScreen(); private static GameData GameData { get; set; } = new GameData(); } } The various UserControls call the public methods the MainWindow provides and then it acts accordingly.
However, then I come to the GameScreen. This is where I need a review. GameScreen also provides a ContentControl that can be swapped out with various UserControls in response to users. I'm not unhappy with that and don't think I need to change it, but I don't know, do I? I give the GameScreen all of the buttons it will need in it's various states. Here is. . .
GameScreen.xaml
<UserControl x:Class="FleetCommand.GameScreen" xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" xmlns:d="http://schemas.microsoft.com/expression/blend/2008" mc:Ignorable="d"> <Grid> <Grid.ColumnDefinitions> <ColumnDefinition Width="20"/> <ColumnDefinition Width="*"/> <ColumnDefinition Width="*"/> <ColumnDefinition Width="*"/> <ColumnDefinition Width="*"/> <ColumnDefinition Width="*"/> <ColumnDefinition Width="*"/> <ColumnDefinition Width="*"/> <ColumnDefinition Width="*"/> <ColumnDefinition Width="20"/> </Grid.ColumnDefinitions> <Grid.RowDefinitions> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> <RowDefinition Height="*"/> </Grid.RowDefinitions> <ContentControl Grid.Column="1" Grid.ColumnSpan="8" Grid.Row="1" Grid.RowSpan="14" x:Name="GameField"/> <Rectangle Grid.Column="0" Grid.Row="0" Grid.RowSpan="16" Width="auto" Height="auto" Fill="SteelBlue"/> <Rectangle Grid.Column="9" Grid.Row="0" Grid.RowSpan="16" Width="auto" Height="auto" Fill="SteelBlue"/> <Button Grid.Column="1" Grid.Row="0" Content="Menu" Click="Click_Button" x:Name="menu" VerticalContentAlignment="Top" BorderThickness="0"/> <Button Grid.Column="1" Grid.Row="1" Content="Save" Click="Click_Button" x:Name="save" Visibility="Collapsed"/> <Button Grid.Column="1" Grid.Row="2" Content="Exit" Click="Click_Button" x:Name="exit" Visibility="Collapsed"/> <Button Grid.Column="1" Grid.ColumnSpan="2" Grid.Row="15" Content="Dashboard" Click="Click_Button" x:Name="dashboard"/> <Button Grid.Column="3" Grid.ColumnSpan="2" Grid.Row="15" Content="Research" Click="Click_Button" x:Name="research"/> <Button Grid.Column="5" Grid.ColumnSpan="2" Grid.Row="15" Content="Fleet" Click="Click_Button" x:Name="fleet"/> <Button Grid.Column="7" Grid.ColumnSpan="2" Grid.Row="15" Content="End Turn" Click="Click_Button" x:Name="endTurn"/> <Button Grid.ColumnSpan="2" Grid.Row="15" Content="Unit Design" Click="Click_Button" x:Name="unit" Visibility="Collapsed"/> <Button Grid.Column="7" Grid.ColumnSpan="2" Grid.Row="15" Content="Map" Click="Click_Button" x:Name="world" Visibility="Collapsed"/> <Button Grid.Column="3" Grid.ColumnSpan="2" Grid.Row="15" Content="City Map" Click="Click_Button" x:Name="cityMap" Visibility="Collapsed"/> <Button Grid.Column="5" Grid.ColumnSpan="2" Grid.Row="15" Content="Upgrade City" Click="Click_Button" x:Name="cityUpgrade" Visibility="Collapsed"/> <Button Grid.Column="1" Grid.ColumnSpan="2" Grid.Row="15" Click="Click_Button" x:Name="cityDashboard" Visibility="Collapsed"/> <TextBlock Grid.Column="2" Grid.ColumnSpan="2" Grid.Row="0" x:Name="playerName" Text="Placeholder Name" Background="SteelBlue" FontSize="36"/> <TextBlock Grid.Column="4" Grid.ColumnSpan="2" Grid.Row="0" x:Name="playerOil" Text="10000" Background="SteelBlue" FontSize="36"/> <TextBlock Grid.Column="6" Grid.ColumnSpan="2" Grid.Row="0" x:Name="playerCash" Text="1000" Background="SteelBlue" FontSize="36"/> <TextBlock Grid.Column="8" Grid.Row="0" x:Name="playerResearch" Text="10" Background="SteelBlue" FontSize="36"/> </Grid> </UserControl> I collapse visibility on buttons not in use. I'm still not sure there is anything wrong with this approach but now I am starting to get into a bit of code duplication and I'm not sure of a way around it. As seen in. . .
GameScreen.xaml.cs
using System.Windows; using System.Windows.Controls; namespace FleetCommand { /// <summary> /// Interaction logic for GameScreen.xaml /// </summary> public partial class GameScreen : UserControl { public GameScreen() { InitializeComponent(); SetGameField(OuterMap); } public void Reset() { SetGameField(OuterMap); OuterMap.Reset(); CloseMenu(); } public void OpenWorldMap() => OpenWorld(); public void OpenCity(string CityName) => OpenCityManager(CityName); private void SetGameField(UserControl field) => GameField.Content = field; private void Click_Button(object sender, RoutedEventArgs e) { Button button = sender as Button; switch (button.Name) { case "menu": Menu(); break; case "save": Save(); break; case "exit": Exit(); break; case "dashboard": OpenDashboard(); break; case "research": OpenResearch(); break; case "fleet": OpenFleet(); break; case "unit": OpenUnitDesign(); break; case "cityMap": OpenCityMap(); break; case "cityUpgrade": OpenCityUpgrade(); break; case "cityDashboard": OpenCity(NameOfCity); break; case "world": OpenWorld(); break; case "endTurn": EndTurn(); break; default: break; } } private void Menu() { if (IsMenuOpen) { CloseMenu(); } else { OpenMenu(); } } private void Save() { SaveWindow saveWindow = new SaveWindow(); saveWindow.ShowDialog(); } private void Exit() { bool? proceed = true; Confirm confirm = new Confirm(); proceed = confirm.ShowDialog(); if (proceed != null && (bool)proceed) { MainWindow window = Application.Current.MainWindow as MainWindow; window.ReturnToSplash(); window.Reset(); } } private void OpenWorld() { dashboard.Visibility = Visibility.Visible; research.Visibility = Visibility.Visible; fleet.Visibility = Visibility.Visible; endTurn.Visibility = Visibility.Visible; unit.Visibility = Visibility.Collapsed; world.Visibility = Visibility.Collapsed; cityMap.Visibility = Visibility.Collapsed; cityUpgrade.Visibility = Visibility.Collapsed; cityDashboard.Visibility = Visibility.Collapsed; SetGameField(OuterMap); OuterMap.OpenWorldMap(); } private void OpenDashboard() { Grid.SetColumn(unit, 1); unit.Visibility = Visibility.Visible; research.Visibility = Visibility.Visible; fleet.Visibility = Visibility.Visible; world.Visibility = Visibility.Visible; dashboard.Visibility = Visibility.Collapsed; endTurn.Visibility = Visibility.Collapsed; cityMap.Visibility = Visibility.Collapsed; cityUpgrade.Visibility = Visibility.Collapsed; cityDashboard.Visibility = Visibility.Collapsed; SetGameField(Dashboard); } private void OpenResearch() { Grid.SetColumn(unit, 3); dashboard.Visibility = Visibility.Visible; unit.Visibility = Visibility.Visible; fleet.Visibility = Visibility.Visible; world.Visibility = Visibility.Visible; research.Visibility = Visibility.Collapsed; endTurn.Visibility = Visibility.Collapsed; cityMap.Visibility = Visibility.Collapsed; cityUpgrade.Visibility = Visibility.Collapsed; cityDashboard.Visibility = Visibility.Collapsed; SetGameField(Research); } private void OpenFleet() { Grid.SetColumn(unit, 5); dashboard.Visibility = Visibility.Visible; research.Visibility = Visibility.Visible; unit.Visibility = Visibility.Visible; world.Visibility = Visibility.Visible; fleet.Visibility = Visibility.Visible; endTurn.Visibility = Visibility.Visible; cityMap.Visibility = Visibility.Collapsed; cityUpgrade.Visibility = Visibility.Collapsed; cityDashboard.Visibility = Visibility.Collapsed; SetGameField(Fleet); } private void OpenCityManager(string CityName) { dashboard.Visibility = Visibility.Visible; cityMap.Visibility = Visibility.Visible; cityUpgrade.Visibility = Visibility.Visible; world.Visibility = Visibility.Visible; research.Visibility = Visibility.Collapsed; unit.Visibility = Visibility.Collapsed; fleet.Visibility = Visibility.Collapsed; endTurn.Visibility = Visibility.Collapsed; cityDashboard.Visibility = Visibility.Collapsed; cityDashboard.Content = NameOfCity = CityName; SetGameField(CityDashboard); } private void OpenCityMap() { Grid.SetColumn(unit, 3); cityDashboard.Visibility = Visibility.Visible; unit.Visibility = Visibility.Visible; cityUpgrade.Visibility = Visibility.Visible; world.Visibility = Visibility.Visible; dashboard.Visibility = Visibility.Collapsed; cityMap.Visibility = Visibility.Collapsed; research.Visibility = Visibility.Collapsed; fleet.Visibility = Visibility.Collapsed; endTurn.Visibility = Visibility.Collapsed; SetGameField(OuterMap); OuterMap.OpenCityMap(NameOfCity); } private void OpenUnitDesign() { dashboard.Visibility = Visibility.Visible; research.Visibility = Visibility.Visible; fleet.Visibility = Visibility.Visible; world.Visibility = Visibility.Visible; cityDashboard.Visibility = Visibility.Collapsed; unit.Visibility = Visibility.Collapsed; cityUpgrade.Visibility = Visibility.Collapsed; cityMap.Visibility = Visibility.Collapsed; endTurn.Visibility = Visibility.Collapsed; SetGameField(UnitDesign); } private void OpenCityUpgrade() { Grid.SetColumn(unit, 5); cityDashboard.Visibility = Visibility.Visible; cityMap.Visibility = Visibility.Visible; unit.Visibility = Visibility.Visible; world.Visibility = Visibility.Visible; dashboard.Visibility = Visibility.Collapsed; research.Visibility = Visibility.Collapsed; fleet.Visibility = Visibility.Collapsed; cityUpgrade.Visibility = Visibility.Collapsed; endTurn.Visibility = Visibility.Collapsed; SetGameField(CityUpgrade); } private void EndTurn() { // TODO: End Turn via Data } private void OpenMenu() { save.Visibility = Visibility.Visible; exit.Visibility = Visibility.Visible; IsMenuOpen = true; } private void CloseMenu() { save.Visibility = Visibility.Hidden; exit.Visibility = Visibility.Hidden; IsMenuOpen = false; } private string NameOfCity { get; set; } private string PlayerName { get; set; } private string PlayerOil { get; set; } private string PlayerCash { get; set; } private string PlayerResearch { get; set; } private bool IsMenuOpen { get; set; } = false; private static OuterMap OuterMap { get; } = new OuterMap(); private static Dashboard Dashboard { get; } = new Dashboard(); private static CityDashboard CityDashboard { get; } = new CityDashboard(); private static Research Research { get; } = new Research(); private static Fleet Fleet { get; } = new Fleet(); private static UnitDesign UnitDesign { get; } = new UnitDesign(); private static CityUpgrade CityUpgrade { get; } = new CityUpgrade(); } } My biggest issue with this approach is manually handling the visibility of each button as I navigate through each subcontent of the GameScreen. I had considered not needing the GameScreen at all (after all each UserControl could simply be the content of the MainWindow, but they all share so many elements that I didn't like that idea. I could move just the <Button>s but that wouldn't actually remove the repetitiveness it would just spread it out across multiple files. I think I would prefer to keep it together in one place. It will make making changes easier I believe.
I can add more of the files if needed but the GameScreen is the big and ugly one I'm worried about. I am also interested in anything else you might see.
