From e5a924a1fa6aa594f75a0213a25c9e8eb61886eb Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Sun, 28 Jan 2024 03:35:24 -0700 Subject: [PATCH 1/6] Mark key modifier methods as `static` --- NAS2D/EventHandler.cpp | 8 ++++---- NAS2D/EventHandler.h | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/NAS2D/EventHandler.cpp b/NAS2D/EventHandler.cpp index d9eb8ead4..bd4a629ef 100644 --- a/NAS2D/EventHandler.cpp +++ b/NAS2D/EventHandler.cpp @@ -748,7 +748,7 @@ bool EventHandler::textInputMode() * * \param mod Modifier value to decode. */ -bool EventHandler::shift(KeyModifier mod) const +bool EventHandler::shift(KeyModifier mod) { return KeyModifier::None != (mod & (KeyModifier::Shift | KeyModifier::Caps)); } @@ -759,7 +759,7 @@ bool EventHandler::shift(KeyModifier mod) const * * \param mod Modifier value to decode. */ -bool EventHandler::alt(KeyModifier mod) const +bool EventHandler::alt(KeyModifier mod) { return KeyModifier::None != (mod & KeyModifier::Alt); } @@ -770,7 +770,7 @@ bool EventHandler::alt(KeyModifier mod) const * * \param mod Modifier value to decode. */ -bool EventHandler::numlock(KeyModifier mod) const +bool EventHandler::numlock(KeyModifier mod) { return KeyModifier::None != (mod & KeyModifier::Num); } @@ -781,7 +781,7 @@ bool EventHandler::numlock(KeyModifier mod) const * * \param mod Modifier value to decode. */ -bool EventHandler::control(KeyModifier mod) const +bool EventHandler::control(KeyModifier mod) { return KeyModifier::None != (mod & KeyModifier::Ctrl); } diff --git a/NAS2D/EventHandler.h b/NAS2D/EventHandler.h index fff1ed5bf..ddcf3075a 100644 --- a/NAS2D/EventHandler.h +++ b/NAS2D/EventHandler.h @@ -317,10 +317,10 @@ namespace NAS2D void textInputMode(bool); bool textInputMode(); - bool shift(KeyModifier mod) const; - bool numlock(KeyModifier mod) const; - bool control(KeyModifier mod) const; - bool alt(KeyModifier mod) const; + static bool shift(KeyModifier mod); + static bool numlock(KeyModifier mod); + static bool control(KeyModifier mod); + static bool alt(KeyModifier mod); bool query_shift() const; bool query_numlock() const; From 629aaed4f81a2bbacfa800a9792813154b8e9a91 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Sun, 28 Jan 2024 03:35:53 -0700 Subject: [PATCH 2/6] Add unit tests for key modifier methods --- test/EventHandler.test.cpp | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 test/EventHandler.test.cpp diff --git a/test/EventHandler.test.cpp b/test/EventHandler.test.cpp new file mode 100644 index 000000000..1148fe862 --- /dev/null +++ b/test/EventHandler.test.cpp @@ -0,0 +1,36 @@ +#include "NAS2D/EventHandler.h" + +#include + + +TEST(EventHandler, shift) { + EXPECT_TRUE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::Shift)); + EXPECT_TRUE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::ShiftLeft)); + EXPECT_TRUE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::ShiftRight)); + EXPECT_TRUE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::ShiftLeft | NAS2D::EventHandler::KeyModifier::CtrlLeft)); + EXPECT_TRUE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::Caps)); + EXPECT_FALSE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::Ctrl)); + EXPECT_FALSE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::Alt)); +} + +TEST(EventHandler, numlock) { + EXPECT_TRUE(NAS2D::EventHandler::numlock(NAS2D::EventHandler::KeyModifier::Num)); + EXPECT_TRUE(NAS2D::EventHandler::numlock(NAS2D::EventHandler::KeyModifier::Num | NAS2D::EventHandler::KeyModifier::CtrlLeft)); + EXPECT_FALSE(NAS2D::EventHandler::numlock(NAS2D::EventHandler::KeyModifier::Shift)); + EXPECT_FALSE(NAS2D::EventHandler::numlock(NAS2D::EventHandler::KeyModifier::Ctrl)); + EXPECT_FALSE(NAS2D::EventHandler::numlock(NAS2D::EventHandler::KeyModifier::Alt)); +} + +TEST(EventHandler, control) { + EXPECT_TRUE(NAS2D::EventHandler::control(NAS2D::EventHandler::KeyModifier::Ctrl)); + EXPECT_TRUE(NAS2D::EventHandler::control(NAS2D::EventHandler::KeyModifier::Ctrl | NAS2D::EventHandler::KeyModifier::CtrlLeft)); + EXPECT_FALSE(NAS2D::EventHandler::control(NAS2D::EventHandler::KeyModifier::Shift)); + EXPECT_FALSE(NAS2D::EventHandler::control(NAS2D::EventHandler::KeyModifier::Alt)); +} + +TEST(EventHandler, alt) { + EXPECT_TRUE(NAS2D::EventHandler::alt(NAS2D::EventHandler::KeyModifier::Alt)); + EXPECT_TRUE(NAS2D::EventHandler::alt(NAS2D::EventHandler::KeyModifier::Alt | NAS2D::EventHandler::KeyModifier::CtrlLeft)); + EXPECT_FALSE(NAS2D::EventHandler::alt(NAS2D::EventHandler::KeyModifier::Shift)); + EXPECT_FALSE(NAS2D::EventHandler::alt(NAS2D::EventHandler::KeyModifier::Ctrl)); +} From 43bb9d412868283ed8a8851435527158b191f0f9 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Sun, 28 Jan 2024 03:58:57 -0700 Subject: [PATCH 3/6] Modify caps lock behaviour of `shift` method We really shouldn't be merging these two checks. Shift behaviour is independent of Caps Locks for most control key combinations. For typing and capitalization, it should be XOR of Shift and Caps, not the OR of them. --- NAS2D/EventHandler.cpp | 2 +- test/EventHandler.test.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/NAS2D/EventHandler.cpp b/NAS2D/EventHandler.cpp index bd4a629ef..1b8ab0e36 100644 --- a/NAS2D/EventHandler.cpp +++ b/NAS2D/EventHandler.cpp @@ -750,7 +750,7 @@ bool EventHandler::textInputMode() */ bool EventHandler::shift(KeyModifier mod) { - return KeyModifier::None != (mod & (KeyModifier::Shift | KeyModifier::Caps)); + return KeyModifier::None != (mod & (KeyModifier::Shift)); } diff --git a/test/EventHandler.test.cpp b/test/EventHandler.test.cpp index 1148fe862..6fb0e4a57 100644 --- a/test/EventHandler.test.cpp +++ b/test/EventHandler.test.cpp @@ -8,7 +8,7 @@ TEST(EventHandler, shift) { EXPECT_TRUE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::ShiftLeft)); EXPECT_TRUE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::ShiftRight)); EXPECT_TRUE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::ShiftLeft | NAS2D::EventHandler::KeyModifier::CtrlLeft)); - EXPECT_TRUE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::Caps)); + EXPECT_FALSE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::Caps)); EXPECT_FALSE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::Ctrl)); EXPECT_FALSE(NAS2D::EventHandler::shift(NAS2D::EventHandler::KeyModifier::Alt)); } From fdb7db876eb8788b500e2ab5941afc02790ad1c3 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Sun, 28 Jan 2024 04:02:13 -0700 Subject: [PATCH 4/6] Simplify casting of `KeyModifier` query functions --- NAS2D/EventHandler.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/NAS2D/EventHandler.cpp b/NAS2D/EventHandler.cpp index 1b8ab0e36..ecce61c2d 100644 --- a/NAS2D/EventHandler.cpp +++ b/NAS2D/EventHandler.cpp @@ -792,8 +792,7 @@ bool EventHandler::control(KeyModifier mod) */ bool EventHandler::query_shift() const { - using underlying = std::underlying_type_t; - return KeyModifier::None != static_cast(SDL_GetModState() & static_cast(KeyModifier::Shift)); + return KeyModifier::None != (static_cast(SDL_GetModState()) & KeyModifier::Shift); } @@ -802,8 +801,7 @@ bool EventHandler::query_shift() const */ bool EventHandler::query_numlock() const { - using underlying = std::underlying_type_t; - return KeyModifier::None != static_cast(SDL_GetModState() & static_cast(KeyModifier::Num)); + return KeyModifier::None != (static_cast(SDL_GetModState()) & KeyModifier::Num); } @@ -812,8 +810,7 @@ bool EventHandler::query_numlock() const */ bool EventHandler::query_control() const { - using underlying = std::underlying_type_t; - return KeyModifier::None != static_cast(SDL_GetModState() & static_cast(KeyModifier::Ctrl)); + return KeyModifier::None != (static_cast(SDL_GetModState()) & KeyModifier::Ctrl); } From d6346aba60242dcb61ab797ff3dd19da9a319678 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Sun, 28 Jan 2024 04:04:15 -0700 Subject: [PATCH 5/6] Implement query methods in terms of `static` methods Reduce code duplication. Ensure functions with similar names behave in the same manner. Previously the `shift` and `query_shift` functions treated Caps Lock differently. --- NAS2D/EventHandler.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/NAS2D/EventHandler.cpp b/NAS2D/EventHandler.cpp index ecce61c2d..0db221881 100644 --- a/NAS2D/EventHandler.cpp +++ b/NAS2D/EventHandler.cpp @@ -792,7 +792,7 @@ bool EventHandler::control(KeyModifier mod) */ bool EventHandler::query_shift() const { - return KeyModifier::None != (static_cast(SDL_GetModState()) & KeyModifier::Shift); + return shift(static_cast(SDL_GetModState())); } @@ -801,7 +801,7 @@ bool EventHandler::query_shift() const */ bool EventHandler::query_numlock() const { - return KeyModifier::None != (static_cast(SDL_GetModState()) & KeyModifier::Num); + return numlock(static_cast(SDL_GetModState())); } @@ -810,7 +810,7 @@ bool EventHandler::query_numlock() const */ bool EventHandler::query_control() const { - return KeyModifier::None != (static_cast(SDL_GetModState()) & KeyModifier::Ctrl); + return control(static_cast(SDL_GetModState())); } From dbfe4946a644de0921250837a5b95d90c597f6d9 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Sun, 28 Jan 2024 04:06:58 -0700 Subject: [PATCH 6/6] Strip `query_` prefix from method names There is sufficient context for function overloading to distinguish which method should be called. The main difference is if a `KeyModifier` is already known, or needs to be queried from the current keyboard state. When no `KeyModifier` is given, a reasonable assumption is to query for the information. Removing the `_` makes the name more consistent with other methods, which prefer camelCase. --- NAS2D/EventHandler.cpp | 6 +++--- NAS2D/EventHandler.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/NAS2D/EventHandler.cpp b/NAS2D/EventHandler.cpp index 0db221881..6149b3b59 100644 --- a/NAS2D/EventHandler.cpp +++ b/NAS2D/EventHandler.cpp @@ -790,7 +790,7 @@ bool EventHandler::control(KeyModifier mod) /** * Queries state of the Shift key modifier. */ -bool EventHandler::query_shift() const +bool EventHandler::shift() const { return shift(static_cast(SDL_GetModState())); } @@ -799,7 +799,7 @@ bool EventHandler::query_shift() const /** * Queries state of the Shift key modifier. */ -bool EventHandler::query_numlock() const +bool EventHandler::numlock() const { return numlock(static_cast(SDL_GetModState())); } @@ -808,7 +808,7 @@ bool EventHandler::query_numlock() const /** * Queries state of the Shift key modifier. */ -bool EventHandler::query_control() const +bool EventHandler::control() const { return control(static_cast(SDL_GetModState())); } diff --git a/NAS2D/EventHandler.h b/NAS2D/EventHandler.h index ddcf3075a..f9dd93262 100644 --- a/NAS2D/EventHandler.h +++ b/NAS2D/EventHandler.h @@ -322,9 +322,9 @@ namespace NAS2D static bool control(KeyModifier mod); static bool alt(KeyModifier mod); - bool query_shift() const; - bool query_numlock() const; - bool query_control() const; + bool shift() const; + bool numlock() const; + bool control() const; void pump();