Skip to content

Refactor PlayerController. #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 25, 2024
Merged

Refactor PlayerController. #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions DogTales/dog/dogtales.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,9 @@ void DogTales::tick() {
if (ImGui::BeginTabItem("Player")) {
bave::im_text("Controller States");
ImGui::Separator();
bave::im_text("move_x (Press A/D or Left/Right): {:.2f}",
m_player.get_controller_state("move_x").value());
bave::im_text("move_y (Press W/S or Up/Down): {:.2f}",
m_player.get_controller_state("move_y").value());
bave::im_text("Jump (Press 'E'): {:.2f}", m_player.get_controller_state("jump").value());
bave::im_text("move_x (Press A/D or Left/Right): {:.2f}", m_player.get_controller_state("move_x"));
bave::im_text("move_y (Press W/S or Up/Down): {:.2f}", m_player.get_controller_state("move_y"));
bave::im_text("Jump (Press Space): {:.2f}", m_player.get_controller_state("jump"));
ImGui::EndTabItem();
}
ImGui::EndTabBar();
Expand Down
2 changes: 1 addition & 1 deletion DogTales/dog/dogtales.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once
#include <bave/driver.hpp>
#include <dog/player.hpp>
#include <dog/player/player.hpp>

namespace dog {
class DogTales : public bave::Driver {
Expand Down
41 changes: 0 additions & 41 deletions DogTales/dog/player/PlayerController.cpp
View file
Open in desktop

This file was deleted.

18 changes: 0 additions & 18 deletions DogTales/dog/player/PlayerController.hpp
View file
Open in desktop

This file was deleted.

17 changes: 10 additions & 7 deletions DogTales/dog/player.cpp → DogTales/dog/player/player.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,24 @@
#include <dog/player.hpp>
#include <dog/player/player.hpp>

namespace dog {
Player::Player(bave::App& app, glm::vec2 const world_space) : m_app(app), m_world_space(world_space) {
m_sprite.set_size(size_v);

m_player_controller.bind_throttle("move_x", {.lo = bave::Key::eLeft, .hi = bave::Key::eRight});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what will happen if both "Left" and "Right" are pressed simultaneously?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Left" will contribute -1.0f to the throttle, "Right" will contribute +1.0f, and they'll cancel each other out.

As you can imagine, this is a subjective/design level decision, rather than being "more technically correct" over any other approach.

m_player_controller.bind_throttle("move_x", {.lo = bave::Key::eA, .hi = bave::Key::eD});
m_player_controller.bind_throttle("move_y", {.lo = bave::Key::eDown, .hi = bave::Key::eUp});
m_player_controller.bind_throttle("move_y", {.lo = bave::Key::eS, .hi = bave::Key::eW});
m_player_controller.bind_key("jump", bave::Key::eSpace);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we wanted to change these in-game (say, settings menu > key binds), how would we go about that? would we have to "unbind" the throttles and keys?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, we would need a way to remove existing bindings as this class is developed.

}

void Player::tick(bave::Seconds const dt) {

if (physics_enabled) { m_physics.tick(dt); }
m_player_controller.tick(dt, m_app);

auto direction = glm::vec2{};

direction.x += m_player_controller.get_controller_state("move_x").value();
direction.y += m_player_controller.get_controller_state("move_y").value();
direction.x += m_player_controller.get_state("move_x");
direction.y += m_player_controller.get_state("move_y");

if (direction.x != 0.0f || direction.y != 0.0f) {
direction = glm::normalize(direction);
Expand All @@ -26,9 +31,7 @@ void Player::tick(bave::Seconds const dt) {

void Player::draw(bave::Shader& shader) const { m_sprite.draw(shader); }

std::optional<float> Player::get_controller_state(std::string_view key) const {
return m_player_controller.get_controller_state(key);
}
float Player::get_controller_state(std::string_view key) const { return m_player_controller.get_state(key); }

void Player::handle_wall_collision() {
auto& position = m_physics.position;
Expand Down
8 changes: 4 additions & 4 deletions DogTales/dog/player.hpp → DogTales/dog/player/player.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#pragma once
#include <bave/app.hpp>
#include <bave/graphics/sprite.hpp>
#include "components/physics.hpp"
#include "player/PlayerController.hpp"
#include <dog/components/physics.hpp>
#include <dog/player/player_controller.hpp>

namespace dog {
class Player {
Expand All @@ -16,7 +16,7 @@ class Player {
bave::Sprite m_sprite{};

component::Physics m_physics{};
PlayerController m_player_controller{};
PlayerController m_player_controller{&m_app};

void handle_wall_collision();

Expand All @@ -26,7 +26,7 @@ class Player {
void tick(bave::Seconds dt);
void draw(bave::Shader& shader) const;

std::optional<float> get_controller_state(std::string_view key) const;
float get_controller_state(std::string_view key) const;

bool physics_enabled{}; // for debugging
};
Expand Down
45 changes: 45 additions & 0 deletions DogTales/dog/player/player_controller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#include <dog/player/player_controller.hpp>
#include <algorithm>
#include <cassert>

namespace dog {

PlayerController::PlayerController(bave::NotNull<bave::App const*> app) : m_app(app) {}

void PlayerController::bind_throttle(std::string_view const id, Throttle const throttle) {
if (throttle.hi == bave::Key::eUnknown) { return; }
auto it = m_mappings.find(id);
if (it == m_mappings.end()) {
auto [i, _] = m_mappings.insert_or_assign(std::string{id}, std::vector<Mapping>{});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you explain the syntax here? what is [i, _] ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a structured binding. You can use it to "destructure" an aggregate type or a std::tuple:

struct Foo {
  int x{};
  char y{};
};

auto f = Foo{42, 'x'};
auto [i, c] = f;
// i == 42, c == 'x'

The underscore is just convention for "I don't care about this member", from the language/compiler's point of view it's just an identifier (name of a variable).

it = i;
}
assert(it != m_mappings.end());
it->second.push_back(throttle);
}

void PlayerController::bind_key(std::string_view id, bave::Key key) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, Throttle is specifically for keys that control a range? if we were making a game with no movement and only jumping, we could theoretically bypass the throttle binding altogether?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, couldn't think of a better name. 😅
And yup, if a game only needs single binary input states (0 or 1), there's no real need to associate a pair of keys to a binding.

// if Throttle::lo is Key::eUnknown it will be ignored in get_state(), so we exploit that here.
bind_throttle(id, Throttle{.hi = key});
}

float PlayerController::get_state(std::string_view const id) const {
auto const search = m_mappings.find(id);
if (search == m_mappings.end()) { return 0.0f; }

auto const& mappings = search->second;
auto const get_throttle_state = [&](Throttle const throttle) {
auto const is_pressed = [this](bave::Key const key) { return m_app->get_key_state().is_pressed(key); };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not too familiar with lambda functions so this part is a bit confusing to me, but I guess the overall idea is to ignore key presses that aren't in our mapping, and for keys that are we just set the throttle based on whether or not they're pressed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah pretty much. Lambdas are very useful! I'd suggest getting familiar, the basics are actually quite straightforward.

if (throttle.lo != bave::Key::eUnknown && is_pressed(throttle.lo)) { return -1.0f; }
if (is_pressed(throttle.hi)) { return 1.0f; }
return 0.0f;
};

auto ret = 0.0f;
for (auto const& mapping : mappings) {
// later a visitor will be required here, to handle Throttle vs GamepadAxis vs GamepadButton.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean by "visitor"? is it a design pattern of some kind?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's a general design pattern, but also the first argument of std::visit for std::variant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://en.cppreference.com/w/cpp/utility/variant/visit

Applies the visitor vis (a Callable that can be called with any combination of types from variants) to the variants vars.

ret += get_throttle_state(mapping);
}
return std::clamp(ret, -1.0f, 1.0f);
}

} // namespace dog
30 changes: 30 additions & 0 deletions DogTales/dog/player/player_controller.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#pragma once
#include <bave/app.hpp>
#include <bave/core/string_hash.hpp>
#include <unordered_map>
#include <vector>

namespace dog {
class PlayerController {

public:
struct Throttle {
bave::Key lo{}; // -1
bave::Key hi{}; // +1
};

explicit PlayerController(bave::NotNull<bave::App const*> app);

void bind_throttle(std::string_view id, Throttle throttle);
void bind_key(std::string_view id, bave::Key key);

float get_state(std::string_view id) const;

private:
bave::NotNull<bave::App const*> m_app;

using Mapping = Throttle; // later to be a variant of Throttle / GamepadAxis / GamepadButton.

std::unordered_map<std::string, std::vector<Mapping>, bave::StringHash, std::equal_to<>> m_mappings{};
};
} // namespace dog