patternMinor
Excel display driver class for Tetris
Viewed 0 times
excelfordriverdisplayclasstetris
Problem
This is the first of (hopefully) several posts as I reimplement an ancient (and unfortunately long lost) Excel VBA Tetris clone. The main workhorse class in the game engine is the display driver, which uses a rectangular area of a Worksheet to emulate a monitor. The display is sprite based, so the class holds a Dictionary loaded with IDrawables, which are basically bitmaps. The interface is as follows:
Properties
Methods
'IDrawable.cls
Option Explicit
Public Property Get Top() As Long
End Property
Public Property Let Top(inValue As Long)
End Property
Public Property Get Left() As Long
End Property
Public Property Let Left(inValue As Long)
End Property
Public Property Get Width() As Long
End Property
Public Property Get Height() As Long
End Property
Public Property Let Bitmap(colors() As Long)
End Property
Public Property Get Bitmap() As Long()
End Property
Public Function GetPixel(pixelRow As Long, pixelColumn As Long) As Long
End FunctionProperties
AnchorCell - This is a one cell Range that determines where the display rectangle will be placed. It will raise an error if more than one cell is passed.Top and Left are the row and column number of the AnchorCell in the parent Worksheet's coordinates. These are read-only and are derived from the AnchorCell.Width and Height set the dimensions of the display.DotPitch is the size of each "pixel", and the rows and columns in the display range are set to this width and height (in pixels). Currently I have it limited to CGA resolution (mainly for convenience when I test it), but the limits can be set by altering the appropriate constants.BackColor sets the background color for the display. Any pixel in an IDrawable that matches the BackColor will be "transparent".Methods
AddDrawable, RemoveDrawable and ClearDrawables are the main methods for interacting with the display. Any item added to the container will be rendered.Refresh forces the display to repaint itself. If changeSolution
First let me say that this is pure awesome, and I only have superficial improvements to suggest. nevermind, this grew bigger than I originally thought.... again.
There is no reason for any of the parameters (well, except the array one) you're passing any of the
Could be:
While we're at it, might as well make the
In the
But this would work just as well:
Actually, I think the
That would change this client code:
Into that one-liner:
The
Then this:
Becomes this:
There is no reason for any of the parameters (well, except the array one) you're passing any of the
IDrawable interface members to be passed ByRef - and they're all passed by reference, implicitly.Public Property Let Top(inValue As Long)
End Property
Public Property Let Left(inValue As Long)
End Property
Public Function GetPixel(pixelRow As Long, pixelColumn As Long) As Long
End FunctionCould be:
Public Property Let Top(ByVal inValue As Long)
End Property
Public Property Let Left(ByVal inValue As Long)
End Property
Public Function GetPixel(ByVal pixelRow As Long, ByVal pixelColumn As Long) As Long
End FunctionWhile we're at it, might as well make the
colors() array explicitly passed ByRef:Public Property Let Bitmap(ByRef colors() As Long)
End PropertyIn the
Sprite class (I know, it's test/demo code), there's not really a need to expose any public members - in fact the only reason you do need them is because you've declared foo As Sprite:Dim foo As Sprite
Set foo = New SpriteBut this would work just as well:
Dim foo As IDrawable
Set foo = New SpriteActually, I think the
IDrawable interface should not expose Property Let members to mutate Top and Left values (but leave them on the concrete implementations... or don't - and give it a PredeclaredId and a Create function instead). It could expose some Move procedure instead of mutators:Public Sub Move(ByVal x As Long, ByVal y As Long)
End SubThat would change this client code:
If right Then
.Left = .Left + 1
Else
.Left = .Left - 1
End If
If down Then
.Top = .Top + 1
Else
.Top = .Top - 1
End IfInto that one-liner:
.Move IIf(right, 1, -1), IIf(down, 1, -1)The
Refresh method feels a bit cluttered, because it's responsible for drawing every pixel of every drawable object. You could have some DrawableObject with a PredeclaredId, and a "static" method where you could draw one single IDrawable object:Public Sub Draw(ByRef drawable As IDrawable, ByRef screen As Range)
Dim r As Long
Dim c As Long
Dim rowTarget As Long
Dim colTarget As Long
For r = 1 To drawable.Height
rowTarget = r + drawable.Top
If rowTarget >= 1 And rowTarget = 1 And colTarget < this.Width Then
screen.Cells(rowTarget, colTarget).Interior.Color = drawable.GetPixel(r, c)
End If
Next
End If
Next
End SubThen this:
With this.DisplayArea
.Interior.Color = this.BackColor
Dim drawable As Variant
For Each drawable In this.Drawables.Keys
Dim r As Long
Dim c As Long
Dim rowTarget As Long
Dim colTarget As Long
For r = 1 To drawable.Height
rowTarget = r + drawable.Top
If rowTarget >= 1 And rowTarget = 1 And colTarget < this.Width Then
.Cells(rowTarget, colTarget).Interior.Color = drawable.GetPixel(r, c)
End If
Next
End If
Next
Next
End WithBecomes this:
With this.DisplayArea
.Interior.Color = this.BackColor
Dim drawable As Variant 'shame this can't be IDrawable.. right?
For Each drawable In this.Drawables.Keys
DrawableObject.Draw drawable, .Range 'might need to cast to IDrawable
Next
End WithCode Snippets
Public Property Let Top(inValue As Long)
End Property
Public Property Let Left(inValue As Long)
End Property
Public Function GetPixel(pixelRow As Long, pixelColumn As Long) As Long
End FunctionPublic Property Let Top(ByVal inValue As Long)
End Property
Public Property Let Left(ByVal inValue As Long)
End Property
Public Function GetPixel(ByVal pixelRow As Long, ByVal pixelColumn As Long) As Long
End FunctionPublic Property Let Bitmap(ByRef colors() As Long)
End PropertyDim foo As Sprite
Set foo = New SpriteDim foo As IDrawable
Set foo = New SpriteContext
StackExchange Code Review Q#141404, answer score: 5
Revisions (0)
No revisions yet.