Fixed compiler and added new features. #106
Conversation
Pause, Game Over and stage complete now give the option to restart the game
Changed the powerup spawning to happen randomly when a brick is destroyed at the point it was destroyed.
|
yeah good changes 👍 no changes needed i think |
Jodiedoonan1
left a comment
Jodiedoonan1
left a comment
There was a problem hiding this comment.
1 Readability
The codebase is reasonably easy to follow. Responsibilities are split into clear classes. Function sizes are generally small and focused. Naming is mostly descriptive and intention revealing for both variables and functions. New features are integrated in a readable way with comments clearly describing each step.
2 maintainability
The code is maintainable and reasonably well structured, with clear class roles and a good start on centralising configuration and audio. The code uses modern C++ features sensibly. The SFML API usage looks current
3 Performance
For the size and complexity of this game, the current code is unlikely to cause performance issues for end users. The update and render loops are straightforward, and there are no obviously heavy per frame operations such as complex physics, huge allocations, or deep nested loops over large data structures.
4 Code quality
The codebase is in a decent state from a quality point of view. Responsibilities are clearly separated between classes, most functions are reasonably small, and the new features are integrated in a way that fits the existing structure.
5 Reinventing the wheel
The code makes good use of the existing language and library features and doesn’t seem to be reinventing major functionality that SFML or the C++ standard library already provides.
6 Reliability
In general, the game is likely to behave reliably under normal play. The main edge cases relate to audio (no return path in getFreeVoice, silent handling of failed sound loads) and the somewhat duplicated powerup timing logic. Addressing those would make the code more robust against unusual situations without changing the visible behaviour of the game.
7 Patterns
The code follows several common patterns that fit well with what’s usually taught in class.
8 Security
From a security standpoint, the code is safe for its purpose. It does not store or expose user data, and only reads local asset files. The main improvements here would be around better error handling for missing assets, rather than fixing any real security vulnerabilities.
9 Comments and documentation
The codebase is generally well documented, with meaningful comments and a clear README. The only significant improvement needed is updating the README to mention the latest gameplay and spawning changes
| { | ||
| auto& v = getFreeVoice(); | ||
| v.setBuffer(_bufferBrickBounce); | ||
| v.setPitch(randRange(_rng, 0.95f, 1.12f)); |
There was a problem hiding this comment.
This is a good way to add some audio variety
|
|
||
| float lerpSpeed = 1.f; //Speed that can be adjusted for smothing the movment | ||
|
|
||
| float newX = currentX + (posX - currentX) * lerpSpeed * dt; |
There was a problem hiding this comment.
Frame independent paddle movement is good
| const sf::Color fireBallEffectColour = sf::Color(255, 126, 0); //Base colour change for the fireball powerup | ||
|
|
||
|
|
||
| constexpr float POWERUP_DROP_CHANCE = 0.15f; |
There was a problem hiding this comment.
adding values here is a really good design choice as it centralises configuration values, making them easy to tweak and maintain
| currentIntensity += (flicker - currentIntensity)*0.2; //Smooth more real fire | ||
|
|
||
| // Red Green Blue | ||
| _sprite.setFillColor(sf::Color(static_cast<sf::Uint8>(255 * currentIntensity), static_cast<sf::Uint8>(120 * currentIntensity), static_cast<sf::Uint8>(30 * currentIntensity))); // fire flickering color |
There was a problem hiding this comment.
great example of using simple math to achieve a strong visual effect
| _ball->update(dt); | ||
| _powerupManager->update(dt); | ||
| //// move paddle | ||
| //if (sf::Keyboard::isKeyPressed(sf::Keyboard::D)) _paddle->moveRight(dt); //Using Mouse so not needed |
There was a problem hiding this comment.
could have left keyboard input in so the player can choose if they want to use mouse or keyboard
|
|
||
| bool SoundManager::initiate() | ||
| { | ||
| bool ok = true; |
There was a problem hiding this comment.
One small improvement would be to rename the variable ok to something more descriptive
| #include <random> | ||
|
|
||
|
|
||
| bool SoundManager::initiate() |
There was a problem hiding this comment.
Good centralising all sound loading in one place
3 participants
Resolved base-class undefined error caused by the include in PowerupBase.h for PowerupFireBall.h (15 min)
Bugs and code understanding, minor fixes and looking over the code to grasp the structure 1 hour
Features:
Controls: Mouse movment implemented and working 45 min
Visual Effects: Flickering fireball effect, and a visual change to the fireball powerup so that it is destinct 1 hour 20 min
Sound Implementation: Sounds for collision with walls the paddle and bricks, and a fire sound effect for the fireball power up. 3 hours
Total 6 hours 20 minutes.
Later additions include:
The addition of a game loop and restart feature.
A change to how powerups spawn based on brick position, not the top of the window.