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

CRAB: Fix wrong keymap being set in the Dialog Box #5424

Merged
merged 1 commit into from Nov 14, 2023

Conversation

Brakkugit
Copy link
Contributor

Now pressing enter key loads the next statement of the character in the dialog box.

Now pressing enter key loads the next statement of the character in the
dialog box.
Copy link
Contributor

coderabbitai bot commented Nov 13, 2023

Walkthrough

Walkthrough

The update to the game's code modifies the event handling within the GameDialogBox. It introduces a mechanism to switch between key binding modes when interacting with the dialog box. When a button click event occurs, the system now checks the current key binding mode. If it's not set to the user interface mode (KBM_UI), it switches to it. After processing a left click, the mode reverts back to the game mode (KBM_GAME). This ensures that key bindings are appropriate for the current context.

Changes

File Summary
.../crab/ui/dialogbox.cpp Modified GameDialogBox::handleEvents to switch key binding modes during button click events.

Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Comment on lines 83 to 98
// Purpose: Handle input
//------------------------------------------------------------------------
bool GameDialogBox::handleEvents(const Common::Event &event) {
return (_button.handleEvents(event) == BUAC_LCLICK);
// Switch to KBM_UI
if (g_engine->_inputManager->getKeyBindingMode() != KBM_UI)
g_engine->_inputManager->setKeyBindingMode(KBM_UI);

bool isLeftClick = (_button.handleEvents(event) == BUAC_LCLICK);

if (isLeftClick) {
// Switch to KBM_GAME
g_engine->_inputManager->setKeyBindingMode(KBM_GAME);
}

return isLeftClick;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The changes correctly implement the logic to switch key binding modes based on the event type. However, it's important to ensure that the key binding mode is only changed when necessary to avoid unnecessary calls which might affect performance or cause unexpected behavior in other parts of the game. Additionally, the code should ensure that the key binding mode is always correctly restored, even if an exception or an early return occurs between the mode switch and the restoration. Consider wrapping the mode switch and restoration logic in a RAII (Resource Acquisition Is Initialization) style object that automatically restores the previous mode when it goes out of scope.

class KeyBindingModeGuard {
public:
    KeyBindingModeGuard(InputManager* inputManager, KeyBindingMode newMode)
    : _inputManager(inputManager), _oldMode(inputManager->getKeyBindingMode()) {
        _inputManager->setKeyBindingMode(newMode);
    }

    ~KeyBindingModeGuard() {
        _inputManager->setKeyBindingMode(_oldMode);
    }

private:
    InputManager* _inputManager;
    KeyBindingMode _oldMode;
};

// Usage in the GameDialogBox::handleEvents function
bool GameDialogBox::handleEvents(const Common::Event &event) {
    KeyBindingModeGuard modeGuard(g_engine->_inputManager, KBM_UI);
    bool isLeftClick = (_button.handleEvents(event) == BUAC_LCLICK);
    if (isLeftClick) {
        // The destructor of KeyBindingModeGuard will automatically switch back to the old mode
    }
    return isLeftClick;
}

This ensures that the key binding mode is always restored, even if an exception is thrown or if there are multiple return paths from the function.

@sev-
Copy link
Member

sev- commented Nov 14, 2023

Thanks!

@sev- sev- merged commit 5fec5a9 into scummvm:master Nov 14, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants