Neat, I am amused by this little game! I'm impressed to see this from a returning beginner.
Hi Trucker, and welcome back to the world of code!
This is the sort of thing I am trying to teach my current 'disciple'... something simple and practical to learn the necessary skills required to make something more substantial.
It works just fine on my machine, and demonstrates exactly the right entry-level skills I believe are necessary to enter our world.
The first thing that jumps out at me is code readability. As a new re-entry into coding, I suggest that you start indenting your code iteratively. All code between a DO and its married LOOP should be intended one TAB. Take the last function of your code for example (CheckLocationClear). Indent everything between the DO and the LOOP with one TAB. Then, further indent everything between the IF and the ENDIF with a second TAB. It makes reading your code far easier.
Try to avoid the EXIT command. I'm going to teach this one in steps to make it easier to see why we're doing it:
In the same function CheckLocationClear(), clear a flag to 0 before the DO:
quitflag = 0
Now, replace the EXIT command with one to set the flag:
quitflag = 1
Then, between the ENDIF and the LOOP commands, abort the loop if the flag has been set, like so:
IF quitflag = 1 THEN EXIT
This gives you the same effect, but now we're using a flag to initiate the abort instead of doing it within an IF statement (which has never been ENDIF'd because you EXIT before we reach that command, creating stack problems later).
It shouldn't be difficult to understand the new logic I've set up here, but I wanted you to see the use of the flag before I move on to the next step:
Replace the DO...LOOP 'unconditional' iteration with a REPEAT...UNTIL 'conditional' iteration by changing the DO into REPEAT and the LOOP into UNTIL.
Now, the UNTIL command requires a condition upon which to abort the loop (which is why it is called a 'conditional' loop), so the line becomes:
UNTIL quitflag = 1
This is our final version of the function. It's much cleaner, avoids the stack problem of never ENDIF'ing an IF, and avoids the EXIT command entirely, which is a horrible, horrible command to get used to using for a billion reasons. For clarity, here is the final version of CheckLocationClear():
FUNCTION CheckLocationClear()
QuitFlag = 0
REPEAT
GridRow=RND(Rows-1)
GridColumn=RND(Columns-1)
IF Grid(GridColumn,GridRow)=0
Grid(GridColumn,GridRow)=1
QuitFlag = 1
ENDIF
UNTIL QuitFlag = 1
ENDFUNCTION
: After typing all that out, I see that you already know how to use REPEAT...UNTIL as you have done elsewhere in your code. Forgive my assumption otherwise, but I'm going to leave my explanation in this post for the benefit of others.
The moral of the story is, never use the EXIT command - always use a 'conditional' loop with a flag to determine when to abort.
The function Populate() uses an 'unspecific' FOR...NEXT loop. This is fine as it stands, but it's good practice to specify which FOR you're NEXTing in case you nest your loops in the future. Use:
NEXT Cycles
Instead of just:
NEXT
The effect is the same, but it's 'specific' now, which is just good practice and also makes it easier to read when you start nesting loops (loops within loops within loops).
In function PickColor(), you've made a logic error in your random number generation. There's no point in structuring this command the way you have:
R=RND(254+1)
Because the argument (254+1) is evaluated before function (RND) the 1 will be added to the 254 before the final result of 255 is used in the RND command. You may therefore just as well be using:
R=RND(255)
I can plainly see, however, that your intent is to avoid the selection of a zero value. The way to do that is thus:
R=RND(254) + 1
The addition needs to be outside the RND function so that the RND takes place before we add one to the result, otherwise a zero will always be possible. You also need to reduce your maximum value by one (we use 254 instead of 255) because we're going to add one to it afterwards, and we don't want a value of 256. This should be easy enough to follow.
It's a little more complicated, but you could reduce the entire function to a single line, one that eliminates the need for variable assignment:
INK RGB(RND(254) + 1, RND(254) + 1, RND(254) + 1), 0
If you don't immediately see why, that's ok, your original logic works just fine.
Now that we see how to use RND to avoid a zero, go back to CheckLocationClear() and have another look at your RND's. You're going to get zero results here sometimes. With the code the way it stands that doesn't matter, since zeroes are allowed as array indices and you're only using that array to check that a box hasn't already been matched to a pair (
really inefficient, but we'll get to that). This means that the box in the top left corner of the field is at array position 0, 0 and not at position 1, 1. That's well and good, as long as you're aware of it - I find a lot of people get confused by the first element of an array being 0 and not 1. If you ever use your Grid array for anything more than merely checking for duplicate pairs (such as storing what color the box is, which we'll do later), you'll need to remember that topleft is 0, 0 and the bottomright is Columns-1, Rows-1.
In ResetHint() I don't see the purpose of these lines:
SET CURSOR 0,0
PRINT " "
Immediately followed by:
SET CURSOR 0,0
PRINT <complex string>
Unless I'm missing something, I think you can do away with the first SET CURSOR and PRINT command altogether, as they don't seem to do anything.
As to relative pathing, it works like this:
LOAD BITMAP "Leather3.jpg", 0
This command will load the file from wherever the executable is, and if you move that executable someplace else you will also have to move the image file to the same folder, as the code will look in that new location for the image file.
LOAD BITMAP "GFX\Leather3.jpg", 0
This one will look for the image file in the folder "GFX" that lies in the same folder as the executable. If you move the executable, you also have to move the folder that contains the image file.
You've used SET TEXT OPAQUE any number of times throughout the code. You only need to do this once (do it right at the top of the initialisation code, immediately below your SET TEXT FONT command). If you ever used SET TEXT TRANSPARENT you'd need to set it back to opaque again later, as needed, but unless you set the text away from opaque you don't need to repeat yourself. DBPro's native text commands are insanely
sloooooow so always use as barebones few of them as you possibly can.
I'm curious about your use of GOSUBs when you clearly have a solid understanding of how to operate functions. You'll have to forgive me, but I'm flat out tired of arguing why the use of GOSUB and GOTO is simply incorrect programming. You'll find the argument all over any programming forum: Change all of your GOSUBs into functions and don't ever type the characters GOSUB or GOTO again
You can keep the labels to which they refer if you like as they help to identify different parts of your code and they're ignored by the compiler anyway, though I'd rather see REM's as a matter of practice.
The only other thing that jumps out at me is spacing. Take a look at these two lines of code, the first being yours and the second being my adjustment to it:
TileSize=TileSize*.75
TileSize = TileSize * .75
It's just a little easier to read. Here's another example in which it makes a far greater difference to code readability:
FirstColor$=STR$(RGBR(POINT(MOUSEX(),MOUSEY())))+STR$(RGBG(POINT(MOUSEX(),MOUSEY())))+STR$(RGBB(POINT(MOUSEX(),MOUSEY())))
FirstColor$ = STR$(RGBR(POINT(MOUSEX(), MOUSEY()))) + STR$(RGBG(POINT(MOUSEX(), MOUSEY()))) + STR$(RGBB(POINT(MOUSEX(), MOUSEY())))
As in written English, put a space after every comma, and around each operator like a plus sign. Code readability is a big issue when asking others to review your code, and also helps with your own debugging as each 'term' is made distinct to the eye.
Overall a solid effort for a returning programmer after an extended hiatus. Not many new guys can produce something like this on their first shot. Please accept my encouragement and continue on the good road back into our world!
