From 1947694e25cbe59379f163702495e5029ded86d3 Mon Sep 17 00:00:00 2001 From: Luke Butters Date: Fri, 29 Sep 2023 19:33:42 +1000 Subject: [PATCH 1/4] Player status is now typed. Previously player direction and status (moving/attacking/idle) was all represented as a string. This replaces that with a type and enums. TODO: add testing --- CMakeLists.txt | 3 +- src/ParticleFactory.cpp | 8 +++--- src/ParticleFactory.h | 3 +- src/Player.cpp | 50 ++++++++++++--------------------- src/Player.h | 5 ++-- src/PlayerStatus.cpp | 62 +++++++++++++++++++++++++++++++++++++++++ src/PlayerStatus.h | 37 ++++++++++++++++++++++++ src/Weapon.cpp | 8 +++--- 8 files changed, 132 insertions(+), 44 deletions(-) create mode 100644 src/PlayerStatus.cpp create mode 100644 src/PlayerStatus.h diff --git a/CMakeLists.txt b/CMakeLists.txt index aef4aca..e34ba38 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -114,7 +114,8 @@ add_library(ZeldaLibrary STATIC src/Tile.cpp src/Particles.h src/Particles.cpp -) + src/PlayerStatus.h + src/PlayerStatus.cpp) target_link_libraries(ZeldaLibrary PUBLIC sfml-system diff --git a/src/ParticleFactory.cpp b/src/ParticleFactory.cpp index d448726..29348f1 100644 --- a/src/ParticleFactory.cpp +++ b/src/ParticleFactory.cpp @@ -60,23 +60,23 @@ void ParticleFactory::CreateHealParticles(const sf::FloatRect& initiatorBounds, group.Add(effect2); } -void ParticleFactory::CreateFlameParticles(const sf::FloatRect& initiatorBounds, const std::string& initiatorDirection, Group& visibleGroup, Group& attackGroup) +void ParticleFactory::CreateFlameParticles(const sf::FloatRect& initiatorBounds, const PlayerDirection initiatorDirection, Group& visibleGroup, Group& attackGroup) { auto effect = std::make_shared(mGroupManager, "magic", "flame"); sf::Vector2f startPosition; sf::Vector2f direction; - if (initiatorDirection == "right") + if (initiatorDirection == PlayerDirection::RIGHT) { startPosition = GetRectMidRight(initiatorBounds) - GetRectMidLeft(effect->GetLocalBounds()); direction = sf::Vector2f(1, 0); } - else if (initiatorDirection == "left") + else if (initiatorDirection == PlayerDirection::LEFT) { startPosition = GetRectMidLeft(initiatorBounds) - GetRectMidRight(effect->GetLocalBounds()); direction = sf::Vector2f(-1, 0); } - else if (initiatorDirection == "down") + else if (initiatorDirection == PlayerDirection::DOWN) { startPosition = GetRectMidBottom(initiatorBounds) - GetRectMidTop(effect->GetLocalBounds()); direction = sf::Vector2f(0, 1); diff --git a/src/ParticleFactory.h b/src/ParticleFactory.h index 8278a73..aed47d5 100644 --- a/src/ParticleFactory.h +++ b/src/ParticleFactory.h @@ -1,6 +1,7 @@ #pragma once #include +#include "PlayerStatus.h" // Forward declarations class GroupManager; @@ -16,7 +17,7 @@ class ParticleFactory void CreateEnemyDeathParticles(std::string enemyName, const sf::FloatRect& initiatorBounds, Group& group); void CreateEnemyAttackParticles(std::string attackName, const sf::FloatRect& initiatorBounds, Group& group); void CreateHealParticles(const sf::FloatRect& initiatorBounds, Group& group); - void CreateFlameParticles(const sf::FloatRect& initiatorBounds, const std::string& initiatorDirection, Group& visibleGroup, Group& attackGroup); + void CreateFlameParticles(const sf::FloatRect& initiatorBounds, const PlayerDirection initiatorDirection, Group& visibleGroup, Group& attackGroup); private: GroupManager& mGroupManager; diff --git a/src/Player.cpp b/src/Player.cpp index 600a232..42a2c5b 100644 --- a/src/Player.cpp +++ b/src/Player.cpp @@ -17,12 +17,12 @@ Player::Player(GroupManager& groupManager, sf::Vector2f position, const Group& obstacles, IPlayerCallbacks& callbacks) : Entity(groupManager, obstacles), - mStatus("down"), + mStatus(PlayerDirection::DOWN), mAttackStatus(AttackStatus::NONE), mIsAttacking(400, false), mCanSwitchWeapons(TOGGLE_COOLDONW_MS, true), mCanSwitchMagic(TOGGLE_COOLDONW_MS, true), - mVulnerable(500, true), + mVulnerable(1, true), mAnimation(AnimationManager::GetInstance().LoadUnique("player")), mCallbacks(callbacks), mWeaponIndex(0), @@ -34,7 +34,7 @@ Player::Player(GroupManager& groupManager, sf::Vector2f position, const Group& o mMagic(PLAYER_STATS.at("magic")), mEXP(123) { - mAnimation->SetAnimationSequence(mStatus); + mAnimation->SetAnimationSequence(mStatus.AsCompatString()); UpdateSequenceFrame(); SetPosition(position); mHitBox = InflateRect(GetGlobalBounds(), 0, -26); @@ -50,14 +50,9 @@ void Player::Update(const sf::Time ×tamp) RecoverEnergy(); } -std::string Player::GetDirection() const +PlayerDirection Player::GetDirection() const { - size_t index = mStatus.find('_'); - if (index == std::string::npos) - { - return mStatus; - } - return mStatus.substr(0, index); + return mStatus.GetPlayerDirection(); } uint16_t Player::GetFullWeaponDamage() const @@ -85,13 +80,12 @@ void Player::AddEnergy(float value) std::shared_ptr Player::GetWeaponTexture() const { std::ostringstream oss; - oss << GetWeaponName() << "_" << GetDirection(); + oss << GetWeaponName() << "_" << mStatus.GetDirectionAsString(); return TextureManager::GetInstance().GetResource(oss.str()); } std::shared_ptr Player::GetWeaponIconTexture() const { - std::string direction = GetDirection(); std::ostringstream oss; oss << GetWeaponName() << "_full"; return TextureManager::GetInstance().GetResource(oss.str()); @@ -110,12 +104,12 @@ void Player::Input() if (sf::Keyboard::isKeyPressed(sf::Keyboard::Key::Up)) { mDirection.y = -1; - mStatus = "up"; + mStatus.UpdatePlayerDirection(PlayerDirection::UP); } else if (sf::Keyboard::isKeyPressed(sf::Keyboard::Key::Down)) { mDirection.y = 1; - mStatus = "down"; + mStatus.UpdatePlayerDirection(PlayerDirection::DOWN); } else { @@ -125,12 +119,12 @@ void Player::Input() if (sf::Keyboard::isKeyPressed(sf::Keyboard::Key::Right)) { mDirection.x = 1; - mStatus = "right"; + mStatus.UpdatePlayerDirection(PlayerDirection::RIGHT); } else if (sf::Keyboard::isKeyPressed(sf::Keyboard::Key::Left)) { mDirection.x = -1; - mStatus = "left"; + mStatus.UpdatePlayerDirection(PlayerDirection::LEFT); } else { @@ -169,9 +163,8 @@ void Player::UpdateStatus() { if (mDirection.lengthSq() == 0) { - if (!isSubstring(mStatus, "idle") && !isSubstring(mStatus, "attack")) - { - mStatus += "_idle"; + if (mStatus.GetPlayerActiveStatus() == PlayerActiveStatus::MOVING) { + mStatus.SetPlayerToIdle(); } } @@ -179,23 +172,16 @@ void Player::UpdateStatus() { mDirection.x = 0; mDirection.y = 0; - if (!isSubstring(mStatus, "attack")) + if (mStatus.GetPlayerActiveStatus() != PlayerActiveStatus::ATTACKING) { - if (isSubstring(mStatus, "idle")) - { - replaceSubstring(mStatus, "_idle", "_attack"); - } - else - { - mStatus += "_attack"; - } + mStatus.SetPlayerToAttacking(); } } else { - if (isSubstring(mStatus, "attack")) + if (mStatus.GetPlayerActiveStatus() == PlayerActiveStatus::ATTACKING) { - replaceSubstring(mStatus, "_attack", ""); + mStatus.SetPlayerToIdle(); } } } @@ -227,9 +213,9 @@ void Player::Cooldowns(const sf::Time ×tamp) void Player::Animate(const sf::Time ×tamp) { - if (mStatus != mAnimation->GetSequenceID()) + if (mStatus.AsCompatString() != mAnimation->GetSequenceID()) { - mAnimation->SetAnimationSequence(mStatus); + mAnimation->SetAnimationSequence(mStatus.AsCompatString()); } mAnimation->Update(timestamp); diff --git a/src/Player.h b/src/Player.h index e65e204..fddaace 100644 --- a/src/Player.h +++ b/src/Player.h @@ -15,6 +15,7 @@ #include "Support.h" #include "Settings.h" #include "Entity.h" +#include "PlayerStatus.h" enum class AttackStatus { @@ -51,7 +52,7 @@ class Player : public Entity // Getters virtual sf::FloatRect GetHitbox() const override { return mHitBox; } - std::string GetDirection() const; + PlayerDirection GetDirection() const; uint16_t GetHealth() const { return mHealth; } uint16_t GetEnergy() const { return mEnergy; } uint16_t GetEXP() const { return mEXP; } @@ -84,7 +85,7 @@ class Player : public Entity void UpdateSequenceFrame(); private: - std::string mStatus; + PlayerStatus mStatus; AttackStatus mAttackStatus; CooldownToggle mIsAttacking; CooldownToggle mCanSwitchWeapons; diff --git a/src/PlayerStatus.cpp b/src/PlayerStatus.cpp new file mode 100644 index 0000000..97d6790 --- /dev/null +++ b/src/PlayerStatus.cpp @@ -0,0 +1,62 @@ +#include "PlayerStatus.h" + +PlayerStatus::PlayerStatus(PlayerDirection playerDirection) : + mPlayerDirection(playerDirection), + mPlayerActiveStatus(PlayerActiveStatus::IDLE) +{ +} + +PlayerDirection PlayerStatus::GetPlayerDirection() const { + return mPlayerDirection; +} + +PlayerActiveStatus PlayerStatus::GetPlayerActiveStatus() const { + return mPlayerActiveStatus; +} + +void PlayerStatus::UpdatePlayerDirection(PlayerDirection playerDirection) +{ + mPlayerActiveStatus = PlayerActiveStatus::MOVING; + mPlayerDirection = playerDirection; +} + +void PlayerStatus::SetPlayerToIdle() +{ + mPlayerActiveStatus = PlayerActiveStatus::IDLE; +} + +void PlayerStatus::SetPlayerToAttacking() +{ + mPlayerActiveStatus = PlayerActiveStatus::ATTACKING; +} + +std::string PlayerStatus::GetDirectionAsString() const { + switch (mPlayerDirection) { + case PlayerDirection::UP: return "up"; + case PlayerDirection::DOWN: return "down"; + case PlayerDirection::LEFT: return "left"; + case PlayerDirection::RIGHT: return "right"; + } + // What do? + return "up"; +} + +std::string PlayerStatus::GetPlayerActoveStatusPostfix() const { + switch (mPlayerActiveStatus) + { + case PlayerActiveStatus::IDLE: + return "_idle"; + case PlayerActiveStatus::MOVING: + return ""; + case PlayerActiveStatus::ATTACKING: + return "_attack"; + break; + default: + break; + } + return ""; +} + +std::string PlayerStatus::AsCompatString() const { + return GetDirectionAsString() + GetPlayerActoveStatusPostfix(); +} diff --git a/src/PlayerStatus.h b/src/PlayerStatus.h new file mode 100644 index 0000000..357c48f --- /dev/null +++ b/src/PlayerStatus.h @@ -0,0 +1,37 @@ +#pragma once +#include + +enum class PlayerDirection +{ + UP, + DOWN, + LEFT, + RIGHT +}; + +enum class PlayerActiveStatus { + IDLE, + MOVING, // Should this be "moving"? + ATTACKING +}; + +class PlayerStatus { +private: + PlayerDirection mPlayerDirection; + PlayerActiveStatus mPlayerActiveStatus; + std::string PlayerStatus::GetPlayerActoveStatusPostfix() const; + +public: + + PlayerStatus(PlayerDirection playerDirection); + PlayerDirection GetPlayerDirection() const; + PlayerActiveStatus GetPlayerActiveStatus() const; + void UpdatePlayerDirection(PlayerDirection playerDirection); + void SetPlayerToIdle(); + void SetPlayerToAttacking(); + std::string PlayerStatus::GetDirectionAsString() const; + std::string PlayerStatus::AsCompatString() const; +}; + + + diff --git a/src/Weapon.cpp b/src/Weapon.cpp index d99c22a..49ad214 100644 --- a/src/Weapon.cpp +++ b/src/Weapon.cpp @@ -16,16 +16,16 @@ Weapon::Weapon(GroupManager& groupManager, const Player &player) { SetTexture(player.GetWeaponTexture(), true); - std::string direction = player.GetDirection(); - if (direction == "right") + PlayerDirection direction = player.GetDirection(); + if (direction == PlayerDirection::RIGHT) { SetPosition(GetRectMidRight(player.GetGlobalBounds()) - GetRectMidLeft(GetLocalBounds()) + sf::Vector2f(0, 16)); } - else if (direction == "left") + else if (direction == PlayerDirection::LEFT) { SetPosition(GetRectMidLeft(player.GetGlobalBounds()) - GetRectMidRight(GetLocalBounds()) + sf::Vector2f(0, 16)); } - else if (direction == "down") + else if (direction == PlayerDirection::DOWN) { SetPosition(GetRectMidBottom(player.GetGlobalBounds()) - GetRectMidTop(GetLocalBounds()) + sf::Vector2f(-10, 0)); } From 1b82b5194a01c6f578661cc00d8aaf2387e3e07e Mon Sep 17 00:00:00 2001 From: Luke Butters Date: Sun, 1 Oct 2023 15:00:01 +1100 Subject: [PATCH 2/4] Add tests, and fix some PR comments --- src/ParticleFactory.cpp | 2 +- src/ParticleFactory.h | 2 +- src/PlayerStatus.cpp | 32 +++++++++----------------------- src/PlayerStatus.h | 12 ++++++------ tests/CMakeLists.txt | 2 +- 5 files changed, 18 insertions(+), 32 deletions(-) diff --git a/src/ParticleFactory.cpp b/src/ParticleFactory.cpp index 29348f1..4328fb7 100644 --- a/src/ParticleFactory.cpp +++ b/src/ParticleFactory.cpp @@ -60,7 +60,7 @@ void ParticleFactory::CreateHealParticles(const sf::FloatRect& initiatorBounds, group.Add(effect2); } -void ParticleFactory::CreateFlameParticles(const sf::FloatRect& initiatorBounds, const PlayerDirection initiatorDirection, Group& visibleGroup, Group& attackGroup) +void ParticleFactory::CreateFlameParticles(const sf::FloatRect& initiatorBounds, PlayerDirection initiatorDirection, Group& visibleGroup, Group& attackGroup) { auto effect = std::make_shared(mGroupManager, "magic", "flame"); diff --git a/src/ParticleFactory.h b/src/ParticleFactory.h index aed47d5..93d3084 100644 --- a/src/ParticleFactory.h +++ b/src/ParticleFactory.h @@ -17,7 +17,7 @@ class ParticleFactory void CreateEnemyDeathParticles(std::string enemyName, const sf::FloatRect& initiatorBounds, Group& group); void CreateEnemyAttackParticles(std::string attackName, const sf::FloatRect& initiatorBounds, Group& group); void CreateHealParticles(const sf::FloatRect& initiatorBounds, Group& group); - void CreateFlameParticles(const sf::FloatRect& initiatorBounds, const PlayerDirection initiatorDirection, Group& visibleGroup, Group& attackGroup); + void CreateFlameParticles(const sf::FloatRect& initiatorBounds, PlayerDirection initiatorDirection, Group& visibleGroup, Group& attackGroup); private: GroupManager& mGroupManager; diff --git a/src/PlayerStatus.cpp b/src/PlayerStatus.cpp index 97d6790..27ece67 100644 --- a/src/PlayerStatus.cpp +++ b/src/PlayerStatus.cpp @@ -1,4 +1,5 @@ #include "PlayerStatus.h" +#include PlayerStatus::PlayerStatus(PlayerDirection playerDirection) : mPlayerDirection(playerDirection), @@ -6,42 +7,26 @@ PlayerStatus::PlayerStatus(PlayerDirection playerDirection) : { } -PlayerDirection PlayerStatus::GetPlayerDirection() const { - return mPlayerDirection; -} - -PlayerActiveStatus PlayerStatus::GetPlayerActiveStatus() const { - return mPlayerActiveStatus; -} - void PlayerStatus::UpdatePlayerDirection(PlayerDirection playerDirection) { mPlayerActiveStatus = PlayerActiveStatus::MOVING; mPlayerDirection = playerDirection; } -void PlayerStatus::SetPlayerToIdle() +std::string PlayerStatus::GetDirectionAsString() const { - mPlayerActiveStatus = PlayerActiveStatus::IDLE; -} - -void PlayerStatus::SetPlayerToAttacking() -{ - mPlayerActiveStatus = PlayerActiveStatus::ATTACKING; -} - -std::string PlayerStatus::GetDirectionAsString() const { switch (mPlayerDirection) { case PlayerDirection::UP: return "up"; case PlayerDirection::DOWN: return "down"; case PlayerDirection::LEFT: return "left"; case PlayerDirection::RIGHT: return "right"; + default: + throw std::runtime_error("Unexpected enum value in GetDirectionAsString"); } - // What do? - return "up"; } -std::string PlayerStatus::GetPlayerActoveStatusPostfix() const { +std::string PlayerStatus::GetPlayerActiveStatusPostfix() const +{ switch (mPlayerActiveStatus) { case PlayerActiveStatus::IDLE: @@ -57,6 +42,7 @@ std::string PlayerStatus::GetPlayerActoveStatusPostfix() const { return ""; } -std::string PlayerStatus::AsCompatString() const { - return GetDirectionAsString() + GetPlayerActoveStatusPostfix(); +std::string PlayerStatus::AsCompatString() const +{ + return GetDirectionAsString() + GetPlayerActiveStatusPostfix(); } diff --git a/src/PlayerStatus.h b/src/PlayerStatus.h index 357c48f..de24620 100644 --- a/src/PlayerStatus.h +++ b/src/PlayerStatus.h @@ -11,7 +11,7 @@ enum class PlayerDirection enum class PlayerActiveStatus { IDLE, - MOVING, // Should this be "moving"? + MOVING, ATTACKING }; @@ -19,16 +19,16 @@ class PlayerStatus { private: PlayerDirection mPlayerDirection; PlayerActiveStatus mPlayerActiveStatus; - std::string PlayerStatus::GetPlayerActoveStatusPostfix() const; + std::string PlayerStatus::GetPlayerActiveStatusPostfix() const; public: PlayerStatus(PlayerDirection playerDirection); - PlayerDirection GetPlayerDirection() const; - PlayerActiveStatus GetPlayerActiveStatus() const; + PlayerDirection GetPlayerDirection() const { return mPlayerDirection; }; + PlayerActiveStatus GetPlayerActiveStatus() const { return mPlayerActiveStatus; }; void UpdatePlayerDirection(PlayerDirection playerDirection); - void SetPlayerToIdle(); - void SetPlayerToAttacking(); + void SetPlayerToIdle() { mPlayerActiveStatus = PlayerActiveStatus::IDLE; }; + void SetPlayerToAttacking() { mPlayerActiveStatus = PlayerActiveStatus::ATTACKING; }; std::string PlayerStatus::GetDirectionAsString() const; std::string PlayerStatus::AsCompatString() const; }; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 32e78b2..1f73cc9 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,6 +1,6 @@ add_executable(ZeldaTests Example.cpp -) + PlayerStatusTest.cpp) target_link_libraries(ZeldaTests PUBLIC gtest_main From 61a342bd9aa60751e05af8ea34e08215314af512 Mon Sep 17 00:00:00 2001 From: Luke Butters Date: Sun, 1 Oct 2023 15:12:48 +1100 Subject: [PATCH 3/4] Throw on unknown PlayerActiveStatus --- src/PlayerStatus.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/PlayerStatus.cpp b/src/PlayerStatus.cpp index 27ece67..ac959d2 100644 --- a/src/PlayerStatus.cpp +++ b/src/PlayerStatus.cpp @@ -37,9 +37,8 @@ std::string PlayerStatus::GetPlayerActiveStatusPostfix() const return "_attack"; break; default: - break; + throw std::runtime_error("Unexpected enum value in GetPlayerActiveStatusPostfix"); } - return ""; } std::string PlayerStatus::AsCompatString() const From 311c67b0f35f79254217f84390dbf933d682686b Mon Sep 17 00:00:00 2001 From: Luke Butters Date: Tue, 3 Oct 2023 13:12:59 +1100 Subject: [PATCH 4/4] Unit tests for player direction --- tests/PlayerStatusTest.cpp | 57 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 tests/PlayerStatusTest.cpp diff --git a/tests/PlayerStatusTest.cpp b/tests/PlayerStatusTest.cpp new file mode 100644 index 0000000..ab517ab --- /dev/null +++ b/tests/PlayerStatusTest.cpp @@ -0,0 +1,57 @@ +#include +#include "PlayerStatus.h" + +TEST(PlayerStatus, WhenPlayersDirectionIsDown_ThePlayerIsMovingDown) +{ + PlayerStatus playerStatus(PlayerDirection::DOWN); + playerStatus.UpdatePlayerDirection(PlayerDirection::DOWN); + + EXPECT_EQ(playerStatus.GetDirectionAsString(), "down"); + EXPECT_EQ(playerStatus.AsCompatString(), "down"); +} + +TEST(PlayerStatus, WhenPlayersDirectionIsLeft_ThePlayerIsMovingLeft) +{ + PlayerStatus playerStatus(PlayerDirection::LEFT); + playerStatus.UpdatePlayerDirection(PlayerDirection::LEFT); + + EXPECT_EQ(playerStatus.GetDirectionAsString(), "left"); + EXPECT_EQ(playerStatus.AsCompatString(), "left"); +} + + +TEST(PlayerStatus, WhenPlayersDirectionIsRight_ThePlayerIsMovingRight) +{ + PlayerStatus playerStatus(PlayerDirection::RIGHT); + playerStatus.UpdatePlayerDirection(PlayerDirection::RIGHT); + + EXPECT_EQ(playerStatus.GetDirectionAsString(), "right"); + EXPECT_EQ(playerStatus.AsCompatString(), "right"); +} + +TEST(PlayerStatus, WhenPlayersDirectionIsUp_ThePlayerIsMovingUp) +{ + PlayerStatus playerStatus(PlayerDirection::UP); + playerStatus.UpdatePlayerDirection(PlayerDirection::UP); + + EXPECT_EQ(playerStatus.GetDirectionAsString(), "up"); + EXPECT_EQ(playerStatus.AsCompatString(), "up"); +} + +TEST(PlayerStatus, WhenPlayersWasMovingLeftAndHasSinceBecomeIdle_ThePlayerCompatStringIsLeftIdle) +{ + PlayerStatus playerStatus(PlayerDirection::LEFT); + playerStatus.UpdatePlayerDirection(PlayerDirection::LEFT); + playerStatus.SetPlayerToIdle(); + + EXPECT_EQ(playerStatus.AsCompatString(), "left_idle"); +} + +TEST(PlayerStatus, WhenPlayersIsAttackingRight_ThePlayerCompatStringIsRightAttack) +{ + PlayerStatus playerStatus(PlayerDirection::LEFT); + playerStatus.UpdatePlayerDirection(PlayerDirection::RIGHT); + playerStatus.SetPlayerToAttacking(); + + EXPECT_EQ(playerStatus.AsCompatString(), "right_attack"); +} \ No newline at end of file