How to restart a switch gamestate?

So in my game, I have an option to roll a dice. It’s simple: it generates a random number from 1 (inclusive) to 7 (exclusive), and if the number is smaller than 4 but bigger than 0 (ie 1, 2, 3), then it will divide your money by 2. If it is bigger than 3 but smaller than 7 (ie 4, 5, 6), then it will multiply your money by 2. I added in a delay so that whenever you roll, you cannot roll again until 5 minutes pass. Unfortunately though, whenever 5 minutes is over, and it switches back to the dice roll gamestate, the multiplication / division / random generation are still working. What I mean is if i got a 5 for example, my money would multiply by 2, then it would switch the gamestate to a delay gamestate, for 5 mins until switching back to the dice roll gamestate, but when it switches back, that mutliplication is still going, and my money will keep on multiplying after the switch. Here is my code so far.

This is quite difficult to unpick because you’ve got a lot going on and some of it is happening in unusual ways.

E.g.

• `didDiceRoll` should probably be a `bool` because it seems like the kind of thing that would only be `true` or `false`.
• `diceRestart` appears to not actually be doing anything useful
• The `if` checks in `diceRandom` are pointless, because you’re running exactly the same code whether `diceRestart` is `true` or `false`, and `diceRestart` can only be `true` or `false`, it can’t be anything else (that is the very definition of a boolean value: being true or false and nothing else), which basically means you might as well just ditch the `if`s because you’re doing the same thing in both possible cases.

Part of your problem is almost certainly that you’re doing the multiply and device in the main part of `diceScreen`, which means that every time `diceScreen` runs it’s going to do the multiply or divide.

It seems that somehow the code is either getting stuck in the `GameDice` state or flicking back and forth between `GameDice` and `GameDiceDelay`. I’m going to have a quick look through to see if I can figure out which and why. (Though it’s late here, so I might have to put it off until the morning.)

If you want to keep an eye on my changes, I’ve made a local fork of your code on GitHub as well as a branch called `fix-dice-bug`, so you can view any changes I commit here:

1 Like

As I started working though it I realised the problem, and it is related to the part I thought it would be, but not exactly how I was expecting.

The problem isn’t that the game gets stuck on the `GameDice` state, because I realise now that it’s supposed to stay on that state.

The problem is in how you’ve been doing the checks for the multiplying and dividing of the money.

You see, the first time the code enters that region, `dice` is `0`, and it seems you are in fact relying on it to be `0` so that it won’t trigger either of those two conditions.

However, it never gets set back to `0`, so the next time the game enters the `GameDice` state, one of those conditions will be true, meaning that the multiply or divide will end up happening once every frame.

The obvious fix is to make sure that instead of calling `diceRandom` in `diceScreenDelay`, you instead set `dice` back to `0`.
I.e. change:

To:

``````if (currentDice >= delayDice) {
didDiceRoll = 0;
gameState = GameState::GameDice;
dice = 0;
}
``````

However, I happen to know that there’s a better way to do it…

For one thing, using `0` as some kind of special signifier isn’t really ideal because then you end up with this weird special case to handle.

But more importantly, those tests shouldn’t be occurring in the main part of the dice state.

Instead, you only need to do the multiplication/division once, and it should only happen in response to the player pressing A, right? Because that’s the whole point of the state: the player presses A to roll the dice and possibly double their money.

So, after a bit of rewriting, you get this:

And you can see the changes I made here:

• Firstly, I removed the `diceRestart` variable because it didn’t appear to be doing anything useful
• Then I changed `didDiceRoll` into a `bool` because it seemed to be suited to being only `true` or `false`.
• (In fact, what you chose to name it should have been a big clue: the answer to any question that starts with the word ‘did’ can only really be ‘yes’ or ‘no’ (i.e. `true` or `false`).)
• Then I moved those pesky range checks into the `if` that was checking if `A` was pressed. That immediately fixed the bug.
• (Frankly, that’s actually the only part of what I did that was really relevant to fixing your bug, the other stuff is all to do with simplifying the code by removing clutter and making it cleaner and more logical.)
• Finally, I removed the `didDiceRoll` variable because I realised it was no longer necessary.
• (Truthfully, I realised it probably wasn’t necessary earlier, but removing it earlier would have caused an even more awkward bug and I wanted to keep the same behaviour to make it clear that step 3 is the part that actually fixes things.)

So, does that all make sense?

Most importantly, do you see why moving the checks into the button check fixes the problem?

If you’re happy with those changes, you can either grab a copy of my `fix-dice-bug` branch, or I can make a ‘pull request’, which would allow you to merge my changes onto your repo (i.e. it copies the ‘commits’ across to your repo).

There’s a few other things that could be improved if you’re interested in making the code simpler and/or improving its general quality. But if you’re happy to just keep going and only stop when you hit a bug, that’s fine too.

2 Likes

Hi, thanks for your reply. But before I saw your post, I actually changed the code a bit, instead of having to wait 5 mins after a roll, you can now roll instantly. The catch is that before you had a 1/2 chance of doubling your money, but now, you only have a 1/3 chance, I think it it a little bit better like this, but thanks for your reply