From c10b0389de7ebe6f50f3d9994164d968b9abd201 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Tue, 28 Feb 2017 14:45:34 -0700 Subject: [PATCH 1/3] Add a getAvatarGain method to NodeList and UsersScriptingInterface --- libraries/networking/src/NodeList.cpp | 50 ++++++++++++------- libraries/networking/src/NodeList.h | 13 +++-- .../src/UsersScriptingInterface.cpp | 6 ++- .../src/UsersScriptingInterface.h | 8 +++ 4 files changed, 52 insertions(+), 25 deletions(-) diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index f4a02ad805..7bd6800921 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -49,7 +49,7 @@ NodeList::NodeList(char newOwnerType, int socketListenPort, int dtlsListenPort) setCustomDeleter([](Dependency* dependency){ static_cast(dependency)->deleteLater(); }); - + auto addressManager = DependencyManager::get(); // 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(); - - // 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 message, SharedNodePointer sendingNode) { - + // send back a reply auto replyPacket = constructPingReplyPacket(*message); const HifiSockAddr& senderSockAddr = message->getSenderSockAddr(); @@ -329,7 +329,7 @@ void NodeList::sendDomainServerCheckIn() { } auto domainPacket = NLPacket::create(domainPacketType); - + QDataStream packetStream(domainPacket.get()); if (domainPacketType == PacketType::DomainConnectRequest) { @@ -488,7 +488,7 @@ void NodeList::processDomainServerPathResponse(QSharedPointer 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 +500,10 @@ void NodeList::processDomainServerPathResponse(QSharedPointer 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()->goToViewpointForPath(viewpoint, pathQuery)) { qCDebug(networking) << "Going to viewpoint" << viewpoint << "which was the lookup result for path" << pathQuery; @@ -664,16 +664,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 +833,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 +896,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 +981,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 +990,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 +1001,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 +1048,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"; diff --git a/libraries/networking/src/NodeList.h b/libraries/networking/src/NodeList.h index 0e0a2fd6c8..293b0942d6 100644 --- a/libraries/networking/src/NodeList.h +++ b/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 message); void processDomainServerConnectionTokenPacket(QSharedPointer message); - + void processPingPacket(QSharedPointer message, SharedNodePointer sendingNode); void processPingReplyPacket(QSharedPointer 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 _ignoredNodeIDs; mutable QReadWriteLock _personalMutedSetLock; tbb::concurrent_unordered_set _personalMutedNodeIDs; + mutable QReadWriteLock _avatarGainMapLock; + tbb::concurrent_unordered_map _avatarGainMap; void sendIgnoreRadiusStateToNode(const SharedNodePointer& destinationNode); Setting::Handle _ignoreRadiusEnabled { "IgnoreRadiusEnabled", true }; diff --git a/libraries/script-engine/src/UsersScriptingInterface.cpp b/libraries/script-engine/src/UsersScriptingInterface.cpp index 3a3225ec75..6dc3188b3f 100644 --- a/libraries/script-engine/src/UsersScriptingInterface.cpp +++ b/libraries/script-engine/src/UsersScriptingInterface.cpp @@ -47,6 +47,10 @@ void UsersScriptingInterface::setAvatarGain(const QUuid& nodeID, float gain) { DependencyManager::get()->setAvatarGain(nodeID, gain); } +float UsersScriptingInterface::getAvatarGain(const QUuid& nodeID) { + return DependencyManager::get()->getAvatarGain(nodeID); +} + void UsersScriptingInterface::kick(const QUuid& nodeID) { // ask the NodeList to kick the user with the given session ID DependencyManager::get()->kickNodeBySessionID(nodeID); @@ -88,4 +92,4 @@ bool UsersScriptingInterface::getRequestsDomainListData() { } void UsersScriptingInterface::setRequestsDomainListData(bool isRequesting) { DependencyManager::get()->setRequestsDomainListData(isRequesting); -} \ No newline at end of file +} diff --git a/libraries/script-engine/src/UsersScriptingInterface.h b/libraries/script-engine/src/UsersScriptingInterface.h index 608fa937c8..acaa92d9c8 100644 --- a/libraries/script-engine/src/UsersScriptingInterface.h +++ b/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 From 678020fed6d1ff95e8b8e06d0a331942865e6cfd Mon Sep 17 00:00:00 2001 From: David Kelly Date: Tue, 28 Feb 2017 15:03:45 -0700 Subject: [PATCH 2/3] now use it (plus a bit of cleanup) --- interface/resources/qml/hifi/NameCard.qml | 6 +++--- interface/resources/qml/hifi/Pal.qml | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/interface/resources/qml/hifi/NameCard.qml b/interface/resources/qml/hifi/NameCard.qml index b55b9c517d..eab9965141 100644 --- a/interface/resources/qml/hifi/NameCard.qml +++ b/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: pal.gainSliderValueDB[uuid] ? pal.gainSliderValueDB[uuid] : 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 } diff --git a/interface/resources/qml/hifi/Pal.qml b/interface/resources/qml/hifi/Pal.qml index cf5ea98b81..a613af9593 100644 --- a/interface/resources/qml/hifi/Pal.qml +++ b/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. @@ -270,7 +270,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 +306,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 +538,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,7 +554,7 @@ Rectangle { } } break; - case 'clearLocalQMLData': + case 'clearLocalQMLData': ignored = {}; gainSliderValueDB = {}; break; From efe44425a63cc54fd27c02742b19bd4e5b4c93a4 Mon Sep 17 00:00:00 2001 From: David Kelly Date: Wed, 1 Mar 2017 09:26:03 -0700 Subject: [PATCH 3/3] cr feedback, plus something I missed --- interface/resources/qml/hifi/NameCard.qml | 13 ++++--------- interface/resources/qml/hifi/Pal.qml | 4 ---- libraries/networking/src/NodeList.cpp | 5 +++++ scripts/system/pal.js | 12 ------------ 4 files changed, 9 insertions(+), 25 deletions(-) diff --git a/interface/resources/qml/hifi/NameCard.qml b/interface/resources/qml/hifi/NameCard.qml index eab9965141..846f1bec3c 100644 --- a/interface/resources/qml/hifi/NameCard.qml +++ b/interface/resources/qml/hifi/NameCard.qml @@ -345,7 +345,7 @@ Item { anchors.verticalCenter: nameCardVUMeter.verticalCenter // Properties visible: !isMyCard && selected - value: pal.gainSliderValueDB[uuid] ? pal.gainSliderValueDB[uuid] : Users.getAvatarGain(uuid) + value: Users.getAvatarGain(uuid) minimumValue: -60.0 maximumValue: 20.0 stepSize: 5 @@ -393,14 +393,9 @@ Item { } function updateGainFromQML(avatarUuid, sliderValue, isReleased) { - if (isReleased || pal.gainSliderValueDB[avatarUuid] !== sliderValue) { - pal.gainSliderValueDB[avatarUuid] = sliderValue; - var data = { - sessionId: avatarUuid, - gain: sliderValue, - isReleased: isReleased - }; - pal.sendToScript({method: 'updateGain', params: data}); + Users.setAvatarGain(avatarUuid, sliderValue); + if (isReleased) { + UserActivityLogger.palAction("avatar_gain_changed", avatarUuid); } } } diff --git a/interface/resources/qml/hifi/Pal.qml b/interface/resources/qml/hifi/Pal.qml index a613af9593..7ff4e8a4b1 100644 --- a/interface/resources/qml/hifi/Pal.qml +++ b/interface/resources/qml/hifi/Pal.qml @@ -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 } @@ -556,7 +553,6 @@ Rectangle { break; case 'clearLocalQMLData': ignored = {}; - gainSliderValueDB = {}; break; case 'avatarDisconnected': var sessionID = message.params[0]; diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 7bd6800921..7147682d48 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -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()); diff --git a/scripts/system/pal.js b/scripts/system/pal.js index 9df4b2df92..106f226a33 100644 --- a/scripts/system/pal.js +++ b/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;