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

Hotkey interface implementation #2070

Merged
merged 51 commits into from May 7, 2013
Merged

Hotkey interface implementation #2070

merged 51 commits into from May 7, 2013

Conversation

cmocanu
Copy link
Contributor

@cmocanu cmocanu commented Apr 6, 2013

This partially fixes #1964 and #1893.
I managed to implement a workable interface for binding hotkeys. Although it is currently usable, there are some places where it could be improved:

  • the settings only take effect after restarting the game
  • the key names are sorted by length ( I have no idea why this happens, and could not find a fix)
  • key combinations are not supported: the interface actually stores a the list of pressed keys, but I don't know how to set them, and combinations might create other problems as well
  • there is currently no way of deleting bindings (only overwriting): I didn't know how to implement this; let me know if you have any suggestions

If the user tries to bind a key which already has a binding, instead of simply removing the old binding, I bind the key binded to the current action to the other action. This is done to prevent having an action without a corresponding key, which is currently not supported by the hotkey implementation.
The problem is that now major modifications to the apply_changes function must be made, and there are some cases in which it's not very clear what to do.
This is done to make it possible to get the actions list properly ordered.
This is not working properly: the listener only detects keypresses when no button has focus and the F keys still aren't properly detected. Also, the listener is used all throughout the main menu, breaking F9 in the first screen.
container = self.widget.findChild(name='keys_container')
button_container = self.widget.findChild(name='button_container')
sec_button_container = self.widget.findChild(name='sec_button_container')
for i in range(len(self.actions)):
Copy link
Member

Choose a reason for hiding this comment

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

The better way to handle this in idiomatic python is to use enumerate.

for i, action in enumerate(self.actions):

@ghost ghost assigned ChrisOelmueller Apr 8, 2013
@cmocanu
Copy link
Contributor Author

cmocanu commented May 2, 2013

After using ChrisOelmueller's hack to place the Hotkeys interface in the Settings menu and making the listeners work, there's only one problem left: I need to call some methods on Confirm and Reset to Default, but those buttons seems to be handled by FIFE, not pychan. Is there any way I could do this?
Also, I'd like to know if it's possible to create confimation pop-ups in the settings window ( Reset to Default seems to be able to do that, but again, it's done in FIFE).

@squiddy
Copy link
Member

squiddy commented May 2, 2013

Also, I'd like to know if it's possible to create confimation pop-ups in the settings window ( Reset to Default seems to be able to do that, but again, it's done in FIFE).

I'm (kinda) working on rewriting the settings entirely, so I wouldn't concentrate too much on this one. It definitly will be possible then. And it's not done in fife, but here: https://github.com/unknown-horizons/unknown-horizons/blob/master/horizons/engine/settingsdialog.py#L65

…tkey_interface.py.

This was done by storing the HotkeyConfiguration in the settingsDialog and calling te corresponding methods before calling applySettings() and setDefaults() from the settingsDialog.
@cmocanu
Copy link
Contributor Author

cmocanu commented May 3, 2013

Okay, I think it works now. Please let me know if you find any problems or if you think I did something (too) stupid in the code.
Also, we need to decide what to put in the Help menu.

@@ -22,6 +22,7 @@
from fife import fife

from horizons.util.python.callback import Callback
from horizons.gui.modules.hotkeys_settings import HotkeyConfiguration
Copy link
Member

Choose a reason for hiding this comment

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

This import looks obsolete.

@stubb
Copy link
Member

stubb commented May 3, 2013

Can you please merge the master branch in your branch, so it's possible again to test your branch with the last git version of fife and merging your PR will works too after that.

Conflicts:
	horizons/gui/keylisteners/keyconfig.py
	horizons/gui/widgets/pickbeltwidget.py
If the text of a button is "Press any key" and the user clicks another button, the interface should update the text of all buttons, so that no two buttons contain the text "Press any key" at the same time.
@stubb
Copy link
Member

stubb commented May 7, 2013

I noticed three things which should be fixed in my opinion before merging:

  • CONSOLE should be renamed to -> SHOW_FPS
  • A confirmation would be nice if a key is already in use
    And the third is already fixed while writing this comment.

@ChrisOelmueller
Copy link
Member

:shipit:

squiddy added a commit that referenced this pull request May 7, 2013
Hotkey interface implementation

Great work ThePawnbreak!
@squiddy squiddy merged commit a4130db into master May 7, 2013
@stubb
Copy link
Member

stubb commented May 7, 2013

Great work!

@nihathrael nihathrael deleted the hotkeys branch May 7, 2013 11:07
@stubb stubb changed the title Hotkey interface implementation Hotkey interface implementation Jan 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hotkey interface redesign
5 participants