Cactus Clicker
h
CodingCactus (2439)

Hi, so i recently asked people to submit suggestions for things that I can code and this was the only suggestion :(

It is a clicker game!


How to play

Click the cactus


Features

  • You can click a cactus
  • Saves your scores and upgrades if you leave or refresh the page
  • 'Collects' cacti when you close the page (depending on your cacti per second)

Have fun

Please tell me how to improve, this is my first time using js, so there is probably something i am doing wrong

Please Please Please give me some more suggestions on more things to code so that I can keep learning this language. Submit your suggestions here


Also, apparently it is pride month, so yeah, I have changed my pfp to one that @Codemonkey51 made :)

You are viewing a single comment. View All
MrEconomical (2195)

im a mean person so I will proceed to brutally critique your code now
1. NEVER NEVER NEVER NEVER NEVER NEVER NEVER NEVER USE INNERHTML - It directly sets the HTML of elements which means malicious users can insert <script>s into your application if you were making something like a chatroom. use innerText instead
2. nobody uses localStorage.getItem just use localStorage[item] or localStorage.item also you don't need to check if something === null (btw kudos for you for using === and not ==) just do if (!something). you can take advantage of these truthy and falsy values which evaluate to true and false, as their name suggests and they are very useful for writing neat code in js
3. instead of having giant if else chains you can use ternary which is useful. your first block from lines 2-6 can be condensed into this: let score = localStorage.score ? + localStorage.score : 0 (you can use + instead of Number() to convert strings to numbers as explained in my post). ternary works a ? (do something if a is true) : (do something if a is false) and you can also use it for assignment x = a ? (assign if a is true) : (assign if a is false) or if you are even bigger brain you can use let score = + localStorage.score || 0 (this works because || will do the first thing, and if the first thing isnt true or truthy it will do the second thing)
4. please try to put spaces in between brackets everytime I see }else{ I die a little bit inside :(
5. you should always use let and const for declaring variables its a better practice since 1) they are both block scoped and 2) they provide a very very slight performance advantage. block scoped means that they only apply to the block they are in or the segment of the code between the curly braces. if I were to use var, it would still exist: if (true) { var a = "hi" } // a still exists here! but this wouldn't be the case with let. it's mainly just a good practice since almost nobody uses var anymore, it has a lot of scoping issues and its weird.
6. never declare variables without let or const it makes them properties of window and immediately global just very weird
7. dont spam document.getElementById() you can assign a variable or something
8. when doing string concatonation you should use template literals, they are just much nicer: `hello ${this} is my message` you can insert any valid javascript expression inside ${} its just like templating as the name suggests
good luck on any future projects you hope to make with javascript!

CodingCactus (2439)

@MrEconomical errmmm ok thanks I will try to eventually understand what you just said :)

ChezCoder (1458)

@MrEconomical yeah, I like to think of let and const as temporary variables. Its just so much more cleeean

ChezCoder (1458)

@CodingCactus can i help you fix up your javascript?

ChezCoder (1458)

@CodingCactus lol i mean like do what mreconomical said above.

CodingCactus (2439)

@ChezCoder ik, but this project is done, I will consider what he has said in future projects and as this works, there isn't much point changing it

ChezCoder (1458)

@CodingCactus also, for the PFP flip click thing, here is the code to flip it:

document.getElementById("cactus").style.transform = `scaleX(${document.getElementById("cactus").style.transform.split("scaleX(")[1].split(")")[0]*-1})`;
ChezCoder (1458)

@CodingCactus wut... where did u put it?

ChezCoder (1458)

@CodingCactus ah, ok i see.
In style.css, add this to the image's style:
transform: scaleX(1);
Then it will work :D

ChezCoder (1458)

@CodingCactus hmm...

beginning of script.js:
document.getElementById("cactus").style.transform = "scaleX(1)";

ChezCoder (1458)

@CodingCactus lol, also,
if (localStorage.getItem("perSec") === null)
can just be
if (localStorage.getItem("perSec"))
This is because null == false

johnstev111 (229)

@CodingCactus I will just say this: you can make a clicker in 20 lines of JS

CodingCactus (2439)

@johnstev111 ok, you do that and show me

johnstev111 (229)

@CodingCactus I've done it already, I just need a hideous amount of items to buy

johnstev111 (229)

@CodingCactus WHAT? I've been testing that (are you lying?)

johnstev111 (229)

@CodingCactus OOF FIXED IT SORRY... IT WAS A MISSING CLOSING BRACE THAT DID IT

CodingCactus (2439)

erm so it has no per second things or saves your data etc.
@johnstev111

johnstev111 (229)

@CodingCactus It does have per second things... Buy Coral Rocket and C#. And I am noob at JS so I don't know how to save data