Fixes and tasks for assignment #91

jerryf276 · 2025-10-21T03:30:52Z

Time Details and Changelist:

⦁ Fixed compiler error (Took 3 minutes)
⦁ Changed pause button to escape (took 1 minute)
⦁ Made rng seed based on the time since the seed would be the same every time you ran it (1 minute)
⦁ Implemented Screen Shaking when a life is lost (took 30 minutes)
⦁ Added ability to restart the game (took 25 minutes)
⦁ Implemented mouse movement for the pad (took 10 minutes)
⦁ Added a leaderboard (Took 2 hours, took a while due to problem solving)
⦁ Replaced the time left with a guage that decreases based on the time left (took 45 minutes, took a while due to problem solving/debugging)

TOTAL TIME: 3 hours and 55 minutes

…Fireball in its base class)

… mins) I also changed the keybind for pause from p to escape for quality of life improving

allows us to see scores up to top 10, when the game finishes an 11th time, the lowest score is deleted off the top 10 scores vector

took a while since i was trying to figure out how to get the maximum time (unfortunately couldn't work it out so i've hard coded 5 seconds since all the power ups use that as their starting time)

Corcso · 2025-11-02T10:26:52Z

Breakout/ScreenShaker.cpp

+
+	if (screenPosition.x != 0 && screenPosition.y != 0) {
+		shakerSwitch *= -1;
+	}


This if statement's purpose is a little confusing, with relation to why you don't switch direction when both = 0, perhaps a comment explainer would be nice.

Corcso · 2025-11-02T10:27:33Z

Breakout/ScreenShaker.h

Overall class is set out well, clear, and decoupled from window nicely. Good work!

Corcso · 2025-11-02T10:29:50Z

Breakout/ScreenShaker.h

+	float shakeDuration = 0;
+	float currentTime = 0;
+	sf::Vector2i screenPosition = sf::Vector2i(0, 0);
+	int shakerSwitch = 1;


Shaker switch only stores -1 or 1, perhaps a smaller 1 byte variable could be used such as int8_t.

Corcso · 2025-11-02T10:49:52Z

Breakout/GameManager.cpp

+            _lives = 3;
+            _ui->addLivesBack();
+            _masterText.setString("");
+        }


This feature and commit is pretty clear and sensible, only thing which could be changed is ball & managers could have a reset function instead of being deleted and recreated. Ok here, but could cause issues if say the paddle had a reference to the ball, which is now invalid as the ball has moved.

In relation to game restarting commit

Corcso · 2025-11-02T10:50:46Z

Breakout/Paddle.cpp

+        _sprite.setPosition(sf::Mouse::getPosition().x, _sprite.getPosition().y);
+    //}
+}
+


Feature is clear and layout is sensible, perhaps could remove unnecessary comments once feature is finalised.

Corcso · 2025-11-02T10:53:24Z

Breakout/GameManager.cpp

    _window->draw(_masterText);
+    _window->draw(scoreText);
+
+    if (gameFinished == true) {


"== true" could be removed here but not a large issue

Corcso · 2025-11-02T11:02:13Z

Breakout/GameManager.cpp

-    {
-        _masterText.setString("Game over. Press R to retry.");
+
+    if (gameFinished == true) {


There is a bug at the moment, if you win the game you cannot play again, it looks to be caused by _levelComplete = false; being missing from the reset logic

Noted, I will fix that when I can.

Corcso · 2025-11-02T11:05:15Z

Breakout/GameManager.cpp

+        //adding new score
+        storedScores.push_back(score);
+        //sorting the current scores by descending order
+        std::sort(storedScores.begin(), storedScores.end(), comp);


comp function could be replaced with std::greater<int>() see https://en.cppreference.com/w/cpp/algorithm/sort.html

Corcso · 2025-11-02T11:08:43Z

Breakout/UI.cpp

 {
 	std::ostringstream oss;

 	switch (powerup.first)


Large amount of repeated code within this switch case, perhaps could be separated into a separate function.

Yeah, it would be more ideal if I do it this way. Thanks for the tip!

Corcso · 2025-11-02T11:11:14Z

Breakout/PowerupBase.h


    // Game logic
    float _time;
+    float maxTime = 5;


Wouldn't a powerup's max time always be their starting time?

If I remember right I believe I was trying to solve the best way to do the timer bar, but got stuck during it.

Corcso · 2025-11-02T11:12:08Z

Breakout/UI.h

 	static constexpr float LIFE_RADIUS = 15.0f;
 	static constexpr float LIFE_PADDING = 20.0f;
+
+	int barSize = 200;


This looks like it could this be const static

That's true, thanks for letting me know.

Corcso · 2025-11-02T11:20:41Z

Overall code is clear and well layed out, changes make sense with no unnecessarily complex class structure or logic.

Some things I noticed besides the comments above:

Naming conventions could be better followed for member variables which are prefixed with _
Entire window is shook with screen shake which is an interesting choice, but the window is currently locked to the top left of the screen and cannot be moved, perhaps a current window position could be taken at the start of the screen shake so the player can move the window.
Bug with restarting after winning, but easy enough fix, mentioned in comment above.
There is a few magic number around, both from the original code and added code. These could be moved to constexpr's or static consts.
It's hidden so don't worry that you didn't catch it but there is a memory leak (from the original code) in fire ball powerup's deconstructor.

Nice work, could follow and understand code easily 🔥

jerryf276 · 2025-11-04T02:30:06Z

@Corcso Thank you for taking the time out to review my code :).

Going forward, I will improve my solution based on the feedback you have given me.

I understand that going forward (based on looking at your review as well):

I will include more suited naming conventions that match the original code's (Since in the games industry it is very common to add onto existing code).
Make use of more modern C++ features (such as the auto keyword and smart pointers)
Fix the bugs as mentioned in the review.
Instead of using magic numbers, I create an h file that involves constexpr's or static constants
Review the original code more in depth so that I can spot bugs (such as the memory leak)

I have taken the feedback regarding the code being clear and well laid out, and using the simple to understand class structure and logic into consideration and will continue to these programming patterns. Thanks again!

jerryf276 added 9 commits October 19, 2025 03:35

commit: STARTED 3:35 am on 19/20/25

0a6c3ff

commit: fixed compiler error (took 3 minutes, Removed include Powerup…

7e76427

…Fireball in its base class)

commit: implemented screen shaking when losing a life (took approx 30…

6c5a6f9

… mins) I also changed the keybind for pause from p to escape for quality of life improving

commit: stopped for now

81e816e

commit: added ability to restart the game (approx 25 mins)

43044fb

commit: added mouse movement (took 10 mins)

bbfb5b2

commit: added leaderboard (took approx 2 hours)

11ebc4e

allows us to see scores up to top 10, when the game finishes an 11th time, the lowest score is deleted off the top 10 scores vector

commit: added health bar (took 45 mins)

0d0008b

took a while since i was trying to figure out how to get the maximum time (unfortunately couldn't work it out so i've hard coded 5 seconds since all the power ups use that as their starting time)

Update README.md

1ab7b48

Corcso reviewed Nov 2, 2025

View reviewed changes

Fixes and tasks for assignment #91

Are you sure you want to change the base?

Fixes and tasks for assignment #91

Uh oh!

Conversation

jerryf276 commented Oct 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Corcso Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Corcso Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Corcso Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Corcso commented Nov 2, 2025

Uh oh!

jerryf276 commented Nov 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Corcso Nov 2, 2025 •

edited

Loading

Corcso Nov 2, 2025 •

edited

Loading

Corcso Nov 2, 2025 •

edited

Loading