Conversation
criezy
left a comment
There was a problem hiding this comment.
It looks good, and is working well. But I added a few comments on things that I think could maybe be improved.
|
|
||
| keymapper->getKeymap("menu")->setEnabled(false); | ||
| keymapper->getKeymap("supernova-default")->setEnabled(true); | ||
| keymapper->getKeymap("improved-mode")->setEnabled(true); |
There was a problem hiding this comment.
I am wondering, would it make sense to only enable this keymap if _improved is true rather than check for _improved in GameManager::processInput?
There was a problem hiding this comment.
That is possible and should work. however it would mean writing checks for _improved every where this keymap is enabled, rather than a single check in GameManager::processInput . let me know if I should change it.
There was a problem hiding this comment.
My suggestion was to replace keymapper->getKeymap("improved-mode")->setEnabled(true); with keymapper->getKeymap("improved-mode")->setEnabled(_improved); (or _vm->_improved). But that was a minor suggestion which I think might be a little bit cleaner, and I think your code is fine as it is. So unless another reviewer thinks it would be better to change it, I think you can leave it as it is currently.
There was a problem hiding this comment.
Ahh, it seems I misunderstood. I've left it unchanged for now.
criezy
left a comment
There was a problem hiding this comment.
It seems good to me.
There is an open question on whether it would be cleaner to only enable the improved keymap when the improved option is on. But I don't see any drawbacks with the way it is currently handled.
|
Thank you! |
No description provided.