2
\$\begingroup\$

This is my first post here, so I apologize if anything is incorrect. I have created an endless arcade stacker game (like the stacker games at the arcade, but when you hit the top the grid resets and you continue endlessly). My intention in the code was to separate out the core game logic (i.e. the code that moves the stack, places it, and resets the grid) from the other logic (i.e. the speed at which the stack moves, the user input, the visual and sound effects) in order to have some sort of separation of concerns.

I have made an IStacker interface and a Stacker class that has been implemented the aforementioned interface. Would anyone be able to critique the code? In particular, any logic that could be simplified, and instances where adherence to SOLID principles could be improved.

Thanks for any help :)

I have included the classes below, but if it is any easier to view them on github I have also provided the links to those below:

https://github.com/impojr/Stacker/blob/main/Assets/Scripts/Core/IStacker.cs https://github.com/impojr/Stacker/blob/main/Assets/Scripts/Core/Stacker.cs

IStacker

using System.Collections.Generic;

namespace Assets.Scripts.Core
{
    public interface IStacker
    {
        void ResetGrid();
        void Tick();
        List<MissResult> Place();
        void ResetHeight();
    }
}

Stacker

using System.Collections.Generic;
using System.Linq;

namespace Assets.Scripts.Core
{
    public enum MovementDirection
    {
        Left,
        Right
    }

    public class Stacker : IStacker
    {
        /// <summary>
        /// The width of the grid. <b>Not</b> the width of the moving stack.
        /// </summary>
        public int Width { get; }

        /// <summary>
        /// The height of the grid.
        /// </summary>
        public int Height { get; }

        /// <summary>
        /// The playing grid. 
        /// </summary>
        public Square[,] Stack { get; private set; }

        /// <summary>
        /// The move direction of the stack.
        /// </summary>
        public MovementDirection MoveDir { get; private set; }

        /// <summary>
        /// The row in the grid where the stack is currently moving.
        /// </summary>
        public int ActiveRow { get; private set; }

        /// <summary>
        /// The width of the moving (current) stack.
        /// </summary>
        public int StackWidth { get; private set; }

        public Stacker(int width, int height, int initialStackWidth)
        {
            Width = width;
            Height = height;
            StackWidth = initialStackWidth;
            ResetGrid();
        }

        /// <summary>
        /// Resets the grid to empty.
        /// </summary>
        public void ResetGrid()
        {
            Stack = new Square[Width, Height];
            ActiveRow = 0;
            MoveDir = MovementDirection.Right;

            for (var i = 0; i < Width; i++)
            {  
                for (var j = 0; j < Height; j++)
                {
                    Stack[i, j] = new Square();
                }
            }
        }

        /// <summary>
        /// Move the stack in the grid.
        /// This will allow move rows if the grid was placed beforehand.
        /// </summary>
        public void Tick()
        {
            var cellsToSwitch = new List<int>();

            for (var i = 0; i < Width; i++)
            {
                if (Stack[i, ActiveRow].State == State.Occupied)
                {
                    cellsToSwitch.Add(i);
                }
            }

            // If there are no cells to switch, then we are starting a new row
            if (!cellsToSwitch.Any())
            {
                StartNewRow();
            }
            else
            {
                MoveStackInRow(cellsToSwitch);
            }
        }

        private void StartNewRow()
        {
            // If there is no active row, it is the start of the game
            // Start from left
            if (ActiveRow == 0)
            {
                FillRow(0);
            }
            // Otherwise, start row from previous stack
            else
            {
                var previousRow = ActiveRow - 1;
                var nextRowStart = 0;
                for (var i = 0; i < Width; i++)
                {
                    if (Stack[i, previousRow].State == State.Occupied)
                    {
                        nextRowStart = i;
                        break;
                    }
                }

                FillRow(nextRowStart);
            }
        }

        private void MoveStackInRow(List<int> cellsToSwitch)
        {
            // If we are going left and the first item in list is at the beginning of the row,
            // Change movement direction
            if (MoveDir == MovementDirection.Left)
            {
                if (cellsToSwitch.First() == 0)
                {
                    MoveDir = MovementDirection.Right;
                    MoveStack(cellsToSwitch, 1);
                }
                else
                {
                    MoveStack(cellsToSwitch, -1);
                }
            }
            else
            {
                // If we are going right and the last item in list at the end of the row,
                // Change movement direction
                if (cellsToSwitch.Last() == Width - 1)
                {
                    MoveDir = MovementDirection.Left;
                    MoveStack(cellsToSwitch, -1);
                }
                else
                {
                    MoveStack(cellsToSwitch, 1);
                }

            }
        }

        /// <summary>
        /// Moves stack left or right.
        /// </summary>
        /// <param name="cellsToSwitch">The column number of the cells that are being moved</param>
        /// <param name="increment">+1 or -1</param>
        private void MoveStack(IList<int> cellsToSwitch, int increment)
        {
            for (var i = 0; i < cellsToSwitch.Count; i++)
            {
                cellsToSwitch[i] += increment;
            }

            for (var i = 0; i < Width; i++)
            {
                Stack[i, ActiveRow].State = cellsToSwitch.Contains(i) ? State.Occupied : State.Vacant;
            }
        }

        /// <summary>
        /// Fills (makes squares occupied) in row in based on StackWidth
        /// </summary>
        /// <param name="startIndex">Where in the row filling starts.</param>
        private void FillRow(int startIndex)
        {
            for (var i = 0; i < StackWidth; i++)
            {
                Stack[i + startIndex, ActiveRow].State = State.Occupied;
            }
        }

        /// <summary>
        /// Places the stack and goes to the next row.
        /// </summary>
        /// <returns>A list of missed stack positions.</returns>
        public List<MissResult> Place()
        {
            var misses = new List<MissResult>();

            // If we are placing at the start of the stack there is no chance for a miss
            if (ActiveRow != 0)
            {
                for (var i = 0; i < Width; i++)
                {
                    if (Stack[i, ActiveRow].State != State.Occupied)
                        continue;

                    if (Stack[i, ActiveRow - 1].State != State.Vacant)
                        continue;

                    Stack[i, ActiveRow].State = State.Vacant;
                    misses.Add(new MissResult(i, ActiveRow));
                    StackWidth--;
                }
            }

            IncreaseActiveRow();

            return misses;
        }

        /// <summary>
        /// This will move the stack down so the game can continue endlessly.
        /// </summary>
        public void ResetHeight()
        {
            // Get the top row variables
            var highestRow = new Square[Width];
            for (var i = 0; i < Width; i++)
            {
                // The active row should be increased ever time a stack is placed,
                // So the highest row should be 1 below the current active row
                highestRow[i] = Stack[i, ActiveRow - 1];
            }

            ResetGrid();

            // The active row is now 0 after being reset
            for (var i = 0; i < Width; i++)
            {
                Stack[i, ActiveRow] = highestRow[i];
            }

            IncreaseActiveRow();
        }

        private void IncreaseActiveRow()
        {
            ActiveRow++;
        }
    }
}

MissResult

public class MissResult
{
    public int XPos { get; set; }
    public int YPos { get; set; }

    public MissResult(int x, int y)
    {
        XPos = x;
        YPos = y;
    }
}

Square

public enum State
{
    Occupied,
    Vacant
}

public class Square
{
    public State State { get; set; }

    public Square()
    {
        State = State.Vacant;
    }
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Stacker does not contain "grid", "stack" and game logic objects rather Stacker IS all of these these objects. Next, there is no such thing as a Grid (upper or lower case) - I don't see a grid defined anywhere. Next, I cannot tell the playing board from the moving "stack". Then calling that moving grid thing a Stack adds more confusion; the game is about stacking (hence creating a stack) grids(?) so a Stack can't be a single moving "grid"? \$\endgroup\$
    – radarbob
    Oct 25 at 3:39
  • \$\begingroup\$ @radarbob You are right, I've called the "grid" a "stack". It seems obvious now but I didn't make the connection when developing (I even used it in the summary... goodness me). I guess clearer variable names is one thing I can brush up on. Stacker holds the Stack, which should be called Grid. I have just realized I have left out two classes: the MissResult and Square - I will add those to the question too. \$\endgroup\$
    – impo
    Oct 25 at 4:23

Your Answer

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

Browse other questions tagged or ask your own question.