Refactor|PlayerImpulse: Separate Impulse, PlayerImpulse and ImpulseAc…

…cumulator components

PlayerImpulse is Record-like (todo) description of a logical player
impulse. Each local player has their own set of ImpulseAccumulators,
which, accumulate and normalize multiple impulse triggers ready for
later rationalize by the player's Brain (on game side).

doomsday/api/api_player.h

@@ -49,16 +49,18 @@ enum {
  * Logical impulse types:
  */
 typedef enum impulsetype_e {
-    IT_NUMERIC,            ///< A numeric value determined by current device-control state.
-    IT_NUMERIC_TRIGGERED,  ///< Numeric, but accepts triggered states as well.
-    IT_BOOLEAN             ///< Always accepts triggered states.
+    IT_ANALOG,            ///< A numeric value determined by current device-control state.
+    IT_ANALOG_TRIGGERED,  ///< Analog, but accepts triggered states as well.
+    IT_BINARY             ///< Always accepts triggered states.
 } impulsetype_t;
 
+#define IMPULSETYPE_IS_TRIGGERABLE(t) ((t) == IT_ANALOG_TRIGGERED || (t) == IT_BINARY)
+
 typedef impulsetype_t controltype_t;
 
-#define CTLT_NUMERIC           IT_NUMERIC
-#define CTLT_NUMERIC_TRIGGERED IT_NUMERIC_TRIGGERED
-#define CTLT_IMPULSE           IT_BOOLEAN
+#define CTLT_NUMERIC           IT_ANALOG
+#define CTLT_NUMERIC_TRIGGERED IT_ANALOG_TRIGGERED
+#define CTLT_IMPULSE           IT_BINARY
 
 /**
  * @defgroup playerFlags Player Flags

doomsday/client/include/ui/bindcontext.h

@@ -30,7 +30,7 @@ typedef int (*FallbackResponderFunc)(event_t *);
 typedef int (*DDFallbackResponderFunc)(ddevent_t const *);
 // todo ends
 
-class PlayerImpulse;
+struct PlayerImpulse;
 
 /**
  * Contextualized grouping of input (and windowing system) event bindings.

doomsday/client/include/ui/inputsystem.h

@@ -231,6 +231,10 @@ class InputSystem : public de::System
     /**
      * Creates a new binding context. The new context has the highest priority
      * of all existing contexts, and is inactive.
+     *
+     * @param name  A unique, symbolic name for the bind context.
+     *
+     * @return  Resultant BindContext. Owernship is retaied.
      */
     BindContext *newContext(de::String const &name);
 

doomsday/client/include/ui/playerimpulse.h

@@ -17,89 +17,64 @@
  * http://www.gnu.org/licenses</small>
  */
 
-#ifndef CLIENT_INPUTSYSTEM_PLAYERIMPULSE_H
-#define CLIENT_INPUTSYSTEM_PLAYERIMPULSE_H
+#ifndef CLIENT_PLAY_PLAYERIMPULSE_H
+#define CLIENT_PLAY_PLAYERIMPULSE_H
 
 #include <de/String>
 #include "api_player.h"
 
 /**
- * Describes a player interaction impulse.
+ * Receives player interaction impulses and normalizes them for later consumption
+ * by the player Brain (on game side).
  *
- * @todo Is "take" the wrong verb in this context?
- * Player impulses are acted upon by the player Brain (on game side). Does it make
- * sense for a "brain" to "consume" an impulse? (Also note established convention
- * in Qt containers for removing an element from the container). Perhaps we need
- * another abstraction here? -ds
+ * @todo The player Brain should have ownership of it's ImpulseAccumulators.
  *
  * @todo Cleanup client/server confusion. On server side, each player will have a
  * local model of a remote human player's impulses. However, Double-clicks can be
  * handled entirely on client side. -ds
+ *
+ * @ingroup playsim
  */
-class PlayerImpulse
+class ImpulseAccumulator
 {
 public:
-    impulsetype_t type;
+    enum AccumulatorType
+    {
+        Analog,
+        Binary
+    };
 
 public:
-    PlayerImpulse(int id, impulsetype_t type, de::String const &name,
-                  de::String bindContextName);
-
-    bool isTriggerable() const;
-
     /**
-     * Returns the unique identifier of the impulse.
+     * @param expireBeforeSharpTick  If the source of the accumulation has changed
+     * state when a sharp tick occurs, the accumulation will expire automatically.
+     * For example, if the key bound to "attack" is not held down when a sharp tick
+     * occurs, it should not considered active even though it has been pressed and
+     * released since the previous sharp tick.
      */
-    int id() const;
+    ImpulseAccumulator(int impulseId, AccumulatorType type, bool expireBeforeSharpTick);
 
     /**
-     * Returns the symbolic name of the impulse. This name is used when resolving
-     * or generating textual binding descriptors.
+     * Returns the unique identifier of the impulse.
      */
-    de::String name() const;
+    int impulseId() const;
 
-    /**
-     * Returns the symbolic name of the attributed binding context.
-     */
-    de::String bindContextName() const;
+    AccumulatorType type() const;
 
-#ifdef __CLIENT__
-    /**
-     * Returns @c true if one or more ImpulseBindings exist in @em any bindContext.
-     *
-     * @param playerNum  Console/player number.
-     */
-    bool haveBindingsFor(int playerNum) const;
-#endif
+    bool expireBeforeSharpTick() const;
 
-public:
-    /**
-     * @param playerNum  Console/player number.
-     */
-    void triggerBoolean(int playerNum);
+    void setPlayerNum(int newPlayerNum);
 
-    /**
-     * @param playerNum  Console/player number.
-     */
-    int takeBoolean(int playerNum);
+    // ---
 
-#ifdef __CLIENT__
-    /**
-     * @param playerNum  Console/player number.
-     * @param pos
-     * @param relOffset
-     */
-    void takeNumeric(int playerNum, float *pos = nullptr, float *relOffset = nullptr);
+    void receiveBinary();
 
-    /**
-     * @param playerNum  Console/player number.
-     */
-    int takeDoubleClick(int playerNum);
+    int takeBinary();
 
-    /**
-     * @param playerNum  Console/player number. Use @c < 0 || >= DDMAXPLAYERS for all.
-     */
-    void clearAccumulation(int playerNum = -1 /*all local players*/);
+#ifdef __CLIENT__
+    void takeAnalog(float *pos = nullptr, float *relOffset = nullptr);
+
+    void clearAll();
 
 public:
 
@@ -114,15 +89,38 @@ class PlayerImpulse
     DENG2_PRIVATE(d)
 };
 
+/**
+ * Describes a player interaction impulse.
+ *
+ * @ingroup playsim
+ */
+struct PlayerImpulse
+{
+    int id = 0;
+    impulsetype_t type;
+    de::String name;             ///< Symbolic. Used when resolving or generating textual binding descriptors.
+    de::String bindContextName;  ///< Symbolic name of the associated binding context.
+
+#ifdef __CLIENT__
+    /**
+     * Returns @c true if one or more bindings for this impulse exist, for the
+     * given @a localPlayer number in the associated BindContext.
+     *
+     * @param localPlayer  Local player number.
+     */
+    bool haveBindingsFor(int playerNumber) const;
+#endif
+};
+
 void P_ImpulseShutdown();
 
 PlayerImpulse *P_ImpulsePtr(int id);
 
 PlayerImpulse *P_ImpulseByName(de::String const &name);
 
 /**
- * Register the console commands and cvars of the player controls subsystem.
+ * Register the console commands and variables of this module.
  */
-void P_ConsoleRegister();
+void P_ImpulseConsoleRegister();
 
-#endif // CLIENT_INPUTSYSTEM_PLAYERIMPULSE_H
+#endif // CLIENT_PLAY_PLAYERIMPULSE_H

doomsday/client/src/con_config.cpp

@@ -227,7 +227,7 @@ static bool writeBindingsState(Path const &filePath)
 
                 fprintf(file, "bindcontrol local%i-%s \"%s\"\n",
                               bind.geti("localPlayer") + 1,
-                              impulse->name().toUtf8().constData(),
+                              impulse->name.toUtf8().constData(),
                               bind.composeDescriptor().toUtf8().constData());
                 return LoopContinue;
             });

doomsday/client/src/dd_main.cpp

@@ -3284,7 +3284,7 @@ static void consoleRegister()
     GL_Register();
     UI_Register();
     Demo_Register();
-    P_ConsoleRegister();
+    P_ImpulseConsoleRegister();
     I_Register();
 #endif
 

doomsday/client/src/ui/bindcontext.cpp

@@ -311,8 +311,7 @@ Record *BindContext::bindCommand(char const *eventDesc, char const *command)
     return nullptr;
 }
 
-Record *BindContext::bindImpulse(char const *ctrlDesc, PlayerImpulse const &impulse,
-    int localPlayer)
+Record *BindContext::bindImpulse(char const *ctrlDesc, PlayerImpulse const &impulse, int localPlayer)
 {
     DENG2_ASSERT(ctrlDesc);
     DENG2_ASSERT(localPlayer >= 0 && localPlayer < DDMAXPLAYERS);
@@ -322,12 +321,12 @@ Record *BindContext::bindImpulse(char const *ctrlDesc, PlayerImpulse const &impu
         std::unique_ptr<Record> newBind(new Record);
         ImpulseBinding bind(*newBind.get());
 
-        bind.configure(ctrlDesc, impulse.id(), localPlayer); // Assign a new unique identifier.
+        bind.configure(ctrlDesc, impulse.id, localPlayer); // Assign a new unique identifier.
         d->impulseBinds[localPlayer].append(newBind.release());
 
         LOG_INPUT_VERBOSE("Impulse " _E(b) "'%s'" _E(.) " of player%i now bound to \"%s\" in " _E(b) "'%s'" _E(.)
                           " with binding Id " _E(b) "%i")
-                << impulse.name() << (localPlayer + 1) << bind.composeDescriptor() << d->name << bind.geti("id");
+                << impulse.name << (localPlayer + 1) << bind.composeDescriptor() << d->name << bind.geti("id");
 
         /// @todo: In interactive binding mode, should ask the user if the
         /// replacement is ok. For now, just delete the other bindings.

doomsday/client/src/ui/inputsystem.cpp

@@ -1341,6 +1341,14 @@ int InputSystem::contextPositionOf(BindContext *context) const
 
 BindContext *InputSystem::newContext(String const &name)
 {
+    // Ensure the given name is unique.
+    if(hasContext(name))
+    {
+        LOG_AS("InputSystem::newContext");
+        LOG_INPUT_WARNING("Name '%s' is not unique - Won't replace") << name;
+        return nullptr;
+    }
+
     BindContext *context = new BindContext(name);
     d->contexts.prepend(context);
 
@@ -1446,7 +1454,7 @@ Record *InputSystem::bindImpulse(char const *ctrlDesc, char const *impulseDesc)
         return nullptr;
     }
 
-    BindContext *context = contextPtr(impulse->bindContextName());
+    BindContext *context = contextPtr(impulse->bindContextName);
     if(!context)
     {
         context = contextPtr(DEFAULT_BINDING_CONTEXT_NAME);
@@ -1622,7 +1630,7 @@ D_CMD(ListBindings)
             DENG2_ASSERT(impulse);
 
             LOG_INPUT_MSG("    [%3i] " _E(>) _E(b) "%s" _E(.) " player%i %s")
-                    << bind.geti("id") << bind.composeDescriptor() << (pl + 1) << impulse->name();
+                    << bind.geti("id") << bind.composeDescriptor() << (pl + 1) << impulse->name;
 
             return LoopContinue;
         });
@@ -1672,7 +1680,7 @@ void InputSystem::consoleRegister() // static
 
     // Variables:
     C_VAR_BYTE("input-conflict-zerocontrol", &zeroControlUponConflict, 0, 0, 1);
-    C_VAR_BYTE("input-sharp",                &useSharpInputEvents, 0, 0, 1);
+    C_VAR_BYTE("input-sharp",                &useSharpInputEvents,     0, 0, 1);
 
     // Commands:
     C_CMD_FLAGS("activatebcontext",     "s",        ActivateContext,    PROTECTED_FLAGS);
@@ -1771,7 +1779,7 @@ DENG_EXTERN_C int B_BindingsForControl(int localPlayer, char const *impulseNameC
             PlayerImpulse const *impulse = P_ImpulsePtr(bind.geti("impulseId"));
             DENG2_ASSERT(impulse);
 
-            if(!impulse->name().compareWithoutCase(impulseName))
+            if(!impulse->name.compareWithoutCase(impulseName))
             {
                 if(inverse == BFCI_BOTH ||
                    (inverse == BFCI_ONLY_NON_INVERSE && !(bind.geti("flags") & IBDF_INVERSE)) ||