Implement more features

[bhouser]

I created a "fake" master branch that references the project state before you new commits. This pull request emulates what we would see if you had made the new commits in a feature branch.

[bhouser]

Very nice progress!

I left a number of comments about things you can improve.

The nice thing is that the app has enough content now, that it makes sense to stop and think about what's working well in the code, and what should be restructured.

So instead of adding more game features, I'd recommend these goals next:

• refactor the various state items into a single useState called gameState. (easyish? But there will be lots of edits)
• eliminate code repetition related to dimensions. See the last comment about rendering Dimensions. (more difficult, but will make it trivial to scale up to 8 dimensions)
• App.tsx is getting long. Try writing a custom react hook to save the game. Put it in a separate file, import it to use here.

Additionally, let's start to use some more version control features! Figure out how to create a branch. For each discrete set of changes, have a separate commit on the branch. Then when it's ready for review before merging back into main, create a pull request and add me as a reviewer. Then I'll be able to leave comments like I have here, and then after the review is done, you can merge the changes into main. It's also possible to add additional commits to the branch after the Pull Request ("PR") has been opened.

[bhouser]

It's not really typical to use rem's or em's for spacing. Recommend to just use pixels.

[bhouser]

Nice usage of flex to center the antimatter counter!

You don't need margin or width here. Maybe some of the why's behind that are better to discuss on a call.

width: 100% is actually causing the element to be wider than the page, because width doesn't yet include the margin, so the width ends up being 100% of page with plus the margin = too wide. This is also why you have a horizontal scrollbar at the bottom of the page.

[mturich]

Couldn't it be that I forgot box-sizing: border-box; ?

[bhouser]

display: flex does nothing here. This is the child element of .heading, which is the "flex parent". The parent causes the flex-child to orient and resize based on the flexbox rules.

This is the visualization I always refer to when I want to check which flex properties are applied on the parent and which one's on the child:
https://css-tricks.com/snippets/css/a-guide-to-flexbox/

justify-content and align-items do nothing because they would have to be applied on the flex-parent.

The reason this element is actually floating center is because you specified margin: auto. This is the old way to do it, before flexbox. Try removing margin: auto, and then in .heading (which is the flex-parent), add justify-content: center`

[bhouser]

I've never learned how to use grid. Good luck 😛! (it's probably super reasonable though). Your resulting layout looks good, so it can't be that bad!

As far as I've heard grid is a really good thing. But when I was doing frontend development, it was not supported on enough browsers to actually use it in real code.

[bhouser]

This would be the realm of opinion: some people might complain that a CSS class named highlight should not apply padding to the element. They would probably say it should only do things that affect the appearance but not the size, so of the four rules you applied here the only one that's ok under .highlight is background-color.

Of course this is semantics. If you were to rename it "score-style-rules" there'd be nothing to complain about. But why does something named "highlight" do non-highlight stuff?

[bhouser]

I hate to break it to you: it doesn't actually work (correctly) 😅. When you start a fresh game and click "Buy Max" it appears to buy twice for the cost of 10 each, instead of 1 for 10 and 1 for 100.

I think this can be done much simpler. Looks like you're doing a recursive purchase. Additionally, it's asynchronously recursive, with a time delay in between. I see you've used a ref for canIstillBuyRef, and keeping it up to date: that's necessary because you're in a timeout. But you didn't do that with tickspeedPrice, that's probably why you're able to buy a second time for the price of 10 instead of 100.

You're calling handleTickBtnClick from inside the timeout, so that's going to be an old version of that function referencing old versions of all the values used inside it. I didn't realize when we chose this project how much of a brainbender it is to use timeouts inside react-hooks components 😃, but basically your timeout will see a snapshot of the universe as it was when it was created. And the dynamic universe gets updated but the timeout can never see the unfolding future, it's stuck in the past.

Let's rewrite this to definitely not have a time-delay. For multiple purchases, we should either simulate the purchases before doing any actual state updates and do it in one shot, or we should try to use the power of math to somehow compute the quantity of purchases we can do.

Probably looks something like this:

const getCostForPurchaseQtyRecursive = (firstCost: number, quantity: number) => {
  if (quantity == 1) {
    return firstCost;
  } else {
    return firstCost + getCostForPurchaseQty(firstCost * 10, quantity - 1);
  }
}

const getCostForPurchaseQtySuperMathematics = (firstCost: number, quantity: number) => {
  // this is the part where you look up the formula for a geometric series so you can flex on your coworkers ^^
}

const handleBuyMaxClick = () => {
  let affordableQuantity = 0;
  while (getCostForPurchaseQty(affordableQuantity + 1) < antimatter) {
    affordableQuantity++;
  }

  const totalPurchaseCost = getCostForPurchaseQty(affordableQuantity);
  setClockSpeed(clockSpeed => clockSpeed * (1 - 0.11) ** affordableQuantity);
  setAntimatter(prevAntimatter => prevAntimatter - totalPurcahseCost);
  setTickspeedPrice(prevPrice => prevPrice * 10 ** affordableQuantity);
}

Most of this code is untested but hopefully that conveys the generally idea haha. And you can probably get rid of several ref's that you won't need once the setTimeout is gone.

[bhouser]

Mmmm I understand exactly why you're putting all the values in a ref. I can't think of a great way to do this more cleanly. One thing that comes to mind is you could pass all the values into the effect like this:

useEffect(() => {
  // lots of cool code
}, [gameState]); // <- note I'm assuming you've put all the values into one piece of state

And then write an effect that checks if a certain amount of time has passed. Rather than setting a timeout.

For example call Date.now(). That will give you the time in milliseconds (the number of Milliseconds since Jan. 1st, 1970, the "beginning of all time" as far as computers are concerned). Save that value in gameState.lastSaveTime.

Whenever this effect hook is called (it will get called whenever gameState is changed, check if Date.now() >= gameState.lastSaveTime + 60000. If it's greater, then save the game, and set lastSaveTime to the new Date.now() value.

This only works because our gameState is constantly changing due to the other timeout, and those changes can trigger our effect. We're basically assuming (quite correctly, I might add), that if gameState has not changed, there's no need to save the game, regardless of how much time has passed.

[bhouser]

Nice usage of localstorage, JSON.parse, and JSON.stringify !!

[bhouser]

Reasonable. One thing you might improve here: try to have only one copy of these initial values, instead of duplicating the numbers used to initialize state near the top of this component.

[bhouser]

Nice work getting 3 dimensions to work!

You probably noticed that you have to copy a lot of code to add additional dimensions. The next refactoring goal here would be to try to do something like this:

{dimensions.map((dim) =>(
  <Dimension
    dimCardinality={dim.cardinality} // which dim number is this?
    dimCount={dim.count}
    antimatter={antimatter}
    setDimCount={setDimCount} // when you call this function you'll pass the new count, plus the cardinality (which dim are we incrementing?)
    ...
))}

This will require you to refactor how you're saving state related to each dimension. Probably you want to save an array of dimension objects, where each object has a set of fields containing the state related to that dimension.