refactor: consolidar Ship::isAlive/isHit/isActive en isActive()

Els tres mètodes retornaven el mateix bool a partir d'is_hit_:
  isActive() = !is_hit_   (override de Entity)
  isAlive()  = !is_hit_
  isHit()    =  is_hit_

Eren tres formes diferents de preguntar el mateix, repartides sense
criteri pels call-sites (collision_system, game_scene). Conservem
isActive() perquè és l'override polimòrfic d'Entity i esborrem els
altres dos. Actualitzats els 5 call-sites externs.

Hallazgo #11 de CODE_REVIEW.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-20 17:37:37 +02:00
parent e1d6cd1bb9
commit 97c98272c9
3 changed files with 167 additions and 164 deletions
+41 -43
View File
@@ -11,54 +11,52 @@
#include "core/types.hpp"
class Ship : public Entities::Entity {
public:
Ship()
: Entity(nullptr) {}
explicit Ship(Rendering::Renderer* renderer, const char* shape_file = "ship.shp");
public:
Ship()
: Entity(nullptr) {}
explicit Ship(Rendering::Renderer* renderer, const char* shape_file = "ship.shp");
void init() override { init(nullptr, false); }
void init(const Vec2* spawn_point, bool activar_invulnerabilitat = false);
void processInput(float delta_time, uint8_t player_id);
void update(float delta_time) override;
void postUpdate(float delta_time) override;
void draw() const override;
void init() override { init(nullptr, false); }
void init(const Vec2* spawn_point, bool activar_invulnerabilitat = false);
void processInput(float delta_time, uint8_t player_id);
void update(float delta_time) override;
void postUpdate(float delta_time) override;
void draw() const override;
// Override: Interfaz de Entity
[[nodiscard]] auto isActive() const -> bool override { return !is_hit_; }
// Override: Interfaz de Entity
[[nodiscard]] auto isActive() const -> bool override { return !is_hit_; }
// Override: Interfaz de colisión
[[nodiscard]] auto getCollisionRadius() const -> float override {
return Defaults::Entities::SHIP_RADIUS;
}
[[nodiscard]] auto isCollidable() const -> bool override {
return !is_hit_ && invulnerable_timer_ <= 0.0F;
}
// Override: Interfaz de colisión
[[nodiscard]] auto getCollisionRadius() const -> float override {
return Defaults::Entities::SHIP_RADIUS;
}
[[nodiscard]] auto isCollidable() const -> bool override {
return !is_hit_ && invulnerable_timer_ <= 0.0F;
}
// Getters (API pública sin cambios)
[[nodiscard]] auto isAlive() const -> bool { return !is_hit_; }
[[nodiscard]] auto isHit() const -> bool { return is_hit_; }
[[nodiscard]] auto isInvulnerable() const -> bool { return invulnerable_timer_ > 0.0F; }
// Velocidad como vector cartesiano (ahora viene directa del body_).
[[nodiscard]] auto getVelocityVector() const -> Vec2 { return body_.velocity; }
// Velocidad escalar (utilidad para draw y debugging).
[[nodiscard]] auto getSpeed() const -> float { return body_.velocity.length(); }
// Getters
[[nodiscard]] auto isInvulnerable() const -> bool { return invulnerable_timer_ > 0.0F; }
// Velocidad como vector cartesiano (ahora viene directa del body_).
[[nodiscard]] auto getVelocityVector() const -> Vec2 { return body_.velocity; }
// Velocidad escalar (utilidad para draw y debugging).
[[nodiscard]] auto getSpeed() const -> float { return body_.velocity.length(); }
// Setters
void setCenter(const Vec2& nou_centre) {
center_ = nou_centre;
body_.position = nou_centre;
}
// Setters
void setCenter(const Vec2& nou_centre) {
center_ = nou_centre;
body_.position = nou_centre;
}
// Colisiones
void markHit() {
is_hit_ = true;
body_.velocity = Vec2{}; // Detener al morir
}
// Colisiones
void markHit() {
is_hit_ = true;
body_.velocity = Vec2{}; // Detener al morir
}
private:
// Miembros específicos de Ship (heredados: renderer_, shape_, center_, angle_, brightness_, body_).
// Inicializados en la declaración: el ctor por defecto deja la nave "viva y sin invulnerabilidad",
// que es el estado coherente al que llevan tanto init() como el ctor con renderer.
bool is_hit_{false};
float invulnerable_timer_{0.0F}; // 0.0f = vulnerable, >0.0f = invulnerable
private:
// Miembros específicos de Ship (heredados: renderer_, shape_, center_, angle_, brightness_, body_).
// Inicializados en la declaración: el ctor por defecto deja la nave "viva y sin invulnerabilidad",
// que es el estado coherente al que llevan tanto init() como el ctor con renderer.
bool is_hit_{false};
float invulnerable_timer_{0.0F}; // 0.0f = vulnerable, >0.0f = invulnerable
};
+3 -3
View File
@@ -573,11 +573,11 @@ void GameScene::drawInitHudState() {
Systems::InitHud::drawScoreboardAnimated(text_, buildScoreboard(), score_progress);
}
if (ship1_progress > 0.0F && match_config_.jugador1_actiu && !ships_[0].isHit()) {
if (ship1_progress > 0.0F && match_config_.jugador1_actiu && ships_[0].isActive()) {
ships_[0].draw();
}
if (ship2_progress > 0.0F && match_config_.jugador2_actiu && !ships_[1].isHit()) {
if (ship2_progress > 0.0F && match_config_.jugador2_actiu && ships_[1].isActive()) {
ships_[1].draw();
}
}
@@ -820,7 +820,7 @@ void GameScene::fireBullet(uint8_t player_id) {
if (hit_timer_per_player_[player_id] > 0.0F) {
return;
}
if (!ships_[player_id].isAlive()) {
if (!ships_[player_id].isActive()) {
return;
}
+123 -118
View File
@@ -10,141 +10,146 @@
namespace Systems::Collision {
void detectBulletEnemy(Context& ctx) {
constexpr float AMPLIFIER = Defaults::Game::COLLISION_BULLET_ENEMY_AMPLIFIER;
constexpr float VELOCITAT_EXPLOSIO = 80.0F; // px/s (explosión suau)
void detectBulletEnemy(Context& ctx) {
constexpr float AMPLIFIER = Defaults::Game::COLLISION_BULLET_ENEMY_AMPLIFIER;
constexpr float VELOCITAT_EXPLOSIO = 80.0F; // px/s (explosión suau)
for (auto& bullet : ctx.bullets) {
for (auto& enemy : ctx.enemies) {
if (!Physics::checkCollision(bullet, enemy, AMPLIFIER)) {
continue;
}
for (auto& bullet : ctx.bullets) {
for (auto& enemy : ctx.enemies) {
if (!Physics::checkCollision(bullet, enemy, AMPLIFIER)) {
continue;
}
// *** COLISIÓN bullet → enemy ***
const Vec2& enemy_pos = enemy.getCenter();
// *** COLISIÓN bullet → enemy ***
const Vec2& enemy_pos = enemy.getCenter();
// 1. Puntos según tipo
int points = 0;
switch (enemy.getType()) {
case EnemyType::PENTAGON:
points = Defaults::Enemies::Scoring::PENTAGON_SCORE;
break;
case EnemyType::QUADRAT:
points = Defaults::Enemies::Scoring::QUADRAT_SCORE;
break;
case EnemyType::MOLINILLO:
points = Defaults::Enemies::Scoring::MOLINILLO_SCORE;
break;
}
// 1. Puntos según tipo
int points = 0;
switch (enemy.getType()) {
case EnemyType::PENTAGON:
points = Defaults::Enemies::Scoring::PENTAGON_SCORE;
break;
case EnemyType::QUADRAT:
points = Defaults::Enemies::Scoring::QUADRAT_SCORE;
break;
case EnemyType::MOLINILLO:
points = Defaults::Enemies::Scoring::MOLINILLO_SCORE;
break;
}
uint8_t owner_id = bullet.getOwnerId();
ctx.score_per_player[owner_id] += points;
ctx.floating_score_manager.crear(points, enemy_pos);
uint8_t owner_id = bullet.getOwnerId();
ctx.score_per_player[owner_id] += points;
ctx.floating_score_manager.crear(points, enemy_pos);
// 2. Destruir enemy + crear explosión (debris hereda color del enemy)
SDL_Color enemy_color{};
switch (enemy.getType()) {
case EnemyType::PENTAGON: enemy_color = Defaults::Palette::PENTAGON; break;
case EnemyType::QUADRAT: enemy_color = Defaults::Palette::QUADRAT; break;
case EnemyType::MOLINILLO: enemy_color = Defaults::Palette::MOLINILLO; break;
}
enemy.destruir();
Vec2 vel_enemic = enemy.getVelocityVector();
ctx.debris_manager.explode(
enemy.getShape(),
enemy_pos,
0.0F, // angle (la rotación es interna del enemy)
1.0F, // escala
VELOCITAT_EXPLOSIO,
enemy.getBrightness(),
vel_enemic,
enemy.getRotationDelta(),
0.0F, // sin herencia visual
Defaults::Sound::EXPLOSION,
enemy_color
);
// 2. Destruir enemy + crear explosión (debris hereda color del enemy)
SDL_Color enemy_color{};
switch (enemy.getType()) {
case EnemyType::PENTAGON:
enemy_color = Defaults::Palette::PENTAGON;
break;
case EnemyType::QUADRAT:
enemy_color = Defaults::Palette::QUADRAT;
break;
case EnemyType::MOLINILLO:
enemy_color = Defaults::Palette::MOLINILLO;
break;
}
enemy.destruir();
Vec2 vel_enemic = enemy.getVelocityVector();
ctx.debris_manager.explode(
enemy.getShape(),
enemy_pos,
0.0F, // angle (la rotación es interna del enemy)
1.0F, // escala
VELOCITAT_EXPLOSIO,
enemy.getBrightness(),
vel_enemic,
enemy.getRotationDelta(),
0.0F, // sin herencia visual
Defaults::Sound::EXPLOSION,
enemy_color);
// 3. Desactivar bullet (solo destruye 1 enemy)
bullet.desactivar();
break;
}
}
}
void detectShipEnemy(Context& ctx) {
constexpr float AMPLIFIER = Defaults::Game::COLLISION_SHIP_ENEMY_AMPLIFIER;
for (uint8_t i = 0; i < 2; i++) {
// Skip si ya tocado / muerto / invulnerable
if (ctx.hit_timer_per_player[i] > 0.0F ||
!ctx.ships[i].isAlive() ||
ctx.ships[i].isInvulnerable()) {
continue;
}
for (const auto& enemy : ctx.enemies) {
if (enemy.isInvulnerable()) {
continue;
}
if (Physics::checkCollision(ctx.ships[i], enemy, AMPLIFIER)) {
ctx.on_player_hit(i);
break; // Solo una colisión por player por frame
// 3. Desactivar bullet (solo destruye 1 enemy)
bullet.desactivar();
break;
}
}
}
}
void detectBulletPlayer(Context& ctx) {
if (!Defaults::Game::FRIENDLY_FIRE_ENABLED) {
return;
}
void detectShipEnemy(Context& ctx) {
constexpr float AMPLIFIER = Defaults::Game::COLLISION_SHIP_ENEMY_AMPLIFIER;
constexpr float AMPLIFIER = Defaults::Game::COLLISION_BULLET_PLAYER_AMPLIFIER;
for (auto& bullet : ctx.bullets) {
if (!bullet.isActive() || bullet.getGraceTimer() > 0.0F) {
continue;
}
const uint8_t BULLET_OWNER = bullet.getOwnerId();
for (uint8_t player_id = 0; player_id < 2; player_id++) {
if (ctx.hit_timer_per_player[player_id] > 0.0F ||
!ctx.ships[player_id].isAlive() ||
ctx.ships[player_id].isInvulnerable()) {
continue;
}
const bool JUGADOR_ACTIU = (player_id == 0)
? ctx.match_config.jugador1_actiu
: ctx.match_config.jugador2_actiu;
if (!JUGADOR_ACTIU) {
for (uint8_t i = 0; i < 2; i++) {
// Skip si ya tocado / muerto / invulnerable
if (ctx.hit_timer_per_player[i] > 0.0F ||
!ctx.ships[i].isActive() ||
ctx.ships[i].isInvulnerable()) {
continue;
}
if (!Physics::checkCollision(bullet, ctx.ships[player_id], AMPLIFIER)) {
continue;
for (const auto& enemy : ctx.enemies) {
if (enemy.isInvulnerable()) {
continue;
}
if (Physics::checkCollision(ctx.ships[i], enemy, AMPLIFIER)) {
ctx.on_player_hit(i);
break; // Solo una colisión por player por frame
}
}
// *** FRIENDLY FIRE HIT ***
if (BULLET_OWNER == player_id) {
// Self-hit: víctima pierde 1 vida.
ctx.on_player_hit(player_id);
} else {
// Teammate hit: víctima pierde 1, atacante gana 1.
ctx.on_player_hit(player_id);
ctx.lives_per_player[BULLET_OWNER]++;
}
Audio::get()->playSound(Defaults::Sound::FRIENDLY_FIRE_HIT, Audio::Group::GAME);
bullet.desactivar();
break; // Una bullet solo impacta una vez por frame
}
}
}
void detectAll(Context& ctx) {
detectBulletEnemy(ctx);
detectShipEnemy(ctx);
detectBulletPlayer(ctx);
}
void detectBulletPlayer(Context& ctx) {
if (!Defaults::Game::FRIENDLY_FIRE_ENABLED) {
return;
}
constexpr float AMPLIFIER = Defaults::Game::COLLISION_BULLET_PLAYER_AMPLIFIER;
for (auto& bullet : ctx.bullets) {
if (!bullet.isActive() || bullet.getGraceTimer() > 0.0F) {
continue;
}
const uint8_t BULLET_OWNER = bullet.getOwnerId();
for (uint8_t player_id = 0; player_id < 2; player_id++) {
if (ctx.hit_timer_per_player[player_id] > 0.0F ||
!ctx.ships[player_id].isActive() ||
ctx.ships[player_id].isInvulnerable()) {
continue;
}
const bool JUGADOR_ACTIU = (player_id == 0)
? ctx.match_config.jugador1_actiu
: ctx.match_config.jugador2_actiu;
if (!JUGADOR_ACTIU) {
continue;
}
if (!Physics::checkCollision(bullet, ctx.ships[player_id], AMPLIFIER)) {
continue;
}
// *** FRIENDLY FIRE HIT ***
if (BULLET_OWNER == player_id) {
// Self-hit: víctima pierde 1 vida.
ctx.on_player_hit(player_id);
} else {
// Teammate hit: víctima pierde 1, atacante gana 1.
ctx.on_player_hit(player_id);
ctx.lives_per_player[BULLET_OWNER]++;
}
Audio::get()->playSound(Defaults::Sound::FRIENDLY_FIRE_HIT, Audio::Group::GAME);
bullet.desactivar();
break; // Una bullet solo impacta una vez por frame
}
}
}
void detectAll(Context& ctx) {
detectBulletEnemy(ctx);
detectShipEnemy(ctx);
detectBulletPlayer(ctx);
}
} // namespace Systems::Collision