3
\$\begingroup\$

I am a beginner in C# and trying to learn how to program with OOP in mind. I am 100% that the code is not OOP-friendly at all, but the game is running and working, so if anyone could look throw the code and give me feedback on how to improve it, that would be helpful.

The game is a simple snake game with three players option. I have done two players part, but the code seems to be ugly and messy, so I didn't want to add anything more to the garbage, and I am completely ok if I have to redo everything I am doing this to learn anyways

Form1.cs

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Diagnostics;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;

namespace snakeGame3players
{
    public partial class Form1 : Form
    {
        private readonly List<Square> SnakeBoddy = new List<Square>();
        private readonly List<Square> SnakeBoddy2 = new List<Square>();
        private readonly List<Square> SnakeFoods1 = new List<Square>();
        private readonly List<Square> SnakeFoods2 = new List<Square>();
        private readonly List<Square> SnakeFoods3 = new List<Square>();
        private readonly List<Square> SnakeFoods4 = new List<Square>();
        System.Windows.Forms.Timer eventTimer = new System.Windows.Forms.Timer();
        System.Windows.Forms.Timer foodTimer = new System.Windows.Forms.Timer();
        System.Windows.Forms.Timer foodTimer2 = new System.Windows.Forms.Timer();
        System.Windows.Forms.Timer foodTimer3 = new System.Windows.Forms.Timer();
        System.Windows.Forms.Timer foodTimer4 = new System.Windows.Forms.Timer();
        Random random = new Random();
        bool exitFlag = false;


        public Form1()
        {
            InitializeComponent();
            choice1.Appearance = System.Windows.Forms.Appearance.Button;
            choice2.Appearance = System.Windows.Forms.Appearance.Button;
            choice3.Appearance = System.Windows.Forms.Appearance.Button;
            choice1.Size = new Size(154, 56);
            choice2.Size = new Size(154, 56);
            choice3.Size = new Size(154, 56);
            new Player1setting();
            new Player2setting();
            new FoodSetting();
            GameTimer.Interval = 100;
            this.KeyPreview = true;
            GameTimer.Tick += UpdateScreen;
            GameTimer.Start();
            label3.Visible = false;
            label2.Visible = false;

        }

        private void UpdateScreen(object sender, EventArgs e)
        {

            if (label1.Visible == true)
            {
                if (KeyInput.KeyInputs(Keys.Q))
                {

                    Application.Restart();
                    Environment.Exit(0);
                }
            }
            if (choice2.Checked && Player1setting.Gameover ==false)
            {
                if (KeyInput.KeyInputs(Keys.Right) && Player1setting.Movment != SnakeMovment.Left)
                {
                    Player1setting.Movment = SnakeMovment.Right;
                }

                else if (KeyInput.KeyInputs(Keys.Left) && Player1setting.Movment != SnakeMovment.Right)
                {
                    Player1setting.Movment = SnakeMovment.Left;
                }

                else if (KeyInput.KeyInputs(Keys.Up) && Player1setting.Movment != SnakeMovment.Down)
                {
                    Player1setting.Movment = SnakeMovment.Up;
                }

                else if (KeyInput.KeyInputs(Keys.Down) && Player1setting.Movment != SnakeMovment.Up)
                {
                    Player1setting.Movment = SnakeMovment.Down;
                }

                PlayerMovment();
            }

            pictureBox1.Invalidate();

            
            if (choice1.Checked && Player2setting.Gameover2 == false)
            {
                if (KeyInput.KeyInputs(Keys.D) && Player2setting.Movment2 != SnakeMovment2.A)
                {
                    Player2setting.Movment2 = SnakeMovment2.D;
                }

                else if (KeyInput.KeyInputs(Keys.A) && Player2setting.Movment2 != SnakeMovment2.D)
                {
                    Player2setting.Movment2 = SnakeMovment2.A;
                }

                else if (KeyInput.KeyInputs(Keys.W) && Player2setting.Movment2 != SnakeMovment2.S)
                {
                    Player2setting.Movment2 = SnakeMovment2.W;
                }

                else if (KeyInput.KeyInputs(Keys.S) && Player2setting.Movment2 != SnakeMovment2.W)
                {
                    Player2setting.Movment2 = SnakeMovment2.S;
                }

                PlayerMovment2();
            }

            pictureBox1.Invalidate();

        }

        private void PlayerMovment()
        {
            if (choice2.Checked) {
                for (int i = SnakeBoddy.Count - 1; i >= 0; i--)
                {
                    if (i == 0)
                    {
                        switch (Player1setting.Movment)
                        {
                            case SnakeMovment.Right:
                                SnakeBoddy[i].X++;
                                break;

                            case SnakeMovment.Left:
                                SnakeBoddy[i].X--;
                                break;

                            case SnakeMovment.Up:
                                SnakeBoddy[i].Y--;
                                break;

                            case SnakeMovment.Down:
                                SnakeBoddy[i].Y++;
                                break;
                        }

                        int maxXPosition = pictureBox1.Size.Width / Player1setting.Width;
                        int maxYPosition = pictureBox1.Size.Height / Player1setting.Height;

                        if (
                            SnakeBoddy[i].X < 0 || SnakeBoddy[i].Y < 0 ||
                            SnakeBoddy[i].X > maxXPosition || SnakeBoddy[i].Y >= maxYPosition
                            )
                        {
                            Die();
                        }

                        for (int J = 1; J < SnakeBoddy.Count; J++)
                        {
                            if (SnakeBoddy[i].X == SnakeBoddy[J].X && SnakeBoddy[i].Y == SnakeBoddy[J].Y)
                            {
                                Die();
                            }
                        }
                        for (int g = 0; g < SnakeBoddy2.Count; g++)
                        {
                            if (SnakeBoddy[0].X == SnakeBoddy2[g].X && SnakeBoddy[0].Y == SnakeBoddy2[g].Y)
                            {
                                Player1setting.GameScore += FoodSetting.food2;
                                label3.Text = Player1setting.GameScore.ToString();
                                Die();
                            }
                        }
                        if (SnakeFoods1.Any(s => s.X == SnakeBoddy[0].X && s.Y == SnakeBoddy[0].Y))
                        {
                            EatFood();
                        }

                        if (SnakeFoods2.Any(s => s.X == SnakeBoddy[0].X && s.Y == SnakeBoddy[0].Y))
                        {
                            EatFood2();
                        }

                        if (SnakeFoods3.Any(s => s.X == SnakeBoddy[0].X && s.Y == SnakeBoddy[0].Y))
                        {
                            EatFood3();
                        }

                        if (SnakeFoods4.Any(s => s.X == SnakeBoddy[0].X && s.Y == SnakeBoddy[0].Y))
                        {
                            EatFood4();
                        }
                    }
                    else
                    {
                        SnakeBoddy[i].X = SnakeBoddy[i - 1].X;
                        SnakeBoddy[i].Y = SnakeBoddy[i - 1].Y;
                    }
                }
            }
                
        }
        
        private void PlayerMovment2()
        {
            if (choice1.Checked)
            {
                for (int x = SnakeBoddy2.Count - 1; x >= 0; x--)
                {
                    if (x == 0)
                    {
                        switch (Player2setting.Movment2)
                        {
                            case SnakeMovment2.D:
                                SnakeBoddy2[x].X++;
                                break;

                            case SnakeMovment2.A:
                                SnakeBoddy2[x].X--;
                                break;

                            case SnakeMovment2.W:
                                SnakeBoddy2[x].Y--;
                                break;

                            case SnakeMovment2.S:
                                SnakeBoddy2[x].Y++;
                                break;
                        }

                        int maxXPosition2 = pictureBox1.Size.Width / Player2setting.Width2;
                        int maxYPosition2 = pictureBox1.Size.Height / Player2setting.Height2;

                        if (
                            SnakeBoddy2[x].X < 0 || SnakeBoddy2[x].Y < 0 ||
                            SnakeBoddy2[x].X > maxXPosition2 || SnakeBoddy2[x].Y >= maxYPosition2
                            )
                        {
                            Die2();
                        }

                        for (int h = 1; h < SnakeBoddy2.Count; h++)
                        {
                            if (SnakeBoddy2[x].X == SnakeBoddy2[h].X && SnakeBoddy2[x].Y == SnakeBoddy2[h].Y)
                            {
                                Die2();
                            }
                        }

                        for (int g = 0; g < SnakeBoddy.Count; g++)
                        {
                            if (SnakeBoddy2[0].X == SnakeBoddy[g].X && SnakeBoddy2[0].Y == SnakeBoddy[g].Y)
                            {
                                Player1setting.GameScore += FoodSetting.food2;
                                label2.Text = Player1setting.GameScore.ToString();
                                Die2();
                            }
                        }


                        if (SnakeFoods1.Any(s => s.X == SnakeBoddy2[0].X && s.Y == SnakeBoddy2[0].Y))
                        {
                            EatFood();
                        }

                        if (SnakeFoods2.Any(s => s.X == SnakeBoddy2[0].X && s.Y == SnakeBoddy2[0].Y))
                        {
                            EatFood2();
                        }

                        if (SnakeFoods3.Any(s => s.X == SnakeBoddy2[0].X && s.Y == SnakeBoddy2[0].Y))
                        {
                            EatFood3();
                        }

                        if (SnakeFoods4.Any(s => s.X == SnakeBoddy2[0].X && s.Y == SnakeBoddy2[0].Y))
                        {
                            EatFood4();
                        }
                    }
                    else
                    {
                        SnakeBoddy2[x].X = SnakeBoddy2[x - 1].X;
                        SnakeBoddy2[x].Y = SnakeBoddy2[x - 1].Y;
                    }
                }
            }
               

        }

        private void Form1_Load(object sender, EventArgs e)
        {

        }

        private void Keyisup(object sender, KeyEventArgs e)
        {
            KeyInput.SnakeDirections(e.KeyCode, false);
        }

        private void Keyisdown(object sender, KeyEventArgs e)
        {
            KeyInput.SnakeDirections(e.KeyCode, true);
        }

        private void UpdateGame(object sender, PaintEventArgs e)
        {

            Graphics canvas = e.Graphics;
            if (label1.Visible==false && Information.Visible==false)
            {
                foreach (Square SnakeFood in SnakeFoods1) {
                    canvas.FillRectangle(Brushes.Red,
                                       new Rectangle(
                                           SnakeFood.X * Player2setting.Width2,
                                           SnakeFood.Y * Player2setting.Width2,
                                           Player2setting.Width2, Player2setting.Height2
                                                  ));
                }
                foreach (Square SnakeFoods2 in SnakeFoods2) { 
                    canvas.FillRectangle(Brushes.Orange,
                                   new Rectangle(
                                       SnakeFoods2.X * Player1setting.Width,
                                       SnakeFoods2.Y * Player1setting.Width,
                                       Player1setting.Width, Player1setting.Height
                                              ));
                }
                foreach (Square SnakeFoods3 in SnakeFoods3) {
                    canvas.FillRectangle(Brushes.Purple,
                                   new Rectangle(
                                       SnakeFoods3.X * Player1setting.Width,
                                       SnakeFoods3.Y * Player1setting.Width,
                                       Player1setting.Width, Player1setting.Height
                                              ));
                }
                foreach (Square SnakeFoods4 in SnakeFoods4) {
                    canvas.FillRectangle(Brushes.Green,
                                   new Rectangle(
                                       SnakeFoods4.X * Player1setting.Width,
                                       SnakeFoods4.Y * Player1setting.Width,
                                       Player1setting.Width, Player1setting.Height
                                              ));
                }
            }
                

            if (choice2.Checked)
            {
                if (Player1setting.Gameover == false)
                {
                    Brush SnakeBoddyColor;

                    for (int i = 0; i < SnakeBoddy.Count; i++)
                    {
                        if (i == 0)
                        {
                            SnakeBoddyColor = Brushes.Black;
                            canvas.FillRectangle(SnakeBoddyColor,
                                               new Rectangle(
                                                   SnakeBoddy[i].X * Player1setting.Width,
                                                   SnakeBoddy[i].Y * Player1setting.Width,
                                                   Player1setting.Width, Player1setting.Height
                                                   ));
                        }
                        else
                        {
                            SnakeBoddyColor = Brushes.Red;
                            canvas.FillRectangle(SnakeBoddyColor,
                                               new Rectangle(
                                                   SnakeBoddy[i].X * Player1setting.Width,
                                                   SnakeBoddy[i].Y * Player1setting.Width,
                                                   Player1setting.Width, Player1setting.Height
                                                          ));
                        }
                    }
                }
            }
            if (choice1.Checked)
            {
                if (Player2setting.Gameover2 == false)
                {
                    Brush SnakeBoddyColor2;

                    for (int x = 0; x < SnakeBoddy2.Count; x++)
                    {
                        if (x == 0)
                        {
                            SnakeBoddyColor2 = Brushes.Black;
                            canvas.FillRectangle(SnakeBoddyColor2,
                                               new Rectangle(
                                                   SnakeBoddy2[x].X * Player2setting.Width2,
                                                   SnakeBoddy2[x].Y * Player2setting.Width2,
                                                   Player2setting.Width2, Player2setting.Height2
                                                   ));
                        }
                        else
                        {
                            SnakeBoddyColor2 = Brushes.Blue;
                            canvas.FillRectangle(SnakeBoddyColor2,
                                               new Rectangle(
                                                   SnakeBoddy2[x].X * Player2setting.Width2,
                                                   SnakeBoddy2[x].Y * Player2setting.Width2,
                                                   Player2setting.Width2, Player2setting.Height2
                                                          ));
                        }
                    }
                }
            }
               

            if (Player2setting.GameScore2 > Player1setting.GameScore && Player2setting.Gameover2 == true)
            {
                string gameEnd = "Player 1 \n" + "With the score of " + Player2setting.GameScore2 + "\nPress Q to play again \n";
                label1.Text = gameEnd;
                label3.Visible = false;
                label2.Visible = false;
                label1.Visible = true;
            }
            if (Player2setting.GameScore2 < Player1setting.GameScore && Player1setting.Gameover == true) 
            {
                string gameEnd = "Player 2 \n" + "With the score of " + Player1setting.GameScore + "\nPress Q to play again\n";
                label1.Text = gameEnd;
                label3.Visible = false;
                label2.Visible = false;
                label1.Visible = true;
            }
            if (Player2setting.GameScore2 == Player1setting.GameScore && Player1setting.Gameover == true && Player2setting.Gameover2 == true)
            {
                string gameEnd = "Draw \nPress Q to play again\n";
                label1.Text = gameEnd;
                label3.Visible = false;
                label2.Visible = false;
                label1.Visible = true;
            }

        }

        private void StartGame()
        {
            pictureBox1.Visible = false;
            Information.Visible = false;
            Player1Information.Visible = false;
            Player2Information.Visible = false;
            Player3Information.Visible = false;
            choice1.Visible = false;
            choice2.Visible = false;
            choice3.Visible = false;
            GameStart.Visible = false;
            pictureBox3.Visible = false;
            pictureBox1.Visible = true;
            label1.Visible = false;
            new FoodSetting();
            new Player1setting();
            new Player2setting();
            if (choice2.Checked)
            {
                label2.Visible = true;
                SnakeBoddy.Clear();
                Square SneakHead = new Square { X = 35, Y = 5 };
                SnakeBoddy.Add(SneakHead);
                if (choice2.Checked && !choice1.Checked)
                {
                    Square SneakHead2 = new Square { X = 35, Y = 5 };
                    Player2setting.GameScore2 = -1;
                    SnakeBoddy2.Add(SneakHead2);
                }
            }
            if (choice1.Checked)
            {
                label3.Visible = true;
                SnakeBoddy2.Clear();
                Square SneakHead2 = new Square { X = 10, Y = 5 };
                SnakeBoddy2.Add(SneakHead2);
                if (choice1.Checked && !choice2.Checked)
                {
                    Square SneakHead = new Square { X = 10, Y = 5 };
                    Player1setting.GameScore = -1;
                    SnakeBoddy.Add(SneakHead);
                }
            }
            label2.Text = Player1setting.GameScore.ToString();
            label3.Text = Player2setting.GameScore2.ToString();
            FoodEvent();

        }

        private void GenerateSnakeFood()
        {
            Square newFood = GenerateRandomFood();

            while (SnakeFoods1.Any(snakeFood => snakeFood.X == newFood.X && snakeFood.Y == newFood.Y))
            {
                newFood = GenerateRandomFood();
            }

            SnakeFoods1.Add(newFood);

        }

        private void GenerateSnakeFood2()
        {
            Square newFood = GenerateRandomFood();

            while (SnakeFoods2.Any(snakeFood => snakeFood.X == newFood.X && snakeFood.Y == newFood.Y))
            {
                newFood = GenerateRandomFood();
            }

            SnakeFoods2.Add(newFood);

        }

        private void GenerateSnakeFood3()
        {
            Square newFood = GenerateRandomFood();

            while (SnakeFoods3.Any(snakeFood => snakeFood.X == newFood.X && snakeFood.Y == newFood.Y))
            {
                newFood = GenerateRandomFood();
            }

            SnakeFoods3.Add(newFood);

        }

        private void GenerateSnakeFood4()
        {
            Square newFood = GenerateRandomFood();

            while (SnakeFoods4.Any(snakeFood => snakeFood.X == newFood.X && snakeFood.Y == newFood.Y))
            {
                newFood = GenerateRandomFood();
            }

            SnakeFoods4.Add(newFood);

        }

        private Square GenerateRandomFood()
        {
            int maxXPosition = pictureBox1.Size.Width / Player1setting.Width;
            int maxYPosition = pictureBox1.Size.Height / Player1setting.Height;

            return new Square { X = random.Next(0, maxXPosition), Y = random.Next(0, maxYPosition) };
        }


        private void EatFood()
        {
            if (SnakeFoods1.Any(s => s.X == SnakeBoddy[0].X && s.Y == SnakeBoddy[0].Y))
            {
                Square foodBody = new Square
                {
                    X = SnakeBoddy[SnakeBoddy.Count - 1].X,
                    Y = SnakeBoddy[SnakeBoddy.Count - 1].Y
                };

                SnakeBoddy.Add(foodBody);
                Player1setting.GameScore += FoodSetting.food1;
                label2.Text = Player1setting.GameScore.ToString();
                var snakePosition = SnakeBoddy[0];
                SnakeFoods1.RemoveAll(s => s.X == snakePosition.X && s.Y == snakePosition.Y);
            }
            if (SnakeFoods1.Any(s => s.X == SnakeBoddy2[0].X && s.Y == SnakeBoddy2[0].Y))
            {
                Square foodBody2 = new Square
                {
                    X = SnakeBoddy2[SnakeBoddy2.Count - 1].X,
                    Y = SnakeBoddy2[SnakeBoddy2.Count - 1].Y
                };

                SnakeBoddy2.Add(foodBody2);
                Player2setting.GameScore2 += FoodSetting.food1;
                label3.Text = Player2setting.GameScore2.ToString();
                var snakePosition = SnakeBoddy2[0];
                SnakeFoods1.RemoveAll(s => s.X == snakePosition.X && s.Y == snakePosition.Y);
            }
        }

        private void EatFood2()
        {
            if (SnakeFoods2.Any(s => s.X == SnakeBoddy[0].X && s.Y == SnakeBoddy[0].Y))
            {
                Square foodBody3 = new Square
                {
                    X = SnakeBoddy[SnakeBoddy.Count - 1].X,
                    Y = SnakeBoddy[SnakeBoddy.Count - 1].Y
                };

                SnakeBoddy.Add(foodBody3);

                Square foodBody35 = new Square
                {
                    X = SnakeBoddy[SnakeBoddy.Count - 1].X,
                    Y = SnakeBoddy[SnakeBoddy.Count - 1].Y
                };

                SnakeBoddy.Add(foodBody35);
                Player1setting.GameScore += FoodSetting.food2;
                label2.Text = Player1setting.GameScore.ToString();
                var snakePosition = SnakeBoddy[0];
                SnakeFoods2.RemoveAll(s => s.X == snakePosition.X && s.Y == snakePosition.Y);
            }
            if (SnakeFoods2.Any(s => s.X == SnakeBoddy2[0].X && s.Y == SnakeBoddy2[0].Y))
            {
                Square foodBody4 = new Square
                {
                    X = SnakeBoddy2[SnakeBoddy2.Count - 1].X,
                    Y = SnakeBoddy2[SnakeBoddy2.Count - 1].Y
                };
                SnakeBoddy2.Add(foodBody4);

                Square foodBody45 = new Square
                {
                    X = SnakeBoddy2[SnakeBoddy2.Count - 1].X,
                    Y = SnakeBoddy2[SnakeBoddy2.Count - 1].Y
                };

                SnakeBoddy2.Add(foodBody45);
                Player2setting.GameScore2 += FoodSetting.food2;
                label3.Text = Player2setting.GameScore2.ToString();
                var snakePosition = SnakeBoddy2[0];
                SnakeFoods2.RemoveAll(s => s.X == snakePosition.X && s.Y == snakePosition.Y);
            }
        }

        private void EatFood3()
        {
            if (SnakeFoods3.Any(s => s.X == SnakeBoddy[0].X && s.Y == SnakeBoddy[0].Y))
            {
                if (SnakeBoddy.Count > 1)
                {
                    SnakeBoddy.RemoveAt(SnakeBoddy.Count - 1);
                }
                Player1setting.GameScore += FoodSetting.food3;
                label2.Text = Player1setting.GameScore.ToString();
                var snakePosition = SnakeBoddy[0];
                SnakeFoods3.RemoveAll(s => s.X == snakePosition.X && s.Y == snakePosition.Y);
            }
            if (SnakeFoods3.Any(s => s.X == SnakeBoddy2[0].X && s.Y == SnakeBoddy2[0].Y))
            {
                if (SnakeBoddy2.Count > 1)
                {
                    SnakeBoddy2.RemoveAt(SnakeBoddy2.Count - 1);
                }
                Player2setting.GameScore2 += FoodSetting.food3;
                label3.Text = Player2setting.GameScore2.ToString();
                var snakePosition = SnakeBoddy2[0];
                SnakeFoods3.RemoveAll(s => s.X == snakePosition.X && s.Y == snakePosition.Y);
            }
        }


        private void EatFood4()
        {
            if (SnakeFoods4.Any(s => s.X == SnakeBoddy[0].X && s.Y == SnakeBoddy[0].Y))
            {
                Player1setting.GameScore += FoodSetting.food4;
                label2.Text = Player1setting.GameScore.ToString();
                var snakePosition = SnakeBoddy[0];
                SnakeFoods4.RemoveAll(s => s.X == snakePosition.X && s.Y == snakePosition.Y);
                RandomEevent();
            }

            if (SnakeFoods4.Any(s => s.X == SnakeBoddy2[0].X && s.Y == SnakeBoddy2[0].Y))
            {
                Player2setting.GameScore2 += FoodSetting.food4;
                label3.Text = Player2setting.GameScore2.ToString();
                var snakePosition = SnakeBoddy2[0];
                SnakeFoods4.RemoveAll(s => s.X == snakePosition.X && s.Y == snakePosition.Y);
                RandomEevent();
            }
        }

        private void RandomEevent()
        {
            var time = DateTime.Now;
            eventTimer.Tick += new EventHandler(TimerEventProcessor);
            eventTimer.Interval = 3000;
            eventTimer.Start();
            while (exitFlag == false)
            {
                //Application.DoEvents();
                if ((DateTime.Now - time).TotalSeconds > 30)
                {
                    eventTimer.Stop();
                    exitFlag = true;
                }
            }
        }

        private void TimerEventProcessor(Object sender, EventArgs e)
        {
            eventTimer.Enabled = true;
            Player2setting.Movment2 = (SnakeMovment2)random.Next(4);
        }

        private void FoodEvent() 
        {
            foodTimer.Tick += new EventHandler(FoodGenerator);
            foodTimer.Interval = random.Next(1000,4000);
            foodTimer.Start();
            foodTimer2.Tick += new EventHandler(FoodGenerator2);
            foodTimer2.Interval = random.Next(4000, 7000);
            foodTimer2.Start();
            foodTimer3.Tick += new EventHandler(FoodGenerator3);
            foodTimer3.Interval = random.Next(5000, 10000);
            foodTimer3.Start();
            foodTimer4.Tick += new EventHandler(FoodGenerator4);
            foodTimer4.Interval = random.Next(3000, 6000);
            foodTimer4.Start();
        }

        private void FoodGenerator(object sender, EventArgs e)
        {
            foodTimer.Enabled = true;
            GenerateSnakeFood();
        }

        private void FoodGenerator2(object sender, EventArgs e)
        {
            foodTimer2.Enabled = true;
            GenerateSnakeFood2();
        }

        private void FoodGenerator3(object sender, EventArgs e)
        {
            foodTimer3.Enabled = true;
            GenerateSnakeFood3();
        }

        private void FoodGenerator4(object sender, EventArgs e)
        {
            foodTimer4.Enabled = true;
            GenerateSnakeFood4();
        }

        private void Die()
        {
            Player1setting.Gameover = true;
        }

        private void Die2()
        {
            Player2setting.Gameover2 = true;
        }

        private void GameStart_Click(object sender, EventArgs e)
        {
            if (choice1.Checked || choice2.Checked || choice3.Checked) {
                StartGame();
            }

        }
    }
}

KeyInput.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Collections;
using System.Windows.Forms;

namespace snakeGame3players
{
    class KeyInput
    {

        private static Hashtable keyTable = new Hashtable();

        public static bool KeyInputs(Keys key)
        {
            if (keyTable[key] == null)
            {
                return false;
            }

            return (bool)keyTable[key];
        }

        public static void SnakeDirections(Keys key, bool direction)
        {
            keyTable[key] = direction;
        }

    }
}

Player1setting.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace snakeGame3players
{
    public enum SnakeMovment
    {
        Right,
        Left,
        Down,
        Up
    };

    public enum SnakeMovment2
    {
        W,
        S,
        D,
        A
    };


    class Player1setting
    {
        public static int Width { get; set; }
        public static int Height { get; set; }
        public static int GameScore { get; set; }
        public static bool Gameover { get; set; }
        public static SnakeMovment Movment { get; set; }

        public Player1setting()
        {
            Height = 16;
            Width = 16;
            Gameover = false;
            Movment = SnakeMovment.Down;
            GameScore = 0;
        }

    }

    class Player2setting
    {
        public static int Width2 { get; set; }
        public static int Height2 { get; set; }
        public static int GameScore2 { get; set; }
        public static bool Gameover2 { get; set; }
        public static SnakeMovment2 Movment2 { get; set; }

        public Player2setting()
        {
            Height2 = 16;
            Width2 = 16;
            Gameover2 = false;
            Movment2 = SnakeMovment2.S;
            GameScore2 = 0;
        }

    }
    class FoodSetting
    {
        public static int food1 { get; set; }
        public static int food2 { get; set; }
        public static int food3 { get; set; }
        public static int food4 { get; set; }

        public FoodSetting()
        {
            food1 = 1;
            food2 = 5;
            food3 = 1;
            food4 = 1;
        }
    }

}

Square.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace snakeGame3players
{
    class Square
    {
        public int X { get; set; }
        public int Y { get; set; }

        public Square()
        {
            X = 0;
            Y = 0;
        }
    }
}

Program.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace snakeGame3players
{
    static class Program
    {
        /// <summary>
        /// The main entry point for the application.
        /// </summary>
        [STAThread]
        static void Main()
        {
            Application.EnableVisualStyles();
            Application.SetCompatibleTextRenderingDefault(false);
            Application.Run(new Form1());
        }
    }
}
\$\endgroup\$
6
  • 3
    \$\begingroup\$ Are you sure that you understand the OOP principles? 700+ lines of Form1 class says you don't. \$\endgroup\$ – Peter Csala Mar 12 at 7:49
  • \$\begingroup\$ From what aspect are you looking for a review? \$\endgroup\$ – Peter Csala Mar 17 at 7:19
  • \$\begingroup\$ I just want to improve it, it can be just one function private void UpdateScreen(object sender, EventArgs e) for an example to how I would do it if I was following OOP standards \$\endgroup\$ – zellez Mar 17 at 14:39
  • 1
    \$\begingroup\$ I've left a quite lengthy answer. Please check it. I hope it helps you. \$\endgroup\$ – Peter Csala Mar 17 at 17:21
  • \$\begingroup\$ To mark the answer as accepted please check the label to the left from the answer. In case it was helpful. \$\endgroup\$ – aepot Mar 25 at 21:07
2
+100
\$\begingroup\$

OP has stated that (s)he is interested mainly about the UpdateScreen.
So, in this review I try to focus only on that part of the application.

First, let me show you the end result and then let me iterate through the changes one-by-one:

private void UpdateScreen(object sender, EventArgs e)
{
    if (label1.Visible)
    {
        if (KeyInput.IsKeyDown(Keys.Q))
        {
            Application.Restart();
            Environment.Exit(0);
        }
    }
    if (choice2.Checked)
    {
        playerOne.UpdateMovement();
        UpdateSnakeBoddy();
    }

    pictureBox1.Invalidate();


    if (choice1.Checked)
    {
        playerTwo.UpdateMovement();
        UpdateSnakeBoddy2();
    }

    pictureBox1.Invalidate();
}

Use instances

  • In your original solution you have a Player1setting class with static properties.
  • In case of OOP you should work on a particular instance not on a global one.

So, lets get rid of the static keywords:

class Player1setting
{
    public int Width { get; set; }
    public int Height { get; set; }
    public int GameScore { get; set; }
    public bool Gameover { get; set; }
    public SnakeMovment Movment { get; set; }

    public Player1setting()
    {
        Height = 16;
        Width = 16;
        Gameover = false;
        Movment = SnakeMovment.Down;
        GameScore = 0;
    }
}

Then create instances:

//Class-wide private members
private readonly Player1setting playerOne;
private readonly Player2setting playerTwo;

public Form1()
{
    InitializeComponent();
    playerOne = new Player1setting();
    playerTwo = new Player2setting();
    ...
}

And use them. So, we have to change all Player1setting. and Player2setting. to playerOne. and playerTwo.. For example:

if (KeyInput.KeyInputs(Keys.Right) && playerOne.Movment != SnakeMovment.Left)
{
    playerOne.Movment = SnakeMovment.Right;
}

Use better naming

In the comments you can find the original

class PlayerOneSettings //Orig: Player1setting
{
    public int Width { get; set; }
    public int Height { get; set; }
    public int GameScore { get; set; }
    public bool IsGameOver { get; set; } //Orig: Gameover
    public SnakeMovement Movement { get; set; } //Orig: SnakeMovment Movment
    ...
}

Please also rename the following methods:

  • PlayerMovment to UpdateSnakeBoddy
  • PlayerMovment2 to UpdateSnakeBoddy2

In my opinion these names are more expressive because in those methods you are changing mainly those collections.

Encapsulate logic

The change of the Movement should reside inside the Player{XYZ}Settings classes.
So, let's introduce an UpdateMovement method:

class PlayerOneSettings
{
    ...
    public void UpdateMovement()
    {
        if (Player1setting.Gameover == false)
        {
            if (KeyInput.KeyInputs(Keys.Right) && Movement != SnakeMovement.Left)
            {
                Movement = SnakeMovement.Right;
            }

            else if (KeyInput.KeyInputs(Keys.Left) && Movement != SnakeMovement.Right)
            {
                Movement = SnakeMovement.Left;
            }

            else if (KeyInput.KeyInputs(Keys.Up) && Movement != SnakeMovement.Down)
            {
                Movement = SnakeMovement.Up;
            }

            else if (KeyInput.KeyInputs(Keys.Down) && Movement != SnakeMovement.Up)
            {
                Movement = SnakeMovement.Down;
            }
        }
    }
}

Here I can access the class members (like Movement) so I don't need to use player1 prefix.
We can also restrict the access of the Movement's setter:

public SnakeMovement Movement { get; private set; }

Now the UpdateScreen should look like as it is was stated above.

Reuse available components

The SnakeMovement contains a subset of the Keys enumeration.
Do not reinvent the wheel, try to re-use existing components.

class PlayerOneSettings
{
    ...
    private readonly Keys[] ValidKeys = new[] { Keys.Right, Keys.Left, Keys.Up, Keys.Down };
}

Also we can define counterparts for each key:

class PlayerOneSettings
{
    ...
    private readonly Keys[] ValidKeys = new[] { Keys.Right, Keys.Left, Keys.Up, Keys.Down };
    private readonly Dictionary<Keys, Keys> KeyCounterpartMapping = new Dictionary<Keys, Keys>
    {
        { Keys.Up, Keys.Down },
        { Keys.Down, Keys.Up },
        { Keys.Right, Keys.Left },
        { Keys.Left, Keys.Right },
    };
}

With these in our hand we can greatly simplify the UpdateMovement:

class PlayerOneSettings
{
    ...
    public void UpdateMovement()
    {
        if (IsGameOver) return;

        foreach (var key in ValidKeys)
        {
            var counterpart = KeyCounterpartMapping[key];
            if (KeyInput.KeyInputs(key) && Movement != counterpart)
            {
                Movement = key;
                break;
            }
        }
    }
}

With this modification the data and the operation is separated cleanly.

Use inheritance

The PlayerOneSettings and PlayerTwoSettings have a huge resembles, which is not a coincidence.
So, it would be wise to define a base class and inherit from it:

abstract class PlayerSettings
{
    public int Width { get; set; }
    public int Height { get; set; }
    public int GameScore { get; set; }
    public bool IsGameOver { get; set; }
    public Keys Movement { get; protected set; } //Orig: private

    protected abstract Keys[] ValidKeys { get; }
    protected abstract Dictionary<Keys, Keys> KeyCounterpartMapping { get; }

    public void UpdateMovement()
    {
        if (IsGameOver) return;

        foreach (var key in ValidKeys)
        {
            var counterpart = KeyCounterpartMapping[key];
            if (KeyInput.IsKeyDown(key) && Movement != counterpart)
            {
                Movement = key;
                break;
            }
        }
    }
}
  • I've changed the Movement's setter's accessibility from private to protected to be able to set it in the constructors of the derived classes.
  • Because fields can't be virtual or abstract that's why I used properties for ValidKeys and KeyCounterpartMapping.
  • I used abstract to enforce override in the derived classes:
class PlayerOneSettings: PlayerSettings
{
    public PlayerOneSettings()
    {
        Height = 16;
        Width = 16;
        IsGameOver = false;
        Movement = Keys.Down;
        GameScore = 0;
    }

   protected override Keys[] ValidKeys { get; } = new[] { Keys.Right, Keys.Left, Keys.Up, Keys.Down };

   protected override Dictionary<Keys, Keys> KeyCounterpartMapping { get; } = new Dictionary<Keys, Keys>
   {
       { Keys.Up, Keys.Down },
       { Keys.Down, Keys.Up },
       { Keys.Right, Keys.Left },
       { Keys.Left, Keys.Right },
   };
}
class PlayerTwoSettings: PlayerSettings
{
    public PlayerTwoSettings()
    {
        Height = 16;
        Width = 16;
        IsGameOver = false;
        Movement = Keys.S;
        GameScore = 0;
    }

    protected override Keys[] ValidKeys { get; } = new[] { Keys.W, Keys.S, Keys.D, Keys.A };

    protected override Dictionary<Keys, Keys> KeyCounterpartMapping { get; } = new Dictionary<Keys, Keys>
    {
        { Keys.D, Keys.A },
        { Keys.A, Keys.D },
        { Keys.W, Keys.S },
        { Keys.S, Keys.W },
    };
}

With these the derived classes only contains the deviations.
All the common stuff are inherited.

\$\endgroup\$
8
  • \$\begingroup\$ Sorry it was a copy-paste issue. Thanks I fix it. \$\endgroup\$ – Peter Csala Mar 17 at 19:06
  • \$\begingroup\$ @zellez Health should be your number one priority. Everything else could wait. After you are better, feel free to contact me and I'll try to help here. \$\endgroup\$ – Peter Csala Mar 29 at 7:52
  • \$\begingroup\$ @zellez I'm glad that you are better. As I stated in the review I've focused on the UpdateScreen method, not on the PlayerMovment. In my opinion the PlayerMovment has too much responsibility. If you want to move some part of this into a separate class then you have to remember data and behaviour should stick together. So, SnakeBoddy and related part of the PlayerMovment method should reside inside the same class. Food related updates should belong to another separate class. Does it make sense to you? \$\endgroup\$ – Peter Csala Apr 7 at 9:18
  • \$\begingroup\$ The collision detection could belong to the same class as the movement handling, because it operates on the same data. The reaction part could be separated into its on class if it operates on other data as well. \$\endgroup\$ – Peter Csala Apr 7 at 9:45
  • \$\begingroup\$ I know it is shameless, but I can't seem to get it to work. I know it has been a while since you have viewed the code, but if could you show me the proper way it doesn't need to be explained, in detail, or anything like that. I could sit and study the code and try to figure out why on my own. I just lack the basic logic to do it, and if you don't want to help me further, it is understandable, and I wish you a nice day. \$\endgroup\$ – zellez Apr 12 at 10:02
3
\$\begingroup\$

In this post I'll focus on the PlayerMovment. Before I try to guide you through the OOP related transformations, let me show you a couple of refactoring techniques which will help you to dramatically reduce the complexity of this method.

Guard expression vs Early exit

Before

private void PlayerMovment()
{
    if (choice2.Checked)
    {
        for (...)
        {
          ...
        }
    }
}

After

private void PlayerMovment()
{
    if (!choice2.Checked) return;
    
    for (...)
    {
        ...
    }
}
  • Instead of guarding the whole operation, do some preliminary checks and if they fail then early exit
  • This will help you streamline your code and reduce the indentation

Splitting logic

Before

for (int i = SnakeBoddy.Count - 1; i >= 0; i--)
{
    if (i == 0)
    {
       //DO a lots of stuff
    }
    else
    { 
      //DO a simple stuff
    }
}

After

//Move each part of the snake body forward (without head) 
for (int i = SnakeBoddy.Count - 1; i > 0; i--)
{
    //DO a simple stuff
}

//DO a lots of stuff
  • In your reverse iteration I changed the exit condition from i >= 0 to i > 0
    • With this modification I could eliminate the branching logic
  • So, the for loop now has that code which was inside the else block
    • In other words we move each part of the snake body forward (without head)

Don't repeat yourself #1

Before

switch (Player1setting.Movment)
{
    case SnakeMovment.Right:
        SnakeBoddy[i].X++;
        break;

    case SnakeMovment.Left:
        SnakeBoddy[i].X--;
        break;

    case SnakeMovment.Up:
        SnakeBoddy[i].Y--;
        break;

    case SnakeMovment.Down:
        SnakeBoddy[i].Y++;
        break;
}

After

//Adjust head
var head = SnakeBoddy[0];
head.X += Player1setting.Movment == SnakeMovment.Right ? 1
        : Player1setting.Movment == SnakeMovment.Left ? -1 : 0;

head.Y += Player1setting.Movment == SnakeMovment.Down ? 1
        : Player1setting.Movment == SnakeMovment.Up ? -1 : 0;
  • One can argue with this modification that it does not improve legibility
    • I can agree with them because using two nested conditional operators (?:) might not be easily readable
    • But logic is quite simple
      • Increase X with 1 if user pressed Right
      • Increase X with -1 if user pressed Left
      • Increase X with 0 otherwise
  • I've also introduced a variable head because you have referred to the same element as SnakeBoddy[0] and sometimes as SnakeBoddy[i] where i is guaranteed to be 0

Don't repeat yourself #2

Before

for (int J = 1; J < SnakeBoddy.Count; J++)
{
    if (SnakeBoddy[i].X == SnakeBoddy[J].X && SnakeBoddy[i].Y == SnakeBoddy[J].Y)
    {
        Die();
    }
}
for (int g = 0; g < SnakeBoddy2.Count; g++)
{
    if (SnakeBoddy[0].X == SnakeBoddy2[g].X && SnakeBoddy[0].Y == SnakeBoddy2[g].Y)
    {
        Player1setting.GameScore += FoodSetting.food2;
        label3.Text = Player1setting.GameScore.ToString();
        Die();
    }
}

After

//Check whether the snake has collided with itself or with the other snake
foreach (var body in SnakeBoddy.Union(SnakeBoddy2))
{
    if (head.X == body.X && head.Y == body.Y)
    {
        Die();
        break;
    }
}
  • Here you can make use of the foreach, you don't have to use for
  • You have done the same operation against SnakeBoddy and SnakeBoddy2
    • With LINQ's Union you can easily merge these two iterations
  • Once you have detected collision then you do not need to iterate through the rest of the items that's why you can break out from the loop
    • Or even return from the PlayerMovment
  • I do believe that GameScore and label3 updates should not belong here that's why I removed from my code
    • This should be handled on the caller side

Separate data and operation

Before

if (SnakeFoods1.Any(s => s.X == SnakeBoddy[0].X && s.Y == SnakeBoddy[0].Y))
{
    EatFood();
}

if (SnakeFoods2.Any(s => s.X == SnakeBoddy[0].X && s.Y == SnakeBoddy[0].Y))
{
    EatFood2();
}

if (SnakeFoods3.Any(s => s.X == SnakeBoddy[0].X && s.Y == SnakeBoddy[0].Y))
{
    EatFood3();
}

if (SnakeFoods4.Any(s => s.X == SnakeBoddy[0].X && s.Y == SnakeBoddy[0].Y))
{
    EatFood4();
}

After

//Eat all food which was in its way
var allFood = new Dictionary<List<Square>, Action> {
    { SnakeFoods1, EatFood  },
    { SnakeFoods2, EatFood2 },
    { SnakeFoods3, EatFood3 },
    { SnakeFoods4, EatFood4 }
};
foreach(var foodsAndEatMapping in allFood)
{
    if (foodsAndEatMapping.Key.Any(s => s.X == head.X && s.Y == head.Y))
    {
        foodsAndEatMapping.Value();
    }
}
  • Yet again you have performed the same operation against different datasets
    • By separating operation from data you can avoid repetition
  • Here I've created a mapping (called allFood) where I have mapped the food source collection to the related EatFood function
    • I did not want to spent time to generalize the EatFood{XYZ} methods but that should your homework

The refactored PlayerMovment

private void PlayerMovment()
{
    if (!choice2.Checked) return;

    //Move each part of the snake body forward (without head) 
    for (int i = SnakeBoddy.Count - 1; i > 0; i--)
    {
        SnakeBoddy[i].X = SnakeBoddy[i - 1].X;
        SnakeBoddy[i].Y = SnakeBoddy[i - 1].Y;
    }

    //Adjust head
    var head = SnakeBoddy[0];
    head.X += Player1setting.Movment == SnakeMovment.Right ? 1
            : Player1setting.Movment == SnakeMovment.Left ? -1 : 0;

    head.Y += Player1setting.Movment == SnakeMovment.Down ? 1
            : Player1setting.Movment == SnakeMovment.Up ? -1 : 0;

    //Check whether the snake has moved out of the arena
    int maxXPosition = pictureBox1.Size.Width / Player1setting.Width;
    int maxYPosition = pictureBox1.Size.Height / Player1setting.Height;

    if (head.X < 0 || head.X > maxXPosition ||
        head.Y < 0 || head.Y >= maxYPosition)
        Die();

    //Check whether the snake has collided with itself or with the other snake
    foreach (var body in SnakeBoddy.Union(SnakeBoddy2))
    {
        if (head.X == body.X && head.Y == body.Y)
        {
            Die();
            break;
        }
    }

    //Eat all food which was in its way
    var allFood = new Dictionary<List<Square>, Action> {
        { SnakeFoods1, EatFood  },
        { SnakeFoods2, EatFood2 },
        { SnakeFoods3, EatFood3 },
        { SnakeFoods4, EatFood4}
    };
    foreach(var foodsAndEatMapping in allFood)
    {
        if (foodsAndEatMapping.Key.Any(s => s.X == head.X && s.Y == head.Y))
        {
            foodsAndEatMapping.Value();
        }
    }
}
  • The same transformation should be applied to PlayerMovment2

Now let's start to focus on the OOP stuff.

Basics

First we have to identify what sort of components do we have.
Then we have to identify what can we do with them.

Now, let's just list the comments here (which I have placed in the refactored version):

//Move each part of the snake body forward (without head) 

//Adjust head

//Check whether the snake has moved out of the arena

//Check whether the snake has collided with itself or with the other snake

//Eat all food which was in its way

Nouns

To identify object let's focus on the nouns:

  • Snake
    • Body
    • Head
  • Arena
  • Food

As you can see our primary object could be the Snake, which encapsulates Body and Head. Depending on the operations Body and Head may or may not be public

  • If we want to perform operations on Body and Head at same time then most probably we don't need to expose them. They can become implementation details from the consumer point of view.
  • If we want to allow separate query operations against Body and Head then we should expose only readonly access. In other words the modification methods remain private to the object.

The Arena is related to pictureBox1 and Player1setting. If they are not changing over time then the Arena should not depend on them, just initialize it with the related values. Yet again exposing its members depends on the exposed behaviours. We will talk about this in the verbs section.

And finally the Food, which is more precisely a set of food. So, here the object could be Foods or FoodManager or what so ever.

Verbs

To identify behaviours let's focus on the verbs:

  • Snake
    • Body
      • Move
    • Head
      • Adjust
      • Check collision
  • Arena
    • Check boundary
  • Food
    • Eat

As you can the Move and Adjust operates on the snake's body and head. If we want to perform them together then we should not separate them.

public class Snake
{
   private List<Square> WholeBody {get; set;}
   private Square Head => WholeBody[0];
   private IEnumerable<Square> Body => WholeBody.Skip(1);
   
   public Snake(...)
   {
      //TODO: Initialize snake's internals
   }

   public void MoveForward(int verticalMove, int horizontalMove)
   {
      AdjustHead(verticalMove, horizontalMove);
      MoveBody();
   }

   private void MoveBody()
   {
      //TODO: perform operation against Body
   }

   private AdjustHead(int verticalMove, int horizontalMove)
   {
      //TODO: perform operation against Head
   }
}

The CheckCollision can be performed against the Snake instance itself or against any other instance. So we can create two separate methods to cover both cases:

public Snake
{
   ...
   public bool HaveIBittenMyself()
   {
      //TODO: Check whether head is collided with any part of the body
   }

   public bool HasOtherBittenMe(Square headOfOtherSnake)
   {
      //TODO: Check whether the other's head is collided with any part of the wholeBody
   }
}
  • As you can see the Snake itself does not know about the rest of the world. It can perform operations on its own data. This is crucial in case of OOP.

I think now you have the flavour so I'll leave Arena and Foods as an exercise for yourself. I hope this lengthy post helped you a bit.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.