Conversation
c36c655 to
eb1863e
Compare
|
Cool work. Considering the size of this I would like more eyes on it, but I think it's pretty good overall. |
8fd6df5 to
fb9fad8
Compare
sev-
left a comment
There was a problem hiding this comment.
Added my notes, mostly related to possible action variety reduction
engines/sherlock/metaengine.cpp
Outdated
| act->addDefaultInputMapping("JOY_UP"); | ||
| scalpelFilesKeymap->addAction(act); | ||
|
|
||
| act = new Action("DOWN", _("Scroll down")); |
There was a problem hiding this comment.
I am really wondering. Does it make sense to repeat these keys in every keymap? Perhaps move them to one, game level keymap?
| act->addDefaultInputMapping("JOY_DOWN"); | ||
| scalpelInvKeymap->addAction(act); | ||
|
|
||
| act = new Action("EXIT", _("Exit")); |
There was a problem hiding this comment.
| act = new Action("EXIT", _("Exit")); | |
| // | |
| // INVENTORY KEYMAP | |
| // | |
| act = new Action("EXIT", _("Exit")); |
So, there is a visual distinction between them. Do the same for other keymaps
engines/sherlock/metaengine.cpp
Outdated
| String fileLabelPrefix = _("Save game slot"); | ||
|
|
||
| for (int i = 0; i < 9; ++i) { | ||
| fileLabel[i] = fileLabelPrefix + String::format(" %d", i + 1); |
There was a problem hiding this comment.
Nope. not going to work for all languages. Rather, have the translatable label as "Save game slot %d". In some languages the number will be in the middle of the sentence
engines/sherlock/metaengine.cpp
Outdated
| tattooFoolscapKeymap->addAction(act); | ||
|
|
||
| // I18N: (Game name: The Lost Files of Sherlock Holmes: The Case of the Rose Tattoo) cursor refers to the text cursor in the foolscap puzzle input field | ||
| act = new Action("UP", _("Move cursor Up")); |
There was a problem hiding this comment.
Shouldn't it be
| act = new Action("UP", _("Move cursor Up")); | |
| act = new Action("UP", _("Move cursor up")); |
and below
engines/sherlock/metaengine.cpp
Outdated
| tattooFilesNameKeymap->addAction(act); | ||
|
|
||
| // I18N: (Game name: The Lost Files of Sherlock Holmes: The Case of the Rose Tattoo) The game has a foolscap puzzle, where the player has to decode a hidden message on a piece of paper called a foolscap. this action is used to exit the puzzle | ||
| act = new Action("EXIT", _("Close foolscap puzzle")); |
There was a problem hiding this comment.
In general, does it matter which puzzle it is? Could it be just "Close puzzle"?
engines/sherlock/metaengine.cpp
Outdated
| tattooPasswordKeymap->addAction(act); | ||
|
|
||
| // I18N: (Game name: The Lost Files of Sherlock Holmes: The Case of the Rose Tattoo) This action is used to go to the start of the password input field | ||
| act = new Action("GOSTART", _("Go to start of password")); |
There was a problem hiding this comment.
Similarly, could it be more generic, like "Go to the line start"? And then reused in other translatable strings?
engines/sherlock/metaengine.cpp
Outdated
| act->addDefaultInputMapping("JOY_Y"); | ||
| tattooTalkKeymap->addAction(act); | ||
|
|
||
| act = new Action("EXIT", _("Exit verb menu")); |
There was a problem hiding this comment.
Shouldn't it be generit "Exit" or "Close window"? So then we reduce number of translateable strings
fb9fad8 to
0ce8a29
Compare
|
Thank you! |
| Keymap *animationKeymap = new Keymap(Keymap::kKeymapTypeGame, "animation", _("Animation keymappings")); | ||
| Keymap *scalpelScrollKeymap = new Keymap(Keymap::kKeymapTypeGame, "scalpel-scroll", _("Scroll keymappings")); | ||
| Keymap *scalpelKeymap = new Keymap(Keymap::kKeymapTypeGame, "scalpel", _("Main user interface keymappings")); | ||
| Keymap *scalpelDartsKeymap = new Keymap(Keymap::kKeymapTypeGame, "scalpel-darts", _("Darts minigame keymappings")); |
There was a problem hiding this comment.
scalpelDartsKeymap doesn't get added to keymaps, so this is unused and a memory leak
No description provided.