HiveBrain v1.2.0
Get Started
← Back to all entries
patternModerate

Racetrack in... VBA?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
racetrackvbastackoverflow

Problem

So, I took a look at the August Challenge:


The top-voted answer is
Racetrack1:
"In the game of
Racetrack2, cars race
around a track bounded by two concentric closed loops drawn on a
square grid. Implement a program that plays this game."


1Links to answer in meta

2Links to wikipedia article of game

and I thought, "hey, self, you should try that" - but given I only really know VBA, I did it in excel. Maybe that's not the intention of the challenge, but I did it anyway.

Experience it on github.

Note that you must have MS excel installed and macros enabled. I couldn't get it working on OSX.

This game is for two human players to use the form interface to move their cars around the board.

You start the game by hitting the reset button that triggers most of the code to run - you can also resume a game by activating the form

Sub Button1_Click()
    MsgBox ("This will create a new gameboard")
    Application.ScreenUpdating = False
    Range("A1:Z24").ClearContents
    Range("A1:Z24").ClearFormats
    CreateGrid
    FillOuterGrid
    FillInnerCircle
    StoreSpeed
    Application.ScreenUpdating = True
    Instruct.Show
    GameControl.Show
End Sub


The first thing to do is create the racetrack, which is semi-random each game -

```
Option Explicit
Sub CreateGrid()
'Store background color in a variable so that adjusting only takes one edit
Const BACKGROUND_COLOR As Long = vbBlack
'In the properties of my worksheet, I gave the WS object an inherent name (like Sheet8), but called it GameBoardSheet
With GameBoardSheet
.Name = "GameBoard"
Columns("B:Y").ColumnWidth = 2.14
Columns("A").ColumnWidth = 50
Columns("Z").ColumnWidth = 50
Rows(1).RowHeight = 100
Rows(24).RowHeight = 100
Range("A1:Z1").Merge
Range("A1").Interior.Color = BACKGROUND_COLOR
Range("A24:Z24").Merge
Range("A24").Interior.Color = BACKGROUND_COLOR
Range("A2:A23").Merge

Solution

Implicit worksheet references

Sub CreateGrid()
    ActiveSheet.Name = "GameBoard"
    Columns("B:Y").ColumnWidth = 2.14
    Columns("A").ColumnWidth = 50
    Columns("Z").ColumnWidth = 50
    Rows(1).RowHeight = 100
    Rows(24).RowHeight = 100
    Range("A1:Z1").Merge
    Range("A1").Interior.Color = vbBlack
    Range("A24:Z24").Merge
    Range("A24").Interior.Color = vbBlack
    Range("A2:A23").Merge
    Range("A2").Interior.Color = vbBlack
    Range("Z2:Z23").Merge
    Range("z2").Interior.Color = vbBlack
    Range("B2").Select
End Sub


While only the first line in this procedure mentions ActiveSheet, every single line in this procedure is referencing Application.ActiveSheet... implicitly. And references to the active sheet are always more or less flaky.

Tip: By turning off Application.ScreenUpdating while you're creating the grid, you'll eliminate that "flicker", and generate the grid even faster: user won't even blink.

CodeName

A better approach would be to give that worksheet a meaningful programmatic name. From your screenshot I can tell you have left it to its default, Sheet8; the value of Sheet.CodeName is a "free" identifier reference - VBA creates an identifier with it, and you can use that identifier in your code.

I'd rename it to GameBoardSheet and perhaps use a With block.

Also, at one point you might want to use another background color than vbBlack, and when that happens you'd rather make the change once:

Private Sub CreateGrid()
    Const BACKGROUND_COLOR As Long = vbBlack
    With GameBoardSheet
        .Name = "GameBoard"
        .Columns("B:Y").ColumnWidth = 2.14
        .Columns("A").ColumnWidth = 50
        .Columns("Z").ColumnWidth = 50
        .Rows(1).RowHeight = 100
        .Rows(24).RowHeight = 100
        .Range("A1:Z1").Merge
        .Range("A1").Interior.Color = BACKGROUND_COLOR
        '...
    End With
End Sub


One nice thing about naming GameBoardSheet, is that you can now do away with all these:

Sheets("GameBoard")


And simply refer to that "free" GameBoardSheet reference instead.

How much effort would it be to refactor your code and reimplement the UI as a panel of ActiveX controls instead of a form? It seems to me that would make a more "natural" UI:

The application logic shouldn't depend on a form (let alone be completely implemented behind a form), it should be encapsulated in its own class module.

Try making a brushed-up UI by hand; freeze panes just outside the bottom-right corner of your game screen, and protect the sheet so that all the user can do is interact with the buttons: a big green "Go!" button and 4 arrow buttons that toggle between red/off and blue/on for each direction. For the user this makes a nice abstraction over the -1/0/+1 directional speed:

  • If [Left] and [Right] are both the same color, X-speed is 0



  • If [Up] and [Down] are both the same color, Y-speed is 0



  • If [Left] is blue and [Right] is red, X-speed is 1



  • If [Right] is blue and [Left] is red, X-speed is -1



  • If [Up] is blue and [Down] is red, Y-speed is 1



  • If [Down] is blue and [Up] is red, Y-speed is -1



  • If the user doesn't toggle anything, previous turn's values are used



I'd make an ITrack interface with some Draw method:

Public Sub Draw()
End Sub


Then I could have an EasyTrack, a MediumTrack and a HardTrack, and move the FillOuterGrid and FillInnerCircle to private methods on, say, the EasyTrack class; Draw would call these two methods. Then the MediumTrack and HardTrack would draw different patterns.

You're saving the vectors on the spreadsheet itself. You know what? It's brilliant. You save the workbook, and you just saved your game! With global variables, not only you would have, well, [grabs nose] global variables... your game state would live-and-die with the program's execution, so you'd have to figure out a way to persist the vectors somewhere anyway, if you wanted to save the game state before quitting.

Code Snippets

Sub CreateGrid()
    ActiveSheet.Name = "GameBoard"
    Columns("B:Y").ColumnWidth = 2.14
    Columns("A").ColumnWidth = 50
    Columns("Z").ColumnWidth = 50
    Rows(1).RowHeight = 100
    Rows(24).RowHeight = 100
    Range("A1:Z1").Merge
    Range("A1").Interior.Color = vbBlack
    Range("A24:Z24").Merge
    Range("A24").Interior.Color = vbBlack
    Range("A2:A23").Merge
    Range("A2").Interior.Color = vbBlack
    Range("Z2:Z23").Merge
    Range("z2").Interior.Color = vbBlack
    Range("B2").Select
End Sub
Private Sub CreateGrid()
    Const BACKGROUND_COLOR As Long = vbBlack
    With GameBoardSheet
        .Name = "GameBoard"
        .Columns("B:Y").ColumnWidth = 2.14
        .Columns("A").ColumnWidth = 50
        .Columns("Z").ColumnWidth = 50
        .Rows(1).RowHeight = 100
        .Rows(24).RowHeight = 100
        .Range("A1:Z1").Merge
        .Range("A1").Interior.Color = BACKGROUND_COLOR
        '...
    End With
End Sub
Sheets("GameBoard")
Public Sub Draw()
End Sub

Context

StackExchange Code Review Q#100714, answer score: 15

Revisions (0)

No revisions yet.