diff --git a/Core/ControlMapper.cpp b/Core/ControlMapper.cpp index b336568be7..9144d7c489 100644 --- a/Core/ControlMapper.cpp +++ b/Core/ControlMapper.cpp @@ -391,6 +391,93 @@ void ControlMapper::SwapMappingIfEnabled(uint32_t *vkey) { } } +// Works out which mappings are currently being overridden by a longer one. If you map something +// to L2+R2, you don't want whatever L2 and R2 are mapped to on their own to fire as well, so while +// a combo is fully held, the shorter mappings sharing an input with it are suppressed. +// mutex_ should be locked, and also KeyMap::LockMappings(). +void ControlMapper::UpdateComboSuppression() { + if (KeyMap::HasChanged(comboMappingsGeneration_)) { + KeyMap::GetAllComboMappingsNoLock(&comboMappings_); + } + + comboSuppressionChanged_.clear(); + if (comboMappings_.empty() && comboSuppression_.empty()) { + // By far the common case - nobody has mapped a combo, so there's nothing to suppress. + return; + } + + std::map prevSuppression = std::move(comboSuppression_); + comboSuppression_.clear(); + + for (const auto &combo : comboMappings_) { + // Is every input of the combo held down? Same conditions as the main loops below. + bool all = true; + double curTime = 0.0; + for (const auto &mapping : combo.mappings) { + auto iter = curInput_.find(mapping); + if (iter == curInput_.end()) { + all = false; + break; + } + // Stop reverse ordering from triggering. + if (g_Config.bStrictComboOrder && iter->second.timestamp < curTime) { + all = false; + break; + } + curTime = iter->second.timestamp; + if (iter->second.value <= 0.0f || iter->second.value <= GetDeviceAxisThreshold(iter->first.deviceId, mapping)) { + all = false; + break; + } + } + if (!all) { + continue; + } + // It is, so record it as the one to beat for each of its inputs. + for (const auto &mapping : combo.mappings) { + size_t &longest = comboSuppression_[mapping]; + longest = std::max(longest, combo.mappings.size()); + } + } + + // Outputs are only re-evaluated when an input they use has changed, so when suppression + // starts or stops for an input, we have to treat that input as changed too. Otherwise + // releasing one button of a held combo wouldn't bring back what the others map to alone. + for (const auto &[mapping, size] : comboSuppression_) { + auto iter = prevSuppression.find(mapping); + if (iter == prevSuppression.end() || iter->second != size) { + comboSuppressionChanged_.push_back(mapping); + } + } + for (const auto &[mapping, size] : prevSuppression) { + if (!comboSuppression_.count(mapping)) { + comboSuppressionChanged_.push_back(mapping); + } + } +} + +bool ControlMapper::SuppressionChanged(const KeyMap::MultiInputMapping &multiMapping) const { + for (const auto &changed : comboSuppressionChanged_) { + if (multiMapping.mappings.contains(changed)) { + return true; + } + } + return false; +} + +bool ControlMapper::IsSuppressedByCombo(const KeyMap::MultiInputMapping &multiMapping) const { + if (comboSuppression_.empty()) { + return false; + } + for (const auto &mapping : multiMapping.mappings) { + auto iter = comboSuppression_.find(mapping); + if (iter != comboSuppression_.end() && multiMapping.mappings.size() < iter->second) { + return true; + } + } + return false; +} + // Can only be called from Key or Axis. // mutex_ should be locked, and also KeyMap::LockMappings(). // TODO: We should probably make a batched version of this. @@ -406,6 +493,8 @@ bool ControlMapper::UpdatePSPState(const InputMapping &changedMapping, double no case ROTATION_LOCKED_VERTICAL180: rotations = 3; break; } + UpdateComboSuppression(); + // For the PSP's digital button inputs, we just go through and put the flags together. uint32_t buttonMask = 0; uint32_t changedButtonMask = 0; @@ -429,7 +518,7 @@ bool ControlMapper::UpdatePSPState(const InputMapping &changedMapping, double no // If a mapping could consist of a combo, we could trivially check it here. for (auto &multiMapping : inputMappings) { // Check if the changed mapping was involved in this PSP key. - if (multiMapping.mappings.contains(changedMapping)) { + if (multiMapping.mappings.contains(changedMapping) || SuppressionChanged(multiMapping)) { changedButtonMask |= mask; } // Check if all inputs are "on". @@ -452,7 +541,7 @@ bool ControlMapper::UpdatePSPState(const InputMapping &changedMapping, double no if (!down) all = false; } - if (all) { + if (all && !IsSuppressedByCombo(multiMapping)) { buttonMask |= mask; } } @@ -484,10 +573,14 @@ bool ControlMapper::UpdatePSPState(const InputMapping &changedMapping, double no bool touchedByMapping = false; float value = 0.0f; for (auto &multiMapping : inputMappings) { - if (multiMapping.mappings.contains(changedMapping)) { + if (multiMapping.mappings.contains(changedMapping) || SuppressionChanged(multiMapping)) { touchedByMapping = true; } + if (IsSuppressedByCombo(multiMapping)) { + continue; + } + float product = 1.0f; // We multiply the various inputs in a combo mapping with each other. double curTime = 0.0; for (auto mapping : multiMapping.mappings) { diff --git a/Core/ControlMapper.h b/Core/ControlMapper.h index d28f526b45..db6ebfe274 100644 --- a/Core/ControlMapper.h +++ b/Core/ControlMapper.h @@ -73,6 +73,9 @@ public: private: void UpdateSwapAxes(); bool UpdatePSPState(const InputMapping &changedMapping, double now); + void UpdateComboSuppression(); + bool IsSuppressedByCombo(const KeyMap::MultiInputMapping &multiMapping) const; + bool SuppressionChanged(const KeyMap::MultiInputMapping &multiMapping) const; float MapAxisValue(float value, int vkId, const InputMapping &mapping, const InputMapping &changedMapping, bool *oppositeTouched); void SwapMappingIfEnabled(uint32_t *vkey); @@ -124,6 +127,17 @@ private: std::map curInput_; + // While a combo mapping is fully held, the shorter mappings that its inputs also belong to are + // suppressed - see UpdateComboSuppression. Maps an input to the size of the longest satisfied + // combo it takes part in, so a mapping is suppressed if it's shorter than that. + std::map comboSuppression_; + // Every combo mapping in the keymap. Cached, since scanning them all isn't free and the + // mappings only change when the user edits them. + std::vector comboMappings_; + int comboMappingsGeneration_ = -1; + // Inputs whose suppression state changed in the current update, see UpdateComboSuppression. + std::vector comboSuppressionChanged_; + // Callbacks std::vector listeners_; }; diff --git a/Core/KeyMap.cpp b/Core/KeyMap.cpp index b9b060a3bd..4522f807ac 100644 --- a/Core/KeyMap.cpp +++ b/Core/KeyMap.cpp @@ -614,6 +614,17 @@ bool InputMappingToPspButton(const InputMapping &mapping, std::vector *pspB return found; } +void GetAllComboMappingsNoLock(std::vector *combos) { + combos->clear(); + for (const auto &iter : g_controllerMap) { + for (const auto &mapping : iter.second) { + if (mapping.mappings.size() > 1) { + combos->push_back(mapping); + } + } + } +} + // This is the main workhorse of the ControlMapper. bool InputMappingsFromPspButtonNoLock(int btn, std::vector *mappings, bool ignoreMouse) { auto iter = g_controllerMap.find(btn); diff --git a/Core/KeyMap.h b/Core/KeyMap.h index 7ba4c83e78..eac575f24a 100644 --- a/Core/KeyMap.h +++ b/Core/KeyMap.h @@ -186,6 +186,8 @@ namespace KeyMap { // Careful with these. bool InputMappingsFromPspButtonNoLock(int btn, std::vector *keys, bool ignoreMouse); + // Collects every mapping that consists of more than one input, ie. a combo. + void GetAllComboMappingsNoLock(std::vector *combos); void LockMappings(); void UnlockMappings(); diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 9b9913cdb7..6190e54233 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -106,6 +106,8 @@ #include "Core/FileSystems/ISOFileSystem.h" #include "Core/MemMap.h" #include "Core/KeyMap.h" +#include "Core/ControlMapper.h" +#include "Core/HLE/sceCtrl.h" #include "Core/Util/PathUtil.h" #include "Core/MIPS/MIPSVFPUUtils.h" #include "GPU/Common/TextureDecoder.h" @@ -2446,6 +2448,77 @@ bool TestInputMapping() { return true; } +// Records what the ControlMapper tells us, so a test can check it. +class TestControlListener : public ControlListener { +public: + void OnVKey(VirtKey vkey, bool down) override { + vkeyDown[vkey] = down; + } + void UpdatePSPButtons(uint32_t buttonMask, uint32_t changedMask) override { + buttons = (buttons & ~changedMask) | buttonMask; + } + uint32_t buttons = 0; + std::map vkeyDown; +}; + +static bool SendKey(ControlMapper *mapper, int keyCode, bool down) { + KeyInput key{}; + key.deviceId = DEVICE_ID_PAD_0; + key.keyCode = (InputKeyCode)keyCode; + key.flags = down ? KeyInputFlags::DOWN : KeyInputFlags::UP; + return mapper->Key(key); +} + +// A mapping shouldn't fire when a longer mapping sharing an input with it is held. See #20621. +bool TestComboSuppression() { + using KeyMap::MultiInputMapping; + + InputMapping a(DEVICE_ID_PAD_0, NKCODE_BUTTON_1); + InputMapping b(DEVICE_ID_PAD_0, NKCODE_BUTTON_2); + + KeyMap::ClearAllMappings(); + KeyMap::SetInputMapping(CTRL_CIRCLE, MultiInputMapping(a), true); + KeyMap::SetInputMapping(CTRL_SQUARE, MultiInputMapping(b), true); + MultiInputMapping combo(a); + combo.mappings.push_back(b); + KeyMap::SetInputMapping(VIRTKEY_PAUSE, combo, true); + + TestControlListener listener; + ControlMapper mapper; + mapper.AddListener(&listener); + + // A on its own presses Circle. + SendKey(&mapper, NKCODE_BUTTON_1, true); + EXPECT_EQ_INT((int)(listener.buttons & CTRL_CIRCLE), (int)CTRL_CIRCLE); + EXPECT_FALSE(listener.vkeyDown[VIRTKEY_PAUSE]); + + // Adding B completes the combo, so Circle lets go and Square never presses. + SendKey(&mapper, NKCODE_BUTTON_2, true); + EXPECT_TRUE(listener.vkeyDown[VIRTKEY_PAUSE]); + EXPECT_EQ_INT((int)(listener.buttons & CTRL_CIRCLE), 0); + EXPECT_EQ_INT((int)(listener.buttons & CTRL_SQUARE), 0); + + // Letting go of B ends the combo, and since A is still held, Circle comes back. + SendKey(&mapper, NKCODE_BUTTON_2, false); + EXPECT_FALSE(listener.vkeyDown[VIRTKEY_PAUSE]); + EXPECT_EQ_INT((int)(listener.buttons & CTRL_CIRCLE), (int)CTRL_CIRCLE); + EXPECT_EQ_INT((int)(listener.buttons & CTRL_SQUARE), 0); + + // And releasing A leaves nothing pressed. + SendKey(&mapper, NKCODE_BUTTON_1, false); + EXPECT_EQ_INT((int)(listener.buttons & (CTRL_CIRCLE | CTRL_SQUARE)), 0); + + // B on its own still presses Square - suppression only applies while the combo is held. + SendKey(&mapper, NKCODE_BUTTON_2, true); + EXPECT_EQ_INT((int)(listener.buttons & CTRL_SQUARE), (int)CTRL_SQUARE); + EXPECT_FALSE(listener.vkeyDown[VIRTKEY_PAUSE]); + SendKey(&mapper, NKCODE_BUTTON_2, false); + + mapper.RemoveListener(&listener); + KeyMap::ClearAllMappings(); + return true; +} + bool TestEscapeMenuString() { char c; std::string temp = UnescapeMenuString("&File", &c); @@ -3010,6 +3083,7 @@ TestItem availableTests[] = { TEST_ITEM(FastVec), TEST_ITEM(SmallDataConvert), TEST_ITEM(InputMapping), + TEST_ITEM(ComboSuppression), TEST_ITEM(EscapeMenuString), TEST_ITEM(VFS), TEST_ITEM(Substitutions),