Keybord Bindings

Programmers discuss here anything related to FreeOrion programming. Primarily for the developers to discuss.

Moderator: Committer

Message
Author
User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Keybord Bindings

#46 Post by Geoff the Medio »

strooka wrote:geoff, the pointer to the window, given in the hotkeymanager::connect functions is needed by gg.
No it isn't. You can connect one signal to annother using GG::Connect without supplying any pointers.

User avatar
strooka
Space Kraken
Posts: 165
Joined: Tue Jun 23, 2009 5:34 pm
Location: Bielefeld, Germany

Re: Keybord Bindings

#47 Post by strooka »

yes, but that signal to that it is connected to needs the pointer cause it connects to the approitate member function.
i could place all connects inside the hotkeymanager in an init function which is called in the coinstructor maybe, but that would be again like "hardcoded"...

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Keybord Bindings

#48 Post by Geoff the Medio »

strooka wrote:yes, but that signal to that it is connected to needs the pointer cause it connects to the approitate member function.
Signals do not need member functions to be connected to. Whether a signal is connected to another function doesn't change how you connect to that signal.

Again: HotKeyManager shouldn't connect signals to external functions (be they member functions or free functions).

Rather, set things up so that GetHotKeyManager().GetCommandSignal("signal-name") returns a signal that fires when the hotkey is pressed. The connection from that signal to a function that actually does something is set up by code that's outside the HKM, and the HKM doesn't need to know anything about what happens after the command signal fires.

User avatar
strooka
Space Kraken
Posts: 165
Joined: Tue Jun 23, 2009 5:34 pm
Location: Bielefeld, Germany

Re: Keybord Bindings

#49 Post by strooka »

Code: Select all

    void Connect(const std::string option_name, bool (MapWnd::*slot)(), MapWnd* pWnd) {
        //update/initialize hotkey contents from db
        HotKey hotkey = GetOptionsDB().Get<HotKey>(option_name);
        //set signal this connection won't be altered
        GG::Connect( *hotkey.GetWindowSignal(), slot, pWnd );
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        //set connection
        hotkey.UpdateConnection();
        GetOptionsDB().Set<HotKey>(option_name, hotkey);
    }

i could delete that line and let you manually connect it by yourself with GG::Connect with the signal function returned by "GetHotKeyManager().GetCommandSignal("signal-name") "
but i don't know if you had to call manually

Code: Select all

        //set connection
        hotkey.UpdateConnection();
        GetOptionsDB().Set<HotKey>(option_name, hotkey);
too.
it seems that it will work, too, but the coding effort will be the same.

i'll do it if you whish.

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Keybord Bindings

#50 Post by Geoff the Medio »

There should be no void Connect function that you've written. The HotKeyManager should internally set up the signal connections between keyboard accelerator signals and the signal returned by GetCommandSignal(). This would happen when adding commands, and would be updated whenever the hotkey assigned to a command is changed.

HotKey should not have an UpdateConnection function; HotKey should just be a struct that contains a keypress combination (including modkeys). HotKey should not contain signals or any other data besides a key and modkey.

User avatar
strooka
Space Kraken
Posts: 165
Joined: Tue Jun 23, 2009 5:34 pm
Location: Bielefeld, Germany

Re: Keybord Bindings

#51 Post by strooka »

look:

Code: Select all

class HotKey 
{
public:
    struct KeyValue
    {
        KeyValue( GG::Key key = GG::GGK_UNKNOWN, GG::Flags<GG::ModKey> mod_keys = GG::Flags<GG::ModKey>() ) :
            m_key(key),
            m_mod_keys(mod_keys)
        {}

        GG::Key m_key; //!< GG code for key stored
        GG::Flags<GG::ModKey> m_mod_keys; //!GG code for the modkey stored
    };
    //!ctors
    HotKey( const std::string& str, const std::string& option_name = "NO_NAME_SET" );
    HotKey( GG::Key key = GG::GGK_UNKNOWN, GG::Flags<GG::ModKey> mod_keys = GG::Flags<GG::ModKey>(),  const std::string& option_name = "NO_NAME_SET" );

    //!stream operators - for interacting with optionsDB
    friend std::ostream& operator<<(std::ostream& os, const HotKey& hotkey);
    friend std::istream& operator>>(std::istream& is, HotKey& hotkey);

    //!disables / enables signal
    void Block();
    void Unblock();

    //!sets the values and updates the connection to the keyboard
    void SetKey( std::pair< GG::Key, GG::Flags<GG::ModKey> > Pair );

    bool GetCtrl() const { return m_key_value.m_mod_keys & (GG::MOD_KEY_CTRL); }//!< Is CTRL key pressed to activate hotkey? 
    bool GetAlt() const { return m_key_value.m_mod_keys & (GG::MOD_KEY_ALT); } //!< ALT key?
    bool GetShift() const { return m_key_value.m_mod_keys & (GG::MOD_KEY_LSHIFT | GG::MOD_KEY_RSHIFT); } //!< SHIFT key?
    bool GetMeta() const { return m_key_value.m_mod_keys & (GG::MOD_KEY_META); } //!< META key?

    //!return key value and modkey value
    GG::Key GetKey() const { return m_key_value.m_key; }
    GG::Flags<GG::ModKey> GetModKeys() const { return m_key_value.m_mod_keys; }
    KeyValue& GetKeyValue() { return m_key_value; }

    //!test wether a given key is a modkey
    static bool IsModKey(GG::Key key) {
        if( (key == GG::GGK_RSHIFT) || (key == GG::GGK_LSHIFT) || (key == GG::GGK_RCTRL) || (key == GG::GGK_LCTRL) || (key == GG::GGK_LALT) || (key == GG::GGK_RALT) || (key == GG::GGK_LMETA) || (key == GG::GGK_RMETA) )
            return true;
        else
            return false;
    }

    //!gets the connection to the keyboard
    boost::signals::connection& GetConnection();
    //!gets the connection to the handler function
    SignalPointerType GetWindowSignal() { return m_p_window_signal; }

    //!changes the connection to the keyboard
    void UpdateConnection(GG::Key last_key = GG::GGK_UNKNOWN, GG::Flags<GG::ModKey> last_mod_keys = GG::Flags<GG::ModKey>());

    //!gets and sets the name of the hotkey, etc."UI.Hotkey.Mapwnd.ReturnToMap"
    void SetOptionName(const std::string& str) { m_option_name = str; }
    std::string GetOptionName() const { return m_option_name; }
private:
    KeyValue m_key_value;
    std::string m_option_name; //!name of the option
    SignalPointerType m_p_window_signal; //!the pointer to the signal that stores the window handler function
    boost::signals::connection m_connection; //!the connection to tke keyboard
};

...

//!manages a list of KeyValue Keys

class HotKeyManager
{
public:
    HotKeyManager();

    //!used in the GG::Connect function to connect to the Hotkey
    SignalPointerType GetCommandSignal(const std::string& hotkey_name);

    //!simple Enabling / Disabling of some keys
    void DisableAlphaNumAccels();
    void EnableAlphaNumAccels();

    std::map<const std::string, HotKey::KeyValue> m_keys_list;
private:
    static HotKeyManager* s_HotKeyManager;

    friend HotKeyManager& GetHotKeyManager();
};


mapwnd:

void MapWnd::ConnectKeyboardAcceleratorSignals()
{
    m_keyboard_accelerator_signals.insert(
        GG::Connect(*GetHotKeyManager().GetCommandSignal("UI.HotKey.MapWnd.ReturnToMap"),
                    &MapWnd::ReturnToMap, this));
...
i hope this is good enough to be added to the svn.

tomorrow i'll make some debugging

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Keybord Bindings

#52 Post by Geoff the Medio »

It would be better for GetCommandSignal to return a reference to a signal than a pointer to a signal. See GG::GUI::AcceleratorSignal

Your struct KeyValue is pretty much what struct HotKey should be, aside from the streaming operators and utility functions like IsModKey. Store all the signal connections in the HKM, not in the HotKey struct. You don't need a GetWindowSignal function, either.

MapWnd and other UI classes probably don't need to store command signals... They're all connected when the UI object is intialized and never need to be modified, and there is no need to disconnect or reconnect them. All the reconnecting and such would happen within the HKM after rebinding hotkeys, for example, but the command signal for zooming in should always zoom in on the MapWnd.

User avatar
strooka
Space Kraken
Posts: 165
Joined: Tue Jun 23, 2009 5:34 pm
Location: Bielefeld, Germany

Re: Keybord Bindings

#53 Post by strooka »

Store all the signal connections in the HKM, not in the HotKey struct. You don't need a GetWindowSignal function, either.
i don't think so. if there is a stream operator that works on a hotkey, it needs to operate on the connection, and storing them all in the hotkeymanager would be a rather buggy implementation and place much overhead.

i need the GetWindowSignal function to return the pointer to the signal of a hotkey.

User avatar
strooka
Space Kraken
Posts: 165
Joined: Tue Jun 23, 2009 5:34 pm
Location: Bielefeld, Germany

Re: Keybord Bindings

#54 Post by strooka »

here is a new implementation of the hotkeymanager.

it is debugged and runs.

note that i only made minor changes to map wnd, its only to react on the results.
Attachments
NewHotKeyManager.zip
the modified hotkeymanager
(101.73 KiB) Downloaded 56 times

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Keybord Bindings

#55 Post by Geoff the Medio »

strooka wrote:if there is a stream operator that works on a hotkey, it needs to operate on the connection
Why? All the connection details between keyboard accelerator signals and command signals are internal to the hotkey manager, so there's no reason to have the publically visible hotkey struct include any signal connections.

As an alternative, you could make the KeyValue struct public, and move the stream operators from HotKey to KeyValue, and store KeyValue in the OptionsDB instead of HotKey. Then you could make HotKey contain a signal, but make HotKey not declared outside of HotKeyManager.cpp. There's no reason any code outside of the HKM needs to know anything about how keyboard accelerator signals are connected to command signals, and currently the publically visible HotKey class exposes that information unnecessarily.
storing them all in the hotkeymanager would be a rather buggy implementation
Why? There's no reasonable reason it would have to be "buggy", unless it was poorly implemented.
and place much overhead.
Again, why?
i need the GetWindowSignal function to return the pointer to the signal of a hotkey.
It should be renamed then, as it has nothing to do with windows.

Edit:

There shouldn't be GetOptionName or SetOptionName functions in HotKey. These are properties of commands, not hotkeys that activate commands.

HotKeyManager does not need the following functions public: GetHotKey, InsertHotKey, RemoveHotKey. Regardless of how you implement things internally, HotKeyManager only need to have an AddCommand function and a GetCommandSignal function, and funcitons to set the hotkey modes that are active. Details of which keypress combinations are active and connected to command signals should be handled internally, and automatically update without additional external code if the keypress assigned to a command signal changes in the options db.

/Edit

User avatar
strooka
Space Kraken
Posts: 165
Joined: Tue Jun 23, 2009 5:34 pm
Location: Bielefeld, Germany

Re: Keybord Bindings

#56 Post by strooka »

ok, here is the latest release:

Code: Select all

//!class HotKey:
//!the key is, that there is a signal stored that connects to a window handler function.
//!and a connection is made to the keyboard that connects to the function call operator of the signal.
//!when a handled key is pressed the function call operator of the signal will be called, calling the //!handler function.
//!resetting of the connection to the keyboard sets the key input of the keyboard anew.
//!also there need not be a signal connected to a function if there is no window object, there will be //!just a empty function call operator executed.

typedef boost::signal< bool() >                          SignalType;
typedef boost::shared_ptr<SignalType>                    SignalPointerType;

struct KeyValue;

/////////////////////////////////////////////
// Free Functions
/////////////////////////////////////////////
//!wrapper function that adds a hotkey to the OptionsDB - use with the other db.Add() functions
void AddCommand(const std::string& name, const std::string& description, KeyValue default_value, OptionsDB& db);

/////////////////////////////////////////////
// class KeyValue
/////////////////////////////////////////////

struct KeyValue
{
    KeyValue( GG::Key key = GG::GGK_UNKNOWN, GG::Flags<GG::ModKey> mod_keys = GG::Flags<GG::ModKey>() ) :
        m_key(key),
        m_mod_keys(mod_keys)
    {}

    GG::Key m_key; //!< GG code for key stored
    GG::Flags<GG::ModKey> m_mod_keys; //!GG code for the modkey stored
};

/////////////////////////////////////////////
// Class HotKeyManager
/////////////////////////////////////////////

//!manages a list of KeyValue Keys

class HotKeyManager
{
public:
    HotKeyManager();

    //!used in the GG::Connect function to connect to the Hotkey
    SignalType& GetCommandSignal(const std::string& hotkey_name);

    //!disables / enables signal
    void Block(const std::string& hotkey_name);
    void Unblock(const std::string& hotkey_name);
    void Block(const KeyValue& key_value);
    void Unblock(const KeyValue& key_value);

    //!simple Enabling / Disabling of some keys
    void DisableAlphaNumAccels();
    void EnableAlphaNumAccels();

    class HotKey 
    {
    public:
        //!ctors
        HotKey() : m_hotkey_name() {}
        HotKey( const std::string& str, const std::string& hotkey_name );
        HotKey( const std::string& hotkey_name, GG::Key key = GG::GGK_UNKNOWN, GG::Flags<GG::ModKey> mod_keys = GG::Flags<GG::ModKey>() );

        //!return key value and modkey value
        GG::Key GetKey() const;
        GG::Flags<GG::ModKey> GetModKeys() const;
        KeyValue& GetKeyValue() const;
        const std::string GetStringValue() const;

        //!Adds an KeyValue to the list in HotkeyManager
        void AddKey(KeyValue& key_value);

        SignalPointerType m_p_window_signal; //!the pointer to the signal that stores the window handler function
        std::string m_hotkey_name; //!name of the kotkey e.g. "UI.HotKey.Mapwnd..."
    private:
        //!sets the values and updates the connection to the keyboard
        void SetKey( std::pair< GG::Key, GG::Flags<GG::ModKey> > Pair );
    
        //!changes the connection to the keyboard
        void UpdateConnection(GG::Key last_key = GG::GGK_UNKNOWN, GG::Flags<GG::ModKey> last_mod_keys = GG::Flags<GG::ModKey>());
    
        //!stream operators - for interacting with optionsDB
        friend std::ostream& operator<<(std::ostream& os, const HotKey& hotkey);
        friend std::istream& operator>>(std::istream& is, HotKey& hotkey);

        boost::signals::connection m_connection; //!the connection to tke keyboard

        friend void HotKeyManager::Block(const std::string& hotkey_name);
        friend void HotKeyManager::Unblock(const std::string& hotkey_name);
        friend SignalType& HotKeyManager::GetCommandSignal(const std::string& hotkey_name);

        friend void HKEdit::LButtonDown(const GG::Pt& pt, GG::Flags<GG::ModKey> mod_keys); 
    };
private:
    std::map<const std::string, KeyValue> m_keys_list;

    static HotKeyManager* s_HotKeyManager;

    friend GG::Key HotKey::GetKey() const;
    friend GG::Flags<GG::ModKey> HotKey::GetModKeys() const;
    friend KeyValue& HotKey::GetKeyValue() const;
    friend void HotKey::AddKey(KeyValue& key_value);

    friend void HKEdit::LButtonDown(const GG::Pt& pt, GG::Flags<GG::ModKey> mod_keys);

    friend HotKeyManager& GetHotKeyManager();
    friend void AddCommand(const std::string& name, const std::string& description, KeyValue default_value, OptionsDB& db);
};


/** returns the single instance of the class HotKeyManager*/

HotKeyManager& GetHotKeyManager();

i ve also removed two bugs: it doesn't let you define ambigousities of keys and there was a initialisation bug if the game wasn't started, the game crashed.
Attachments
HotKeyManager0.9.tar.gz
version 0.9 of HotKeyManager
(93.28 KiB) Downloaded 74 times

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Keybord Bindings

#57 Post by Geoff the Medio »

The code in your posted file (particularly HotKeyManager.h) doesn't match the posted code snippit. Did you post an old version of a file?

However:

It looks like class HotKey doesn't need to be declared or defined in HotKeyManager.h. Keep it local to HotKeyManager.cpp if possible.

Do you really need to pass in an OptionsDB reference to the AddCommand function? Presumably it could call RegisterOptions itself internally...?

User avatar
strooka
Space Kraken
Posts: 165
Joined: Tue Jun 23, 2009 5:34 pm
Location: Bielefeld, Germany

Re: Keybord Bindings

#58 Post by strooka »

oh, sorry, here are the new files:

Code: Select all

// -*- C++ -*-
#ifndef _HotKeyManager_h_
#define _HotKeyManager_h_

#include "../util/OptionsDB.h"
#include "CUIWnd.h"
#include "CUIControls.h"

#include <map>

typedef boost::signal< bool() >                          SignalType;
typedef boost::shared_ptr<SignalType>                    SignalPointerType;

struct KeyValue;

/////////////////////////////////////////////
// Free Functions
/////////////////////////////////////////////
//!wrapper function that adds a hotkey to the OptionsDB - use with the other db.Add() functions
void AddCommand(const std::string& name, const std::string& description, KeyValue default_value, OptionsDB& db);

////////////////////////////////////
// class HKEdit, KeyInputDialog
///////////////////////////////////

class HKEdit : public CUIEdit
{
public:
    /** \name Structors */ //@{
    HKEdit(GG::X x, GG::Y y, GG::X w, const std::string& str, const std::string& option_name, const boost::shared_ptr<GG::Font>& font = boost::shared_ptr<GG::Font>(),
            GG::Clr border_color = ClientUI::CtrlBorderColor(), GG::Clr text_color = ClientUI::TextColor(),
            GG::Clr interior = ClientUI::CtrlColor(), GG::Flags<GG::WndFlag> flags = GG::INTERACTIVE); ///< basic ctor
    //@}

    /** \name Mutators */ //@{
    virtual void LButtonDown(const GG::Pt& pt, GG::Flags<GG::ModKey> mod_keys);
    //@}
private:
    const std::string m_option_name;
};

class KeyInputDialog : public CUIWnd
{
public:
    KeyInputDialog(const std::string& msg_text, const std::string& bn_text);

    virtual void KeyPress(GG::Key key, boost::uint32_t key_code_point, GG::Flags<GG::ModKey> mod_keys);
    const std::pair< GG::Key, GG::Flags<GG::ModKey> > Result() const { return m_result; }
private:
    void BnClicked();

    std::pair< GG::Key, GG::Flags<GG::ModKey> > m_result;
    CUIButton* m_bn;
};

/////////////////////////////////////////////
// class KeyValue
/////////////////////////////////////////////

struct KeyValue
{
    KeyValue( GG::Key key = GG::GGK_UNKNOWN, GG::Flags<GG::ModKey> mod_keys = GG::Flags<GG::ModKey>() ) :
        m_key(key),
        m_mod_keys(mod_keys)
    {}

    std::string ToString();

    GG::Key m_key; //!< GG code for key stored
    GG::Flags<GG::ModKey> m_mod_keys; //!GG code for the modkey stored
};

/////////////////////////////////////////////
// Class HotKeyManager
/////////////////////////////////////////////

//!manages a list of KeyValue Keys

class HotKeyManager
{
public:
    HotKeyManager();

    //!used in the GG::Connect function to connect to the Hotkey
    SignalType& GetCommandSignal(const std::string& hotkey_name);

    //!disables / enables signal
    void Block(const std::string& hotkey_name);
    void Unblock(const std::string& hotkey_name);
    void Block(const KeyValue& key_value);
    void Unblock(const KeyValue& key_value);

    //!simple Enabling / Disabling of some keys
    void DisableAlphaNumAccels();
    void EnableAlphaNumAccels();

    //!the list of the Keys
    std::map<const std::string, KeyValue> m_keys_list;

private:
    static HotKeyManager* s_HotKeyManager;

    friend void HKEdit::LButtonDown(const GG::Pt& pt, GG::Flags<GG::ModKey> mod_keys);

    friend HotKeyManager& GetHotKeyManager();
};


/** returns the single instance of the class HotKeyManager*/

HotKeyManager& GetHotKeyManager();


#endif // _HotKeyManager_h_
note: i had to include the edit windows HKEdit, KeyInputDialog in the hotkeymanager.h since class HotKey is just acting internally now and they act on hotkeys.
Do you really need to pass in an OptionsDB reference to the AddCommand function? Presumably it could call RegisterOptions itself internally...?
i think this would be too much. we should leave it like it is.
Attachments
HotKeyManager1.0.zip
with hidden HotKey class
(98.77 KiB) Downloaded 70 times

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Keybord Bindings

#59 Post by Geoff the Medio »

It's getting better. However:

If AddCommand is going to be a free function, ie. not a function in the HotKeyManager class, it should be named something like AddHotKeyCommand.

You don't need to make GetHotKeyManager a friend function of HotKeyManager and then reference the static variable inside... Just set up the manager getting and singleton tracking like the other managers already in the code, like TechManager.

You should be storing KeyValue in the options db, not HotKey. HotKey should be internal to the HotKeyManager, or at least at file scope in HotKeyManager.cpp. The HotKeyManager can store your HotKey class if that's how you want to implement things, which seems to be the case. You could then look up HotKey objects from the HotKeyManager's internal storage, rather than using the OptionsDB to do this storage and retreival. Outside the HKM, and in particular in the OptionsDB, you should just store KeyValues, indexed by command name. This is better because the KeyValue is really the only part that's actually stored, so it's more consistent and easier to understand.

Consequently, the streaming operators should exist on the KeyValue struct, and not on HotKey.

KeyValue will still be defined in the HotKeyManager.h file, so it can be used in code outside the HotKeyManager.cpp file, so HKEdit and KeyInputDialog can use it without needing to be defined in HotKeyManager.cpp.

Since HotKey is / will be a fairly complicated class, with signals inside it and such, it's probably best to store HotKey objects in the HKM by pointer instead of by value, so that you don't end up with temporary copies being made when the map storing them is modified. To do this with minimal coplication, you could add a

std::map<std::string, boost::shared_ptr<HotKey> >

as a member variable of HotKeyManager, and then just forward-declare HotKey in HotKeyManager.h. The definition of HotKeyManager would remain in HotKeyManager.cpp, so that other code doesn't need to know about how it works internally.

The Block and Unblock functions might be clearer if be renamed BlockCommand and UnblockCommand, and the parameter name should be command_name not hotkey_name.

There still needs to be a way to assign a group label or labels to commands, so that they can be enabled or disabled in groups.

Post Reply