Hello everybody! Finally took the time to finish my little game. Many thanks to @drummyfish@ESPboy@Pharap for helping with the damned bridge!
A little about the game:
Your task is to move from one platform to another through the constructed blade-bridge. If you get to the exact center of the bridge, you earn one coin. Perfect hit 5 times in a row gives +5 coins. At the hard level, these indicators double. Well, also you will find a small bonus in the store
Have fun! BladeRunner.hex (60.4 KB)
I would also like to ask professionals to evaluate my code, but it is so convoluted that even I get lost in it Anyway, all comments and advice are welcome.
Congratulation, very nice game Thanks also for sharing it under a free license. It’s pretty cool you included the users of this forums as the players! Is this your first game?
If you want my comments, there are things to improve, but you’re on the right track. Don’t take it as bashing your code, just a constructive criticism. Remember, you’re already on a programming level most people will never reach.
E.g. chosenPlayer doesn’t have to be an array at all, it can simply be a variable that holds the number of the selected player. The player bool variables (PharapCH, drummyfishCH, …) can all be deleted – it seems like you’re thinking that you are storing the bool variables in arrays (buyPlayer, costPlayer, …) but you’re not, you only initialize the arrays with their initial value (0). I am actually kinda confused about all these arrays. If you share what you intended to do, we may tell you how it could be done.
Formatting could be better – yeah, it’s extra work but as code gets longer it becomes more and more important. E.g. the super long line at the very beginning doesn’t help readability, unless there are like 3 variables it’s best to put each variable on a separate line. Also try to stay consistent with bracket formatting in control structure (if, for, …), it greatly helps to see the code structure. For example I always use
if (something)
{
doSomething
}
else
{
doSomethingElse
}
Naming functions and variables should also be consistent: functions should ideally be named by actions, e.g. handleMenu() instead of just menu(). Probably most common convention is to use lowercase-starting camelCase (like myAwesomeVariable) – your functions sometimes start with lowercase, sometimes uppercase (uppercase is used for type and class names).
I am not sure what this function does but this is where arrays could actually help make it much much shorter. Example:
As we discussed in your Variable for array topic, your sprites should have a height that’s a multiple of 8. If you round up all heights to the nearest multiple of 8, you will be properly following the API. It shouldn’t affect the operation of the game in any way.
You would be better to use the Arduboy2 audio class to control sound muting. To do this:
Many thanks! This is my second game. The first was Ardubloxx. Thanks for the constructive criticism! The fact is that my knowledge is less than my creative impulse. So when I try to create something, I only use what I have already learned. I start writing structured code, but at some point my brain says: hey dude, I already want to play this, finish quickly! I’m in the process of making a fighting game. And in it, I already feel the need for the order of the code and its efficiency, because there is not enough memory. In general, I will work on my impulsiveness Thanks for the reminder about the correct use of the array and the readability of the code. I will definitely fix it!
I’ll definitely try to resize the sprite, thanks for reminding me!
The built-in mute function is what I was looking for! thank you so much:)
Regarding the sound, I also wanted to clarify the question: when launching the Ardubloxx and Blade Runner games on ESPboy, the sound does not sound correctly. I understand that it is better to discuss this topic on the appropriate forum, but still I want to figure out if I am using beep.tone correctly?
Then why not just let the ARDUBOY logo be displayed and then display your logo after that in setup()? You could clear the display then display your logo as a sprite and then add a delay to give time to read it.
If people don’t want to wait for the ARDUBOY logo, it can be bypassed in two ways:
Pressing the RIGHT button before or during the logo display will abort it.
A user can set a flag in system EEPROM to eliminate the display of the logo for all sketches that use the Arduboy2 library in the normal way. The flag can be set using the SetSytemEEPROM example sketch included in the Arduboy2 library.
I apologise in advance if I repeat something someone else has said,.
(If multiple people are saying the same thing, it’s probably a significant issue.)
Firstly, defining variables by doing int variable1, variable2, variable3 is usually less readable than:
int variable1;
int variable2;
int variable3;
You’re mixing byte and uint8_t, but they’re both the same thing (on Arduboy at least).
Typically you should prefer uint8_t because byte is Arduino-specific.
You should probably have moved your images to a separate header file, but I’m presuming you might not actually know how to do that yet. Essentially you’d just make a file with the extension .h, e.g. images.h, and then you’d include it from the main file with #include "images.h".
I decided this would be easiest to demonstrate by example so I made a branch* with these changes to demonstrate.
* If you’re reading this in the (possibly distant) future and the link is broken, try here instead.
As @drummyfish already mentioned, YourPlayer could be changed to deal with arrays instead.
The way you are using chosenPlayer is inefficient. You’ve got an array of about 11 values, each being either 0 or 1, presumably to indicate which character the player chose, but really you only need one variable to indicate this.
You could instead have just a single variable, uint8_t chosenPlayer, and use a variation of the code I just posted above to figure out which player that represents. E.g.
In fact you could probably get rid of the for loop in Players by replacing references to i with markerPlayer.
I also notice that you are comparing some bool variables to 1, but this is redundant…
bool, the boolean data type, holds the boolean values of true or false.
When you use an if, the value between the brackets is implicitly converted to bool, i.e. a value of true or false.
For example, when you do lengthBridge > 0, the value of lengthBridge is compared to 0 and if it is greater than 0 the result of the expression lengthBridge > 0 is true. Otherwise if lengthBridge is 0 or less, the result of the expression lengthBridge > 0 is false.
When you write playerOnPlatform == 1, you are effectively writing true == 1 or false == 1, neither of which makes sense because a boolean value isn’t an integer. To account for this, the compiler will attempt to convert 1 to a boolean value, which it does by saying ’0 is false, everything else is true'. In this case 1 would be true, which means your code ends up being true == true or false == true, either of which is redundant.
Long story short: you don’t need to compare a boolean variable to anything, you can just do if(playerOnPlatform).
If you want to do the opposite (e.g. if player is not on the platform) you would use !, the ‘not’ operator, e.g. if(!playerOnPlatform) (read as ‘if not playerOnPlatform’).
Similarly, instead of assigning values of 1 and 0 to boolean variables, you should assign values of true or false.
E.g. instead of if (lengthBridge > 0) bridgeIsDown = 1; you should do either:
if (lengthBridge > 0)
bridgeIsDown = true;
Or:
bridgeIsDown = (lengthBridge > 0);
(If you aren’t sure why this works, think about what I said earlier: lengthBridge > 0 is an expression that is evaluated to either true or false, so the datatypes match up.)
You have a bug here:
= is the assignment operator, == is the comparison operator.
What happens with level = 2 is that level is assigned a value of 2, then implicitly converted to bool (as mentioned above, non-zero values are converted to true), thus if(level = 2) is always true.
As others have pointed out, your formatting is a bit hard to follow.
As others have mentioned, there’s a few naming oddities.
Typically for Arduino projects:
Function and variable names are camel case
e.g. drawLine
Type names are Pascal case
Generally function names should start with verbs and represent actions and variable names should be nouns.
One notable exception is bool variables and functions returning bool, which some people prefer to phrase like a question, e.g. soundEnabled/isSoundEnabled or soundOn/isSoundOn.
Good function name examples: drawLine, drawBridge, updateMenu
Bad function name examples: YourPlayer, Platform, Menu, Bonus
When you use local variables, you should declare them as close to the point you use them as possible.
For example, in Players you declare uint8_t i; at the top of the function and then don’t use it until half way down in for (i = 0; i < 12; i++). You’d be better off just doing for (uint8_t i = 0; i < 12; i++)
Lastly, tittle should be title and ninga should be ninja.
A few of your other words are a bit odd, like markerPlayer (should be playerMarker) and lengthBridge (should be bridgeLength), but I expect that’s due to language differences. E.g. I could imagine someone who spoke French would be tempted to have an enemyBlue instead of blueEnemy.
I can think of a few other ways to improve your code (e.g. introducing you to enumerations) and how your code is hosted on GitHub (e.g. putting it into a subfolder, putting the .hex in a release), but I will let you read through and make sense of what I have already written first. I don’t want to bombard you with too many things.
Thanks for the feedback and detailed explanation! I will definitely work on the bugs in my spare time! I’m sure this will free up a significant amount of memory, and creating separate libraries and writing the code correctly will improve readability. Thanks again for your work!
Bugs fixed. Corrected everything you talked about. Hopefully the code looks more sane now. Debugging my code freed 1500 bytes anyway! My compliments to you guys I’m already starting to feel my brain getting a little bigger