Merge pull request #9789 from davidkelly/dk/3313

Remember gain settings when pal is closed and reopened

interface/resources/qml/hifi/NameCard.qml

@@ -335,7 +335,7 @@ Item {
             }
         }
 
-        // Per-Avatar Gain Slider 
+        // Per-Avatar Gain Slider
         Slider {
             id: gainSlider
             // Size
@@ -345,7 +345,7 @@ Item {
             anchors.verticalCenter: nameCardVUMeter.verticalCenter
             // Properties
             visible: !isMyCard && selected
-            value: pal.gainSliderValueDB[uuid] ? pal.gainSliderValueDB[uuid] : 0.0
+            value: Users.getAvatarGain(uuid)
             minimumValue: -60.0
             maximumValue: 20.0
             stepSize: 5
@@ -369,7 +369,7 @@ Item {
                     mouse.accepted = false
                 }
                 onReleased: {
-                    // the above mouse.accepted seems to make this 
+                    // the above mouse.accepted seems to make this
                     // never get called, nonetheless...
                     mouse.accepted = false
                 }
@@ -393,14 +393,9 @@ Item {
     }
 
     function updateGainFromQML(avatarUuid, sliderValue, isReleased) {
-        if (isReleased || pal.gainSliderValueDB[avatarUuid] !== sliderValue) {
+        Users.setAvatarGain(avatarUuid, sliderValue);
-            pal.gainSliderValueDB[avatarUuid] = sliderValue;
+        if (isReleased) {
-            var data = {
+           UserActivityLogger.palAction("avatar_gain_changed", avatarUuid);
-                sessionId: avatarUuid,
-                gain: sliderValue,
-                isReleased: isReleased
-            };
-            pal.sendToScript({method: 'updateGain', params: data});
         }
     }
 }

interface/resources/qml/hifi/Pal.qml

@@ -2,7 +2,7 @@
 //  Pal.qml
 //  qml/hifi
 //
-//  People Action List 
+//  People Action List
 //
 //  Created by Howard Stearns on 12/12/2016
 //  Copyright 2016 High Fidelity, Inc.
@@ -37,9 +37,6 @@ Rectangle {
     property var ignored: ({}); // Keep a local list of ignored avatars & their data. Necessary because HashMap is slow to respond after ignoring.
     property var userModelData: [] // This simple list is essentially a mirror of the userModel listModel without all the extra complexities.
     property bool iAmAdmin: false
-    // Keep a local list of per-avatar gainSliderValueDBs. Far faster than fetching this data from the server.
-    // NOTE: if another script modifies the per-avatar gain, this value won't be accurate!
-    property var gainSliderValueDB: ({});
 
     HifiConstants { id: hifi }
 
@@ -270,7 +267,7 @@ Rectangle {
                 // Anchors
                 anchors.left: parent.left
             }
-            
+
             // This CheckBox belongs in the columns that contain the stateful action buttons ("Mute" & "Ignore" for now)
             // KNOWN BUG with the Checkboxes: When clicking in the center of the sorting header, the checkbox
             // will appear in the "hovered" state. Hovering over the checkbox will fix it.
@@ -306,7 +303,7 @@ Rectangle {
                     checked = Qt.binding(function() { return (model[styleData.role])})
                 }
             }
-            
+
             // This Button belongs in the columns that contain the stateless action buttons ("Silence" & "Ban" for now)
             HifiControls.Button {
                 id: actionButton
@@ -538,7 +535,7 @@ Rectangle {
                 }
             }
             break;
-        case 'updateAudioLevel': 
+        case 'updateAudioLevel':
             for (var userId in message.params) {
                 var audioLevel = message.params[userId];
                 // If the userId is 0, we're updating "myData".
@@ -554,9 +551,8 @@ Rectangle {
                 }
             }
             break;
-        case 'clearLocalQMLData': 
+        case 'clearLocalQMLData':
             ignored = {};
-            gainSliderValueDB = {};
             break;
         case 'avatarDisconnected':
             var sessionID = message.params[0];

libraries/networking/src/NodeList.cpp

@@ -49,7 +49,7 @@ NodeList::NodeList(char newOwnerType, int socketListenPort, int dtlsListenPort)
     setCustomDeleter([](Dependency* dependency){
         static_cast<NodeList*>(dependency)->deleteLater();
     });
-    
+
     auto addressManager = DependencyManager::get<AddressManager>();
 
     // handle domain change signals from AddressManager
@@ -85,8 +85,8 @@ NodeList::NodeList(char newOwnerType, int socketListenPort, int dtlsListenPort)
     connect(&_domainHandler, &DomainHandler::icePeerSocketsReceived, this, &NodeList::pingPunchForDomainServer);
 
     auto accountManager = DependencyManager::get<AccountManager>();
-    
+
-    // assume that we may need to send a new DS check in anytime a new keypair is generated 
+    // assume that we may need to send a new DS check in anytime a new keypair is generated
     connect(accountManager.data(), &AccountManager::newKeypair, this, &NodeList::sendDomainServerCheckIn);
 
     // clear out NodeList when login is finished
@@ -101,7 +101,7 @@ NodeList::NodeList(char newOwnerType, int socketListenPort, int dtlsListenPort)
 
     // anytime we get a new node we may need to re-send our set of ignored node IDs to it
     connect(this, &LimitedNodeList::nodeActivated, this, &NodeList::maybeSendIgnoreSetToNode);
-    
+
     // setup our timer to send keepalive pings (it's started and stopped on domain connect/disconnect)
     _keepAlivePingTimer.setInterval(KEEPALIVE_PING_INTERVAL_MS); // 1s, Qt::CoarseTimer acceptable
     connect(&_keepAlivePingTimer, &QTimer::timeout, this, &NodeList::sendKeepAlivePings);
@@ -161,11 +161,11 @@ qint64 NodeList::sendStatsToDomainServer(QJsonObject statsObject) {
 
 void NodeList::timePingReply(ReceivedMessage& message, const SharedNodePointer& sendingNode) {
     PingType_t pingType;
-    
+
     quint64 ourOriginalTime, othersReplyTime;
-    
+
     message.seek(0);
-    
+
     message.readPrimitive(&pingType);
     message.readPrimitive(&ourOriginalTime);
     message.readPrimitive(&othersReplyTime);
@@ -199,7 +199,7 @@ void NodeList::timePingReply(ReceivedMessage& message, const SharedNodePointer&
 }
 
 void NodeList::processPingPacket(QSharedPointer<ReceivedMessage> message, SharedNodePointer sendingNode) {
-    
+
     // send back a reply
     auto replyPacket = constructPingReplyPacket(*message);
     const HifiSockAddr& senderSockAddr = message->getSenderSockAddr();
@@ -252,6 +252,11 @@ void NodeList::reset() {
     _personalMutedNodeIDs.clear();
     _personalMutedSetLock.unlock();
 
+    // lock and clear out set of avatarGains
+    _avatarGainMapLock.lockForWrite();
+    _avatarGainMap.clear();
+    _avatarGainMapLock.unlock();
+
     // refresh the owner UUID to the NULL UUID
     setSessionUUID(QUuid());
 
@@ -329,7 +334,7 @@ void NodeList::sendDomainServerCheckIn() {
         }
 
         auto domainPacket = NLPacket::create(domainPacketType);
-        
+
         QDataStream packetStream(domainPacket.get());
 
         if (domainPacketType == PacketType::DomainConnectRequest) {
@@ -488,7 +493,7 @@ void NodeList::processDomainServerPathResponse(QSharedPointer<ReceivedMessage> m
         qCDebug(networking) << "Could not read query path from DomainServerPathQueryResponse. Bailing.";
         return;
     }
-    
+
     QString pathQuery = QString::fromUtf8(message->getRawMessage() + message->getPosition(), numPathBytes);
     message->seek(message->getPosition() + numPathBytes);
 
@@ -500,10 +505,10 @@ void NodeList::processDomainServerPathResponse(QSharedPointer<ReceivedMessage> m
         qCDebug(networking) << "Could not read resulting viewpoint from DomainServerPathQueryReponse. Bailing";
         return;
     }
-    
+
     // pull the viewpoint from the packet
     QString viewpoint = QString::fromUtf8(message->getRawMessage() + message->getPosition(), numViewpointBytes);
-    
+
     // Hand it off to the AddressManager so it can handle it as a relative viewpoint
     if (DependencyManager::get<AddressManager>()->goToViewpointForPath(viewpoint, pathQuery)) {
         qCDebug(networking) << "Going to viewpoint" << viewpoint << "which was the lookup result for path" << pathQuery;
@@ -664,16 +669,16 @@ void NodeList::parseNodeFromPacketStream(QDataStream& packetStream) {
 }
 
 void NodeList::sendAssignment(Assignment& assignment) {
- 
+
     PacketType assignmentPacketType = assignment.getCommand() == Assignment::CreateCommand
         ? PacketType::CreateAssignment
         : PacketType::RequestAssignment;
 
     auto assignmentPacket = NLPacket::create(assignmentPacketType);
-    
+
     QDataStream packetStream(assignmentPacket.get());
     packetStream << assignment;
-    
+
     sendPacket(std::move(assignmentPacket), _assignmentServerSocket);
 }
 
@@ -833,7 +838,7 @@ void NodeList::ignoreNodeBySessionID(const QUuid& nodeID, bool ignoreEnabled) {
                 _ignoredNodeIDs.insert(nodeID);
             }
             {
-                QReadLocker personalMutedSetLocker{ &_personalMutedSetLock }; // read lock for insert 
+                QReadLocker personalMutedSetLocker{ &_personalMutedSetLock }; // read lock for insert
                 // add this nodeID to our set of personal muted IDs
                 _personalMutedNodeIDs.insert(nodeID);
             }
@@ -896,7 +901,7 @@ void NodeList::personalMuteNodeBySessionID(const QUuid& nodeID, bool muteEnabled
 
 
                 if (muteEnabled) {
-                    QReadLocker personalMutedSetLocker{ &_personalMutedSetLock }; // read lock for insert 
+                    QReadLocker personalMutedSetLocker{ &_personalMutedSetLock }; // read lock for insert
                     // add this nodeID to our set of personal muted IDs
                     _personalMutedNodeIDs.insert(nodeID);
                 } else {
@@ -981,7 +986,7 @@ void NodeList::setAvatarGain(const QUuid& nodeID, float gain) {
         if (audioMixer) {
             // setup the packet
             auto setAvatarGainPacket = NLPacket::create(PacketType::PerAvatarGainSet, NUM_BYTES_RFC4122_UUID + sizeof(float), true);
-            
+
             // write the node ID to the packet
             setAvatarGainPacket->write(nodeID.toRfc4122());
             // We need to convert the gain in dB (from the script) to an amplitude before packing it.
@@ -990,6 +995,9 @@ void NodeList::setAvatarGain(const QUuid& nodeID, float gain) {
             qCDebug(networking) << "Sending Set Avatar Gain packet UUID: " << uuidStringWithoutCurlyBraces(nodeID) << "Gain:" << gain;
 
             sendPacket(std::move(setAvatarGainPacket), *audioMixer);
+            QWriteLocker{ &_avatarGainMapLock };
+            _avatarGainMap[nodeID] = gain;
+
         } else {
             qWarning() << "Couldn't find audio mixer to send set gain request";
         }
@@ -998,6 +1006,15 @@ void NodeList::setAvatarGain(const QUuid& nodeID, float gain) {
     }
 }
 
+float NodeList::getAvatarGain(const QUuid& nodeID) {
+    QReadLocker{ &_avatarGainMapLock };
+    auto it = _avatarGainMap.find(nodeID);
+    if (it != _avatarGainMap.cend()) {
+        return it->second;
+    }
+    return 0.0f;
+}
+
 void NodeList::kickNodeBySessionID(const QUuid& nodeID) {
     // send a request to domain-server to kick the node with the given session ID
     // the domain-server will handle the persistence of the kick (via username or IP)
@@ -1036,7 +1053,7 @@ void NodeList::muteNodeBySessionID(const QUuid& nodeID) {
                 mutePacket->write(nodeID.toRfc4122());
 
                 qCDebug(networking) << "Sending packet to mute node" << uuidStringWithoutCurlyBraces(nodeID);
-            
+
                 sendPacket(std::move(mutePacket), *audioMixer);
             } else {
                 qWarning() << "Couldn't find audio mixer to send node mute request";

libraries/networking/src/NodeList.h

@@ -68,7 +68,7 @@ public:
 
     void setAssignmentServerSocket(const HifiSockAddr& serverSocket) { _assignmentServerSocket = serverSocket; }
     void sendAssignment(Assignment& assignment);
-    
+
     void setIsShuttingDown(bool isShuttingDown) { _isShuttingDown = isShuttingDown; }
 
     void ignoreNodesInRadius(bool enabled = true);
@@ -83,6 +83,7 @@ public:
     void personalMuteNodeBySessionID(const QUuid& nodeID, bool muteEnabled);
     bool isPersonalMutingNode(const QUuid& nodeID) const;
     void setAvatarGain(const QUuid& nodeID, float gain);
+    float getAvatarGain(const QUuid& nodeID);
 
     void kickNodeBySessionID(const QUuid& nodeID);
     void muteNodeBySessionID(const QUuid& nodeID);
@@ -103,7 +104,7 @@ public slots:
     void processDomainServerPathResponse(QSharedPointer<ReceivedMessage> message);
 
     void processDomainServerConnectionTokenPacket(QSharedPointer<ReceivedMessage> message);
-    
+
     void processPingPacket(QSharedPointer<ReceivedMessage> message, SharedNodePointer sendingNode);
     void processPingReplyPacket(QSharedPointer<ReceivedMessage> message, SharedNodePointer sendingNode);
 
@@ -131,11 +132,11 @@ private slots:
     void handleNodePingTimeout();
 
     void pingPunchForDomainServer();
-    
+
     void sendKeepAlivePings();
 
     void maybeSendIgnoreSetToNode(SharedNodePointer node);
-    
+
 private:
     NodeList() : LimitedNodeList(INVALID_PORT, INVALID_PORT) { assert(false); } // Not implemented, needed for DependencyManager templates compile
     NodeList(char ownerType, int socketListenPort = INVALID_PORT, int dtlsListenPort = INVALID_PORT);
@@ -148,7 +149,7 @@ private:
     void timePingReply(ReceivedMessage& message, const SharedNodePointer& sendingNode);
 
     void sendDSPathQuery(const QString& newPath);
- 
+
     void parseNodeFromPacketStream(QDataStream& packetStream);
 
     void pingPunchForInactiveNode(const SharedNodePointer& node);
@@ -170,6 +171,8 @@ private:
     tbb::concurrent_unordered_set<QUuid, UUIDHasher> _ignoredNodeIDs;
     mutable QReadWriteLock _personalMutedSetLock;
     tbb::concurrent_unordered_set<QUuid, UUIDHasher> _personalMutedNodeIDs;
+    mutable QReadWriteLock _avatarGainMapLock;
+    tbb::concurrent_unordered_map<QUuid, float, UUIDHasher> _avatarGainMap;
 
     void sendIgnoreRadiusStateToNode(const SharedNodePointer& destinationNode);
     Setting::Handle<bool> _ignoreRadiusEnabled { "IgnoreRadiusEnabled", true };

libraries/script-engine/src/UsersScriptingInterface.cpp

@@ -47,6 +47,10 @@ void UsersScriptingInterface::setAvatarGain(const QUuid& nodeID, float gain) {
     DependencyManager::get<NodeList>()->setAvatarGain(nodeID, gain);
 }
 
+float UsersScriptingInterface::getAvatarGain(const QUuid& nodeID) {
+    return DependencyManager::get<NodeList>()->getAvatarGain(nodeID);
+}
+
 void UsersScriptingInterface::kick(const QUuid& nodeID) {
     // ask the NodeList to kick the user with the given session ID
     DependencyManager::get<NodeList>()->kickNodeBySessionID(nodeID);
@@ -88,4 +92,4 @@ bool UsersScriptingInterface::getRequestsDomainListData() {
 }
 void UsersScriptingInterface::setRequestsDomainListData(bool isRequesting) {
     DependencyManager::get<NodeList>()->setRequestsDomainListData(isRequesting);
-}
+}

libraries/script-engine/src/UsersScriptingInterface.h

@@ -70,6 +70,14 @@ public slots:
     */
     void setAvatarGain(const QUuid& nodeID, float gain);
 
+    /**jsdoc
+    * Gets an avatar's gain for you and you only.
+    * @function Users.getAvatarGain
+    * @param {nodeID} nodeID The node or session ID of the user whose gain you want to get.
+    * @return {float} gain (in dB)
+    */
+    float getAvatarGain(const QUuid& nodeID);
+
     /**jsdoc
     * Kick another user.
     * @function Users.kick

scripts/system/pal.js

@@ -245,18 +245,6 @@ function fromQml(message) { // messages are {method, params}, like json-rpc. See
         populateUserList(message.params.selected);
         UserActivityLogger.palAction("refresh", "");
         break;
-    case 'updateGain':
-        data = message.params;
-        if (data['isReleased']) {
-            // isReleased=true happens once at the end of a cycle of dragging
-            // the slider about, but with same gain as last isReleased=false so
-            // we don't set the gain in that case, and only here do we want to
-            // send an analytic event.
-            UserActivityLogger.palAction("avatar_gain_changed", data['sessionId']);
-        } else {
-            Users.setAvatarGain(data['sessionId'], data['gain']);
-        }
-        break;
     case 'displayNameUpdate':
         if (MyAvatar.displayName !== message.params) {
             MyAvatar.displayName = message.params;