From 8299e4db22ea145a059727ae954d632a3ad35c09 Mon Sep 17 00:00:00 2001 From: Malin E Date: Tue, 30 Nov 2021 10:46:44 +0100 Subject: [PATCH] Address PR comments --- .../interaction/joystickcamerastates.h | 26 ++++--- .../openspace/navigation/navigationhandler.h | 6 +- src/interaction/joystickcamerastates.cpp | 73 +++++++++---------- src/interaction/joystickinputstate.cpp | 6 +- src/navigation/navigationhandler.cpp | 12 +-- src/navigation/navigationhandler_lua.inl | 6 +- 6 files changed, 67 insertions(+), 62 deletions(-) diff --git a/include/openspace/interaction/joystickcamerastates.h b/include/openspace/interaction/joystickcamerastates.h index d0b2d74039..2c96b58a5c 100644 --- a/include/openspace/interaction/joystickcamerastates.h +++ b/include/openspace/interaction/joystickcamerastates.h @@ -54,7 +54,7 @@ public: }; enum class JoystickType { - JoystickLike = 0, + JoystickLike, TriggerLike }; @@ -88,14 +88,14 @@ public: void updateStateFromInput( const JoystickInputStates& joystickInputStates, double deltaTime); - void setAxisMapping(const std::string& joystickName, int axis, AxisType mapping, + void setAxisMapping(std::string joystickName, int axis, AxisType mapping, AxisInvert shouldInvert = AxisInvert::No, JoystickType joystickType = JoystickType::JoystickLike, bool isSticky = false, double sensitivity = 0.0 ); - void setAxisMappingProperty(const std::string& joystickName, int axis, - const std::string& propertyUri, float min = 0.f, float max = 1.f, + void setAxisMappingProperty(std::string joystickName, int axis, + std::string propertyUri, float min = 0.f, float max = 1.f, AxisInvert shouldInvert = AxisInvert::No, bool isRemote = true ); @@ -119,7 +119,6 @@ private: // be accessed much more often and thus have to be more efficient. And storing a few // extra AxisInformation that are not used will not matter that much; finding an axis // location in a potential map each frame, however, would - std::array axisMapping; // This array is used to store the old axis values from the previous frame, @@ -127,9 +126,16 @@ private: std::array prevAxisValues; struct ButtonInformation { + // The script that is run when the button is activated std::string command; + + // When is the button considered activated JoystickAction action; + + // If the script should be syncronised to other remote sessions or not ButtonCommandRemote synchronization; + + // Short documentation on what the script of this button does std::string documentation; }; @@ -140,8 +146,8 @@ private: // Find the item in _joystickCameraStates that corresponds to the given joystickName // return a pointer to the item, if not found then return nullptr - JoystickCameraState* getJoystickCameraState(const std::string& joystickName); - const JoystickCameraState* getJoystickCameraState(const std::string& joystickName) const; + JoystickCameraState* joystickCameraState(const std::string& joystickName); + const JoystickCameraState* joystickCameraState(const std::string& joystickName) const; // Ues getJoystickCameraState(name) to find the joystickCameraState that // corresponds to the given joystickName. If not found then add a new item if possible @@ -204,9 +210,9 @@ inline std::string to_string( { using T = openspace::interaction::JoystickCameraStates::JoystickType; switch (value) { - case T::JoystickLike: return "JoystickLike"; - case T::TriggerLike: return "TriggerLike"; - default: return ""; + case T::JoystickLike: return "JoystickLike"; + case T::TriggerLike: return "TriggerLike"; + default: return ""; } } diff --git a/include/openspace/navigation/navigationhandler.h b/include/openspace/navigation/navigationhandler.h index 3a6bbeac36..f66e765ac5 100644 --- a/include/openspace/navigation/navigationhandler.h +++ b/include/openspace/navigation/navigationhandler.h @@ -98,7 +98,7 @@ public: void mousePositionCallback(double x, double y); void mouseScrollWheelCallback(double pos); - void setJoystickAxisMapping(const std::string& joystickName, + void setJoystickAxisMapping(std::string joystickName, int axis, JoystickCameraStates::AxisType mapping, JoystickCameraStates::AxisInvert shouldInvert = JoystickCameraStates::AxisInvert::No, @@ -107,8 +107,8 @@ public: bool isSticky = false, double sensitivity = 0.0 ); - void setJoystickAxisMappingProperty(const std::string& joystickName, - int axis, const std::string& propertyUri, + void setJoystickAxisMappingProperty(std::string joystickName, + int axis, std::string propertyUri, float min = 0.f, float max = 1.f, JoystickCameraStates::AxisInvert shouldInvert = JoystickCameraStates::AxisInvert::No, bool isRemote = true diff --git a/src/interaction/joystickcamerastates.cpp b/src/interaction/joystickcamerastates.cpp index 9d4b5ea4df..22a9c6f53e 100644 --- a/src/interaction/joystickcamerastates.cpp +++ b/src/interaction/joystickcamerastates.cpp @@ -56,15 +56,14 @@ void JoystickCameraStates::updateStateFromInput( continue; } - JoystickCameraState* joystickCameraState = - getJoystickCameraState(joystickInputState.name); + JoystickCameraState* joystick = joystickCameraState(joystickInputState.name); - if (!joystickCameraState) { + if (!joystick) { continue; } for (int i = 0; i < JoystickInputState::MaxAxes; ++i) { - AxisInformation t = joystickCameraState->axisMapping[i]; + AxisInformation t = joystick->axisMapping[i]; if (t.type == AxisType::None) { continue; } @@ -73,13 +72,13 @@ void JoystickCameraStates::updateStateFromInput( float value = rawValue; if (t.isSticky) { - value = rawValue - joystickCameraState->prevAxisValues[i]; - joystickCameraState->prevAxisValues[i] = rawValue; + value = rawValue - joystick->prevAxisValues[i]; + joystick->prevAxisValues[i] = rawValue; } if ((t.joystickType == JoystickType::JoystickLike && - std::fabs(value) <= t.deadzone) || - (t.joystickType == JoystickType::TriggerLike && value <= -1 + t.deadzone)) + std::abs(value) <= t.deadzone) || + (t.joystickType == JoystickType::TriggerLike && value <= -1.f + t.deadzone)) { continue; } @@ -151,8 +150,8 @@ void JoystickCameraStates::updateStateFromInput( localRotation.second.y += value; break; case AxisType::Property: - std::string script = "openspace.setPropertyValue(\"" + - t.propertyUri + "\", " + std::to_string(value) + ");"; + std::string script = fmt::format("openspace.setPropertyValue('{}', {});", + t.propertyUri, value); global::scriptEngine->queueScript( script, @@ -163,7 +162,7 @@ void JoystickCameraStates::updateStateFromInput( } for (int i = 0; i < JoystickInputState::MaxButtons; ++i) { - auto itRange = joystickCameraState->buttonMapping.equal_range(i); + auto itRange = joystick->buttonMapping.equal_range(i); for (auto it = itRange.first; it != itRange.second; ++it) { bool active = global::joystickInputStates->button( joystickInputState.name, @@ -217,7 +216,7 @@ void JoystickCameraStates::updateStateFromInput( } } -void JoystickCameraStates::setAxisMapping(const std::string& joystickName, +void JoystickCameraStates::setAxisMapping(std::string joystickName, int axis, AxisType mapping, AxisInvert shouldInvert, JoystickType joystickType, @@ -241,9 +240,9 @@ void JoystickCameraStates::setAxisMapping(const std::string& joystickName, global::joystickInputStates->axis(joystickName, axis); } -void JoystickCameraStates::setAxisMappingProperty(const std::string& joystickName, +void JoystickCameraStates::setAxisMappingProperty(std::string joystickName, int axis, - const std::string& propertyUri, + std::string propertyUri, float min, float max, AxisInvert shouldInvert, bool isRemote) @@ -270,13 +269,13 @@ JoystickCameraStates::AxisInformation JoystickCameraStates::axisMapping( const std::string& joystickName, int axis) const { - const JoystickCameraState* joystickCameraState = getJoystickCameraState(joystickName); - if (!joystickCameraState) { + const JoystickCameraState* joystick = joystickCameraState(joystickName); + if (!joystick) { JoystickCameraStates::AxisInformation dummy; return dummy; } - return joystickCameraState->axisMapping[axis]; + return joystick->axisMapping[axis]; } void JoystickCameraStates::setDeadzone(const std::string& joystickName, int axis, @@ -291,12 +290,12 @@ void JoystickCameraStates::setDeadzone(const std::string& joystickName, int axis } float JoystickCameraStates::deadzone(const std::string& joystickName, int axis) const { - const JoystickCameraState* joystickCameraState = getJoystickCameraState(joystickName); - if (!joystickCameraState) { - return 0.0f; + const JoystickCameraState* joystick = joystickCameraState(joystickName); + if (!joystick) { + return 0.f; } - return joystickCameraState->axisMapping[axis].deadzone; + return joystick->axisMapping[axis].deadzone; } void JoystickCameraStates::bindButtonCommand(const std::string& joystickName, @@ -319,18 +318,18 @@ void JoystickCameraStates::bindButtonCommand(const std::string& joystickName, void JoystickCameraStates::clearButtonCommand(const std::string& joystickName, int button) { - JoystickCameraState* joystickCameraState = getJoystickCameraState(joystickName); - if (!joystickCameraState) { + JoystickCameraState* joystick = joystickCameraState(joystickName); + if (!joystick) { return; } - for (auto it = joystickCameraState->buttonMapping.begin(); - it != joystickCameraState->buttonMapping.end(); ) + for (auto it = joystick->buttonMapping.begin(); + it != joystick->buttonMapping.end(); ) { // If the current iterator is the button that we are looking for, delete it // (std::multimap::erase will return the iterator to the next element for us) if (it->first == button) { - it = joystickCameraState->buttonMapping.erase(it); + it = joystick->buttonMapping.erase(it); } else { ++it; @@ -343,19 +342,19 @@ std::vector JoystickCameraStates::buttonCommand( int button) const { std::vector result; - const JoystickCameraState* joystickCameraState = getJoystickCameraState(joystickName); - if (!joystickCameraState) { + const JoystickCameraState* joystick = joystickCameraState(joystickName); + if (!joystick) { return result; } - auto itRange = joystickCameraState->buttonMapping.equal_range(button); + auto itRange = joystick->buttonMapping.equal_range(button); for (auto it = itRange.first; it != itRange.second; ++it) { result.push_back(it->second.command); } return result; } -JoystickCameraStates::JoystickCameraState* JoystickCameraStates::getJoystickCameraState( +JoystickCameraStates::JoystickCameraState* JoystickCameraStates::joystickCameraState( const std::string& joystickName) { for (JoystickCameraState& joystickCameraState : _joystickCameraStates) { @@ -368,7 +367,7 @@ JoystickCameraStates::JoystickCameraState* JoystickCameraStates::getJoystickCame } const JoystickCameraStates::JoystickCameraState* - JoystickCameraStates::getJoystickCameraState(const std::string& joystickName) const +JoystickCameraStates::joystickCameraState(const std::string& joystickName) const { for (const JoystickCameraState& joystickCameraState : _joystickCameraStates) { if (joystickCameraState.joystickName == joystickName) { @@ -381,14 +380,14 @@ const JoystickCameraStates::JoystickCameraState* } JoystickCameraStates::JoystickCameraState* - JoystickCameraStates::findOrAddJoystickCameraState(const std::string& joystickName) +JoystickCameraStates::findOrAddJoystickCameraState(const std::string& joystickName) { - JoystickCameraState* joystickCameraState = getJoystickCameraState(joystickName); - if (!joystickCameraState) { + JoystickCameraState* joystick = joystickCameraState(joystickName); + if (!joystick) { if (_joystickCameraStates.size() < JoystickInputStates::MaxNumJoysticks) { _joystickCameraStates.push_back(JoystickCameraState()); - joystickCameraState = &_joystickCameraStates.back(); - joystickCameraState->joystickName = joystickName; + joystick = &_joystickCameraStates.back(); + joystick->joystickName = joystickName; } else { LWARNING(fmt::format("Cannot add more joysticks, only {} joysticks are " @@ -396,7 +395,7 @@ JoystickCameraStates::JoystickCameraState* return nullptr; } } - return joystickCameraState; + return joystick; } diff --git a/src/interaction/joystickinputstate.cpp b/src/interaction/joystickinputstate.cpp index 9c0da27225..170a40e296 100644 --- a/src/interaction/joystickinputstate.cpp +++ b/src/interaction/joystickinputstate.cpp @@ -36,7 +36,7 @@ namespace openspace::interaction { float JoystickInputStates::axis(const std::string& joystickName, int axis) const { ghoul_precondition(axis >= 0, "axis must be 0 or positive"); - if(joystickName.empty()) { + if (joystickName.empty()) { float res = std::accumulate( begin(), end(), @@ -63,7 +63,7 @@ float JoystickInputStates::axis(const std::string& joystickName, int axis) const } if (!state) { - return 0.0f; + return 0.f; } return state->axes[axis]; @@ -72,7 +72,7 @@ float JoystickInputStates::axis(const std::string& joystickName, int axis) const bool JoystickInputStates::button(const std::string& joystickName, int button, JoystickAction action) const { ghoul_precondition(button >= 0, "button must be 0 or positive"); - if(joystickName.empty()) { + if (joystickName.empty()) { bool res = std::any_of( begin(), end(), diff --git a/src/navigation/navigationhandler.cpp b/src/navigation/navigationhandler.cpp index 439bcd503e..7f5ae8854e 100644 --- a/src/navigation/navigationhandler.cpp +++ b/src/navigation/navigationhandler.cpp @@ -498,7 +498,7 @@ void NavigationHandler::loadNavigationState(const std::string& filepath) { } } -void NavigationHandler::setJoystickAxisMapping(const std::string& joystickName, int axis, +void NavigationHandler::setJoystickAxisMapping(std::string joystickName, int axis, JoystickCameraStates::AxisType mapping, JoystickCameraStates::AxisInvert shouldInvert, JoystickCameraStates::JoystickType joystickType, @@ -506,7 +506,7 @@ void NavigationHandler::setJoystickAxisMapping(const std::string& joystickName, double sensitivity) { _orbitalNavigator.joystickStates().setAxisMapping( - joystickName, + std::move(joystickName), axis, mapping, shouldInvert, @@ -516,17 +516,17 @@ void NavigationHandler::setJoystickAxisMapping(const std::string& joystickName, ); } -void NavigationHandler::setJoystickAxisMappingProperty(const std::string& joystickName, +void NavigationHandler::setJoystickAxisMappingProperty(std::string joystickName, int axis, - const std::string& propertyUri, + std::string propertyUri, float min, float max, JoystickCameraStates::AxisInvert shouldInvert, bool isRemote) { _orbitalNavigator.joystickStates().setAxisMappingProperty( - joystickName, + std::move(joystickName), axis, - propertyUri, + std::move(propertyUri), min, max, shouldInvert, diff --git a/src/navigation/navigationhandler_lua.inl b/src/navigation/navigationhandler_lua.inl index 9be6957ce2..0b7faca565 100644 --- a/src/navigation/navigationhandler_lua.inl +++ b/src/navigation/navigationhandler_lua.inl @@ -164,7 +164,7 @@ int bindJoystickAxis(lua_State* L) { joystickType = joystickType.value_or("JoystickLike"); global::navigationHandler->setJoystickAxisMapping( - joystickName, + std::move(joystickName), axis, ghoul::from_string(axisType), interaction::JoystickCameraStates::AxisInvert(*shouldInvert), @@ -188,9 +188,9 @@ int bindJoystickAxisProperty(lua_State* L) { isRemote = isRemote.value_or(true); global::navigationHandler->setJoystickAxisMappingProperty( - joystickName, + std::move(joystickName), axis, - propertyUri, + std::move(propertyUri), *min, *max, interaction::JoystickCameraStates::AxisInvert(*shouldInvert),