Revise: update Key code to be more like other items

This commit makes the enableKey(<name>) and disable(<name.) lua functions
return true only if a key with the given name was found (instead of when
any Key was present in the profile) - which is more like the handling for
other items such as Triggers and Aliases.

It also arranges for all temporary keys to be "blown-away" when the profile
is reset - which was not happening even though it does happen for other
item types.

Furthermore when the "Emergency Stop" is activated it now also disables all
the keys like it disables all timers/aliases/triggers.  I think this is
desirable because if the user sets a binding on a normal key with no
modifiers that it is impossible to type the character on that key into the
profile's command line - so being able to disable the interception of the
key by a temporary key (that will not show up in the editor) is just one
reason to add keys to what the control does...!

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>

src/Host.cpp

@@ -344,10 +344,12 @@ void Host::resetProfile()
     mudlet::self()->mTimerMap.clear();
     getTimerUnit()->removeAllTempTimers();
     getTriggerUnit()->removeAllTempTriggers();
+    getKeyUnit()->removeAllTempKeys();
 
 
     mTimerUnit.doCleanup();
     mTriggerUnit.doCleanup();
+    mKeyUnit.doCleanup();
     mpConsole->resetMainConsole();
     mEventHandlerMap.clear();
     mEventMap.clear();
@@ -362,7 +364,7 @@ void Host::resetProfile()
     getActionUnit()->compileAll();
     getKeyUnit()->compileAll();
     getScriptUnit()->compileAll();
-    //getTimerUnit()->compileAll();
+    // All the Timers are NOT compiled here;
     mResetProfile = false;
 
     mTimerUnit.reenableAllTriggers();
@@ -504,13 +506,15 @@ void Host::stopAllTriggers()
     mTriggerUnit.stopAllTriggers();
     mAliasUnit.stopAllTriggers();
     mTimerUnit.stopAllTriggers();
+    mKeyUnit.stopAllTriggers();
 }
 
 void Host::reenableAllTriggers()
 {
     mTriggerUnit.reenableAllTriggers();
     mAliasUnit.reenableAllTriggers();
     mTimerUnit.reenableAllTriggers();
+    mKeyUnit.reenableAllTriggers();
 }
 
 QPair<QString, QString> Host::getSearchEngine()

src/KeyUnit.cpp

@@ -78,6 +78,20 @@ void KeyUnit::compileAll()
     }
 }
 
+void KeyUnit::stopAllTriggers()
+{
+    for (auto key : mKeyRootNodeList) {
+        key->disableFamily();
+    }
+}
+
+void KeyUnit::reenableAllTriggers()
+{
+    for (auto key : mKeyRootNodeList) {
+        key->enableFamily();
+    }
+}
+
 TKey* KeyUnit::findKey(QString& name)
 {
     QMap<QString, TKey*>::const_iterator it = mLookupTable.constFind(name);
@@ -91,9 +105,15 @@ TKey* KeyUnit::findKey(QString& name)
 bool KeyUnit::enableKey(const QString& name)
 {
     bool found = false;
-    QMutexLocker locker(&mKeyUnitLock);
-    for (auto key : mKeyRootNodeList) {
-        key->enableKey(name);
+    QMap<QString, TKey*>::const_iterator it = mLookupTable.constFind(name);
+    while (it != mLookupTable.cend() && it.key() == name) {
+        TKey* pT = it.value();
+        // Unlike the TTriggerUnit version of this code we directly set
+        // the mActive flag (and it shows up in the editor) rather than the
+        // mUserActiveState one (which does not)
+        // So do not use pT->setIsActive(true) here:
+        pT->enableKey(name);
+        ++it;
         found = true;
     }
     return found;
@@ -102,9 +122,15 @@ bool KeyUnit::enableKey(const QString& name)
 bool KeyUnit::disableKey(const QString& name)
 {
     bool found = false;
-    QMutexLocker locker(&mKeyUnitLock);
-    for (auto key : mKeyRootNodeList) {
-        key->disableKey(name);
+    QMap<QString, TKey*>::const_iterator it = mLookupTable.constFind(name);
+    while (it != mLookupTable.cend() && it.key() == name) {
+        TKey* pT = it.value();
+        // Unlike the TTriggerUnit version of this code we directly clear
+        // the mActive flag (and it shows up in the editor) rather than the
+        // mUserActiveState one (which does not)
+        // So do not use pT->setIsActive(false) here:
+        pT->disableKey(name);
+        ++it;
         found = true;
     }
     return found;
@@ -128,6 +154,16 @@ bool KeyUnit::killKey(QString& name)
     return false;
 }
 
+void KeyUnit::removeAllTempKeys()
+{
+    for (auto key : mKeyRootNodeList) {
+        if (key->isTemporary()) {
+            key->setIsActive(false);
+            markCleanup(key);
+        }
+    }
+}
+
 void KeyUnit::addKeyRootNode(TKey* pT, int parentPosition, int childPosition)
 {
     if (!pT) {
@@ -170,13 +206,12 @@ void KeyUnit::reParentKey(int childID, int oldParentID, int newParentID, int par
         pOldParent->popChild(pChild);
     }
     if (!pOldParent) {
+        // CHECKME: TriggerUnit copy of this code uses mXxxxxRootNodeList.remove(pChild) - which is best?
         removeKeyRootNode(pChild);
     }
     if (pNewParent) {
         pNewParent->addChild(pChild, parentPosition, childPosition);
         pChild->setParent(pNewParent);
-        //cout << "dumping family of newParent:"<<endl;
-        //pNewParent->Dump();
     } else {
         pChild->Tree<TKey>::setParent(nullptr);
         addKeyRootNode(pChild, parentPosition, childPosition);
@@ -253,7 +288,10 @@ void KeyUnit::addKey(TKey* pT)
         return;
     }
 
-    pT->setID(getNewID());
+    if (!pT->getID()) {
+        // Only get a new Id if it has not been previously set (the default is 0)
+        pT->setID(getNewID());
+    }
 
     mKeyMap.insert(pT->getID(), pT);
 }
@@ -382,8 +420,8 @@ QString KeyUnit::assembleReport()
 
 void KeyUnit::markCleanup(TKey* pT)
 {
-    for (auto it = mCleanupList.begin(); it != mCleanupList.end(); it++) {
-        if (*it == pT) {
+    for (auto key : mCleanupList) {
+        if (key == pT) {
             return;
         }
     }
@@ -392,8 +430,8 @@ void KeyUnit::markCleanup(TKey* pT)
 
 void KeyUnit::doCleanup()
 {
-    for (auto it = mCleanupList.begin(); it != mCleanupList.end(); it++) {
-        delete *it;
+    for (auto key : mCleanupList) {
+        delete key;
     }
     mCleanupList.clear();
 }

src/KeyUnit.h

@@ -50,6 +50,7 @@ class KeyUnit
     }
 
     TKey* getKey(int id);
+    void removeAllTempKeys();
     void compileAll();
     TKey* findKey(QString & name);
     bool enableKey(const QString& name);
@@ -67,6 +68,8 @@ class KeyUnit
     bool processDataStream(int, int);
     void markCleanup( TKey * pT );
     void doCleanup();
+    void stopAllTriggers();
+    void reenableAllTriggers();
 
     QMultiMap<QString, TKey*> mLookupTable;
     std::list<TKey*> mCleanupList;

src/TriggerUnit.cpp

@@ -279,7 +279,6 @@ void TriggerUnit::compileAll()
 void TriggerUnit::stopAllTriggers()
 {
     for (auto trigger : mTriggerRootNodeList) {
-        QString name = trigger->getName();
         trigger->disableFamily();
     }
 }