CRAB: Add Debug output for Keymaps #5416
Conversation
WalkthroughWalkthroughThe Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
engines/crab/console.cpp
Outdated
| bool Console::cmdWrite(int argc, const char **argv) { | ||
| using namespace Crab::pyrodactyl::input; | ||
|
|
||
| if (argc > 1) { | ||
| for (int i = 1; i < argc; i++) { | ||
| if (!scumm_stricmp(argv[i], "keymap")) { | ||
|
|
||
| switch (g_engine->_inputManager->getKeyBindingMode()) { | ||
| case KBM_NONE: | ||
| debugPrintf("KBM_NONE\n"); | ||
| break; | ||
| case KBM_GAME: | ||
| debugPrintf("KBM_GAME\n"); | ||
| break; | ||
| case KBM_UI: | ||
| debugPrintf("KBM_UI\n"); | ||
| break; | ||
| default: | ||
| debugPrintf("Unknown KBM\n"); | ||
| break; | ||
| } | ||
| } else | ||
| debugPrintf("Valid parameters are 'keymap'\n"); | ||
| return true; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The cmdWrite function is implemented correctly. It checks for the "keymap" argument and prints the current key binding mode. However, the return true; statement is inside the for loop, which means the function will return after processing the first argument, even if there are more arguments. This should be corrected.
- return true;Move the return true; statement outside the for loop.
+ return true;
engines/crab/console.cpp
Outdated
| bool Console::cmdWrite(int argc, const char **argv) { | ||
| using namespace Crab::pyrodactyl::input; | ||
|
|
||
| if (argc > 1) { | ||
| for (int i = 1; i < argc; i++) { | ||
| if (!scumm_stricmp(argv[i], "keymap")) { | ||
|
|
||
| switch (g_engine->_inputManager->getKeyBindingMode()) { | ||
| case KBM_NONE: | ||
| debugPrintf("KBM_NONE\n"); | ||
| break; | ||
| case KBM_GAME: | ||
| debugPrintf("KBM_GAME\n"); | ||
| break; | ||
| case KBM_UI: | ||
| debugPrintf("KBM_UI\n"); | ||
| break; | ||
| default: | ||
| debugPrintf("Unknown KBM\n"); | ||
| break; | ||
| } | ||
| } else | ||
| debugPrintf("Valid parameters are 'keymap'\n"); | ||
| } | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
The cmdWrite method is implemented correctly. It checks for the "keymap" argument and prints the current key binding mode. However, it might be more user-friendly to provide a more descriptive message when the key binding mode is printed. For example, instead of just printing "KBM_GAME", you could print "The current key binding mode is KBM_GAME".
- debugPrintf("KBM_GAME\n");
+ debugPrintf("The current key binding mode is KBM_GAME.\n");Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| bool Console::cmdWrite(int argc, const char **argv) { | |
| using namespace Crab::pyrodactyl::input; | |
| if (argc > 1) { | |
| for (int i = 1; i < argc; i++) { | |
| if (!scumm_stricmp(argv[i], "keymap")) { | |
| switch (g_engine->_inputManager->getKeyBindingMode()) { | |
| case KBM_NONE: | |
| debugPrintf("KBM_NONE\n"); | |
| break; | |
| case KBM_GAME: | |
| debugPrintf("KBM_GAME\n"); | |
| break; | |
| case KBM_UI: | |
| debugPrintf("KBM_UI\n"); | |
| break; | |
| default: | |
| debugPrintf("Unknown KBM\n"); | |
| break; | |
| } | |
| } else | |
| debugPrintf("Valid parameters are 'keymap'\n"); | |
| } | |
| } | |
| return true; | |
| } | |
| bool Console::cmdWrite(int argc, const char **argv) { | |
| using namespace Crab::pyrodactyl::input; | |
| if (argc > 1) { | |
| for (int i = 1; i < argc; i++) { | |
| if (!scumm_stricmp(argv[i], "keymap")) { | |
| switch (g_engine->_inputManager->getKeyBindingMode()) { | |
| case KBM_NONE: | |
| debugPrintf("KBM_NONE\n"); | |
| break; | |
| case KBM_GAME: | |
| debugPrintf("The current key binding mode is KBM_GAME.\n"); | |
| break; | |
| case KBM_UI: | |
| debugPrintf("KBM_UI\n"); | |
| break; | |
| default: | |
| debugPrintf("Unknown KBM\n"); | |
| break; | |
| } | |
| } else | |
| debugPrintf("Valid parameters are 'keymap'\n"); | |
| } | |
| } | |
| return true; | |
| } |
Brakkugit
force-pushed
the
enterissuebranch
branch
from
7e77288
to
7feb5a1
Compare
engines/crab/console.cpp
Outdated
| } | ||
|
|
||
| } | ||
| return 0; |
There was a problem hiding this comment.
The cmdWhat method should return true on successful execution, not 0. This is because the method's return type is bool.
- return 0;
+ return true;Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| return 0; | |
| return true; |
Brakkugit
force-pushed
the
enterissuebranch
branch
2 times, most recently
from
3d16fc6
to
a6912f6
Compare
Now keymap being used can known by typing "what keymap" in the debugger console.
Brakkugit
force-pushed
the
enterissuebranch
branch
from
a6912f6
to
aa4fdf3
Compare
| bool Console::cmdWhat(int argc, const char **argv) { | ||
| if (argc > 1) { | ||
| for (int i = 1; i < argc; i++) { | ||
| if (!scumm_stricmp(argv[i], "keymap")) { | ||
| switch (g_engine->_inputManager->getKeyBindingMode()) { | ||
| case KBM_NONE: | ||
| debugPrintf("KBM_NONE\n"); | ||
| break; | ||
|
|
||
| case KBM_GAME: | ||
| debugPrintf("KBM_GAME\n"); | ||
| break; | ||
|
|
||
| case KBM_UI: | ||
| debugPrintf("KBM_UI\n"); | ||
| break; | ||
|
|
||
| default: | ||
| debugPrintf("Unknown KBM\n"); | ||
| break; | ||
| } | ||
| } else | ||
| debugPrintf("Valid parameters are 'keymap'\n"); | ||
| } | ||
| } | ||
| return true; |
There was a problem hiding this comment.
The implementation of the cmdWhat function appears to be correct and follows the established pattern for command functions in the ScummVM debugger console. However, the loop over argv is unnecessary if the command only supports a single argument "keymap". This could be simplified to a single check against argv[1]. Additionally, the default case in the switch statement could be enhanced by printing out the actual value of the unknown key binding mode, which would aid in debugging if new key binding modes are added in the future.
bool Console::cmdWhat(int argc, const char **argv) {
if (argc == 2 && !scumm_stricmp(argv[1], "keymap")) {
switch (g_engine->_inputManager->getKeyBindingMode()) {
// ... existing cases ...
default:
debugPrintf("Unknown KBM: %d\n", g_engine->_inputManager->getKeyBindingMode());
break;
}
} else {
debugPrintf("Valid parameters are 'keymap'\n");
}
return true;
}---end hunk 1---
<!-- This is an auto-generated comment by CodeRabbit -->
|
LGTM, merging! Thanks for the good work. |
Now keymap being used can be known by typing "what keymap" in the debugger console