Skip to content
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.

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

[PATCH] Fix sticky mouse state after special launched #590

Open
cm8 opened this issue Dec 10, 2023 · 1 comment
Open

[PATCH] Fix sticky mouse state after special launched #590

cm8 opened this issue Dec 10, 2023 · 1 comment
Projects

Comments

@cm8
Copy link

cm8 commented Dec 10, 2023

void cMouseDeployState::onNotifyGameEvent(const s_GameEvent &) {
m_context->toPreviousState();
}

After a special launched (e.g. combo Deathhand / Harkonnen), the mouse may not return from MOUSESTATE_DEPLOY, but rather sticks to it. This is observable in the game by the crosshair cursor colored red.

Since the following logic snippet prevents identity of m_prevState and m_state it presumably should not occur, because m_prevState == MOUSESTATE_DEPLOY when onNotifyGameEvent handler is called, but rather because it is called twice or more times. Not having debugged this directly, this seems to be the only explanation for the game's observable, unexpected behavior.

void cGameControlsContext::setMouseState(eMouseState newState) {
if (newState != m_state) {
// Remember previous state as long as it is not the PLACE state, since we can't go back to that state
if (m_state != eMouseState::MOUSESTATE_PLACE) {
m_prevState = m_state;

If onNotifyGameEvent is handled twice, or an even number of times, the current code toggles first to m_prevState, but consequently setting m_prevState == MOUSESTATE_DEPLOY doing this. On the second call it reverts/swaps these states again, such that the player is unable to further control game's actions (instead of waiting for another special being ready..).

There may be a better or more general solution to this, so instead of a PR, please review and test wip-d2tm-fix-sticky-deploy-mousestate.patch.zip

diff --git a/controls/cGameControlsContext.cpp b/controls/cGameControlsContext.cpp
index 92f5c301..0d91c95c 100644
--- a/controls/cGameControlsContext.cpp
+++ b/controls/cGameControlsContext.cpp
@@ -258,6 +258,10 @@ void cGameControlsContext::toPreviousState() {
     setMouseState(m_prevState);
 }
 
+bool cGameControlsContext::isPreviousState(eMouseState other) const {
+    return this->m_prevState == other;
+}
+
 bool cGameControlsContext::isState(eMouseState other) const {
     return this->m_state == other;
 }
diff --git a/controls/cGameControlsContext.h b/controls/cGameControlsContext.h
index 9f924a58..5a9ca81b 100644
--- a/controls/cGameControlsContext.h
+++ b/controls/cGameControlsContext.h
@@ -55,6 +55,7 @@ class cGameControlsContext : public cInputObserver, cScenarioObserver {
 
         void toPreviousState();
 
+        bool isPreviousState(eMouseState other) const;
         bool isState(eMouseState other) const;
 
         void onFocusMouseStateEvent();
diff --git a/controls/mousestates/cMouseDeployState.cpp b/controls/mousestates/cMouseDeployState.cpp
index 0ef6da09..edaf558a 100644
--- a/controls/mousestates/cMouseDeployState.cpp
+++ b/controls/mousestates/cMouseDeployState.cpp
@@ -91,7 +91,11 @@ void cMouseDeployState::onFocus() {
 }
 
 void cMouseDeployState::onNotifyGameEvent(const s_GameEvent &) {
-    m_context->toPreviousState();
+    if (m_context->isPreviousState(MOUSESTATE_DEPLOY)) {
+        m_context->setMouseState(MOUSESTATE_SELECT);
+    } else {
+        m_context->toPreviousState();
+    }
 }
 
 void cMouseDeployState::onBlur() {

EDIT: a less invasive bugfix for the problem described is

diff --git a/controls/cGameControlsContext.cpp b/controls/cGameControlsContext.cpp
index 365fcfcf..49fdf4ea 100644
--- a/controls/cGameControlsContext.cpp
+++ b/controls/cGameControlsContext.cpp
@@ -229,8 +229,8 @@ void cGameControlsContext::onNotifyKeyboardEvent(const cKeyboardEvent &event) {
 
 void cGameControlsContext::setMouseState(eMouseState newState) {
     if (newState != m_state) {
-        // Remember previous state as long as it is not the PLACE state, since we can't go back to that state
-        if (m_state != eMouseState::MOUSESTATE_PLACE) {
+        // Remember previous state as long as it is not the PLACE or DEPLOY state
+        if (m_state != eMouseState::MOUSESTATE_PLACE && m_state != eMouseState::MOUSESTATE_DEPLOY) {
             m_prevState = m_state;
             if (newState == eMouseState::MOUSESTATE_REPAIR) {
                 m_prevStateBeforeRepair = m_state;
@cm8
Copy link
Author

cm8 commented Dec 10, 2023

If this bug exists to intentionally mimic behavior of the original 1990s game, and if people care to retain those bugs for nostalgic sake, eventually a command line option to d2tm executable makes sense as to activate 1990s conformance or not. But its more work to implement..

@stefanhendriks stefanhendriks added this to Backlog in D2TM via automation May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
D2TM
  
Backlog
Development

No branches or pull requests

1 participant