Login [Register]
Don't have an account? Register now to chat, post, use our tools, and much more.
I've been working on a Sudoku game written in C for the CE series of calculators.



This implementation of the game will be very much inspired by JWinslow23's Puzzler's Sudoku for the monochrome calcs.

Currently Implemented Features:
  • Draw a puzzle from a 9x9 array
  • Move the cursor and insert numbers in empty cells
  • Erase number from a cell with [0]

Planned Features:
  • Enlarged view of current puzzle section, as can be seen in Puzzler's Sudoku linked above
  • "Penciling in" numbers, shown in the enlarged section
  • Loading level packs from an Appvar
  • Randomly generated puzzles

As this is my first ever project written in C, I would greatly appreciate any feedback related to code. The GitHub repository for this project can be found here.

My first (minor) question is, what is the "proper" way to prevent super fast key repeating? Right now I just have a delay(100) after every arrow key press, but is there a better/more generally accepted way to do this?
I like to have it loop until the user releases the key, which would look something like this:

Code:
if (kb_Data[6] & kb_Enter) {
//do stuff
while (kb_Data[6] & kb_Enter) {
Kb_Scan();
}
}


So that would do something when you press enter, then loop until enter is released. This means that the game will respond as fast as you can click, as well.
Some notes:

* Global variables like this: https://github.com/Jeffitus/Sudoku-CE/blob/master/src/main.c#L26-L29
Don't use global variables, unless they are large data blocks or some other item. Even then they should be avoided.
Use local variables as much as possible. The variable names you gave also are not at all helpful in telling what they do.

* This data block: https://github.com/Jeffitus/Sudoku-CE/blob/master/src/main.c#L26-L29
Is a global, yet you are passing it around to other other functions. If you are going to make it a global, make it a global and never treat it like a local. Otherwise, put this in the main function as a "static" local.

* There's a lot of these kind of lines: https://github.com/Jeffitus/Sudoku-CE/blob/master/src/main.c#L80-L83
This is nearly impossible to read. Use a local variable inside the loop to store intermediates rather than one huge line. E.g. "int col = i * (PLAYING_GRID_SIZE - 1);", and then combine them in the statement.

* This should be a do {} while loop: https://github.com/Jeffitus/Sudoku-CE/blob/master/src/main.c#L101
This is because kb_Data isn't updated until kb_Scan is called.

* This chuck of code is unnecessary: https://github.com/Jeffitus/Sudoku-CE/blob/master/src/main.c#L137-L187
This code can be greatly optimized. Feel free to think about how, remember if you ever feel like you are copying / pasting there's usually a better way.

* This other chunk of code: https://github.com/Jeffitus/Sudoku-CE/blob/master/src/main.c#L108-L135
You can see here for detecting presses, and then extend that to repeating.

* Use gfx_BlitBuffer() here: https://github.com/Jeffitus/Sudoku-CE/blob/master/src/main.c#L207

* For some reason this function is called multiple times: https://github.com/Jeffitus/Sudoku-CE/blob/master/src/main.c#L96

* These two lines don't make sense: https://github.com/Jeffitus/Sudoku-CE/blob/master/src/main.c#L75-L76
Either use blitting, or use swapping, but usually it doesn't make sense to use both. I'll point you to this page.

* Unless I'm mistaken, you only need to check current_board here: https://github.com/Jeffitus/Sudoku-CE/blob/master/src/main.c#L199

* And lastly: why the hell is this function not used anywhere but at the start: https://github.com/Jeffitus/Sudoku-CE/blob/master/src/main.c#L54
This function should render the lines and the numbers lol.
Mateo wrote:
(lots of things)

Thanks! I'll be sure to look into all of these in-depth later, but this:
Quote:
* Unless I'm mistaken, you only need to check current_board here: https://github.com/Jeffitus/Sudoku-CE/blob/master/src/main.c#L199

I'm checking both because I only want it to draw to numbers in cells that are allowed to be updated. If I don't check puzzle, it will draw the whole puzzle in the dark blue color, rather than the dark gray color (which reminds me, I should probably change those colors so they aren't so similar).
  
Register to Join the Conversation
Have your own thoughts to add to this or any other topic? Want to ask a question, offer a suggestion, share your own programs and projects, upload a file to the file archives, get help with calculator and computer programming, or simply chat with like-minded coders and tech and calculator enthusiasts via the site-wide AJAX SAX widget? Registration for a free Cemetech account only takes a minute.

» Go to Registration page
Page 1 of 1
» All times are GMT - 5 Hours
 
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum

 

Advertisement