From d0fb09a3bd2a792d8cef2717a4beff8b7808b3c4 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Thu, 31 Jan 2019 14:58:58 -0800
Subject: [PATCH 1/3] Assign lowest available suffix when display-names collide
---
assignment-client/src/avatars/AvatarMixer.cpp | 62 +++++++++++++++----
assignment-client/src/avatars/AvatarMixer.h | 20 +++++-
libraries/avatars/src/AvatarData.cpp | 9 ++-
3 files changed, 74 insertions(+), 17 deletions(-)
diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp
index 500772c1b5..f885b8110d 100644
--- a/assignment-client/src/avatars/AvatarMixer.cpp
+++ b/assignment-client/src/avatars/AvatarMixer.cpp
@@ -38,6 +38,19 @@ const QString AVATAR_MIXER_LOGGING_NAME = "avatar-mixer";
// FIXME - what we'd actually like to do is send to users at ~50% of their present rate down to 30hz. Assume 90 for now.
const int AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND = 45;
+const QRegularExpression AvatarMixer::suffixedNamePattern { R"(^\s*(.+)\s*_(\d)+\s*$)" };
+
+// Lexicographic comparison:
+bool AvatarMixer::SessionDisplayName::operator<(const SessionDisplayName& rhs) const {
+ if (_baseName < rhs._baseName) {
+ return true;
+ } else if (rhs._baseName < _baseName) {
+ return false;
+ } else {
+ return _suffix < rhs._suffix;
+ }
+}
+
AvatarMixer::AvatarMixer(ReceivedMessage& message) :
ThreadedAssignment(message),
_slavePool(&_slaveSharedData)
@@ -313,27 +326,40 @@ void AvatarMixer::manageIdentityData(const SharedNodePointer& node) {
bool sendIdentity = false;
if (nodeData && nodeData->getAvatarSessionDisplayNameMustChange()) {
AvatarData& avatar = nodeData->getAvatar();
- const QString& existingBaseDisplayName = nodeData->getBaseDisplayName();
- if (--_sessionDisplayNames[existingBaseDisplayName].second <= 0) {
- _sessionDisplayNames.remove(existingBaseDisplayName);
+ const QString& existingBaseDisplayName = nodeData->getAvatar().getSessionDisplayName();
+ if (!existingBaseDisplayName.isEmpty()) {
+ SessionDisplayName existingDisplayName { existingBaseDisplayName };
+
+ auto suffixMatch = suffixedNamePattern.match(existingBaseDisplayName);
+ if (suffixMatch.hasMatch()) {
+ existingDisplayName._baseName = suffixMatch.captured(1);
+ existingDisplayName._suffix = suffixMatch.captured(2).toInt();
+ }
+ _sessionDisplayNames.erase(existingDisplayName);
}
QString baseName = avatar.getDisplayName().trimmed();
const QRegularExpression curses { "fuck|shit|damn|cock|cunt" }; // POC. We may eventually want something much more elaborate (subscription?).
baseName = baseName.replace(curses, "*"); // Replace rather than remove, so that people have a clue that the person's a jerk.
- const QRegularExpression trailingDigits { "\\s*(_\\d+\\s*)?(\\s*\\n[^$]*)?$" }; // trailing whitespace "_123" and any subsequent lines
+ static const QRegularExpression trailingDigits { R"(\s*(_\d+\s*)?(\s*\n[^$]*)?$)" }; // trailing whitespace "_123" and any subsequent lines
baseName = baseName.remove(trailingDigits);
if (baseName.isEmpty()) {
baseName = "anonymous";
}
- QPair<int, int>& soFar = _sessionDisplayNames[baseName]; // Inserts and answers 0, 0 if not already present, which is what we want.
- int& highWater = soFar.first;
- nodeData->setBaseDisplayName(baseName);
- QString sessionDisplayName = (highWater > 0) ? baseName + "_" + QString::number(highWater) : baseName;
+ SessionDisplayName newDisplayName { baseName };
+ auto nameIter = _sessionDisplayNames.lower_bound(newDisplayName);
+ if (nameIter != _sessionDisplayNames.end() && nameIter->_baseName == baseName) {
+ // Existing instance(s) of name; find first free suffix
+ while (*nameIter == newDisplayName && ++newDisplayName._suffix && ++nameIter != _sessionDisplayNames.end())
+ ;
+ }
+
+ _sessionDisplayNames.insert(newDisplayName);
+ QString sessionDisplayName = (newDisplayName._suffix > 0) ? baseName + "_" + QString::number(newDisplayName._suffix) : baseName;
avatar.setSessionDisplayName(sessionDisplayName);
- highWater++;
- soFar.second++; // refcount
+ nodeData->setBaseDisplayName(baseName);
+
nodeData->flagIdentityChange();
nodeData->setAvatarSessionDisplayNameMustChange(false);
sendIdentity = true;
@@ -410,9 +436,19 @@ void AvatarMixer::handleAvatarKilled(SharedNodePointer avatarNode) {
QMutexLocker nodeDataLocker(&avatarNode->getLinkedData()->getMutex());
AvatarMixerClientData* nodeData = dynamic_cast<AvatarMixerClientData*>(avatarNode->getLinkedData());
const QString& baseDisplayName = nodeData->getBaseDisplayName();
- // No sense guarding against very rare case of a node with no entry, as this will work without the guard and do one less lookup in the common case.
- if (--_sessionDisplayNames[baseDisplayName].second <= 0) {
- _sessionDisplayNames.remove(baseDisplayName);
+ const QString& displayName = nodeData->getAvatar().getSessionDisplayName();
+ SessionDisplayName exitingDisplayName { displayName };
+
+ auto suffixMatch = suffixedNamePattern.match(displayName);
+ if (suffixMatch.hasMatch()) {
+ exitingDisplayName._baseName = suffixMatch.captured(1);
+ exitingDisplayName._suffix = suffixMatch.captured(2).toInt();
+ }
+ auto displayNameIter = _sessionDisplayNames.find(exitingDisplayName);
+ if (displayNameIter == _sessionDisplayNames.end()) {
+ qCDebug(avatars, "Exiting avatar displayname", displayName, "not found");
+ } else {
+ _sessionDisplayNames.erase(displayNameIter);
}
}
diff --git a/assignment-client/src/avatars/AvatarMixer.h b/assignment-client/src/avatars/AvatarMixer.h
index 764656a2d5..2992e19b8f 100644
--- a/assignment-client/src/avatars/AvatarMixer.h
+++ b/assignment-client/src/avatars/AvatarMixer.h
@@ -15,6 +15,7 @@
#ifndef hifi_AvatarMixer_h
#define hifi_AvatarMixer_h
+#include <set>
#include <shared/RateCounter.h>
#include <PortableHighResolutionClock.h>
@@ -88,7 +89,24 @@ private:
RateCounter<> _broadcastRate;
p_high_resolution_clock::time_point _lastDebugMessage;
- QHash<QString, QPair<int, int>> _sessionDisplayNames;
+
+ // Pair of basename + uniquifying integer suffix.
+ struct SessionDisplayName {
+ explicit SessionDisplayName(QString baseName = QString(), int suffix = 0) :
+ _baseName(baseName),
+ _suffix(suffix) { }
+ // Does lexicographic ordering:
+ bool operator<(const SessionDisplayName& rhs) const;
+ bool operator==(const SessionDisplayName& rhs) const {
+ return _baseName == rhs._baseName && _suffix == rhs._suffix;
+ }
+
+ QString _baseName;
+ int _suffix;
+ };
+ static const QRegularExpression suffixedNamePattern;
+
+ std::set<SessionDisplayName> _sessionDisplayNames;
quint64 _displayNameManagementElapsedTime { 0 }; // total time spent in broadcastAvatarData/display name management... since last stats window
quint64 _ignoreCalculationElapsedTime { 0 };
diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp
index 4e95774efb..c733cfa291 100755
--- a/libraries/avatars/src/AvatarData.cpp
+++ b/libraries/avatars/src/AvatarData.cpp
@@ -632,9 +632,11 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent
// include jointData if there is room for the most minimal section. i.e. no translations or rotations.
IF_AVATAR_SPACE(PACKET_HAS_JOINT_DATA, AvatarDataPacket::minJointDataSize(numJoints)) {
- // Allow for faux joints + translation bit-vector:
- const ptrdiff_t minSizeForJoint = sizeof(AvatarDataPacket::SixByteQuat)
- + jointBitVectorSize + AvatarDataPacket::FAUX_JOINTS_SIZE;
+ // Minimum space required for another rotation joint -
+ // size of joint + following translation bit-vector + translation scale + faux joints:
+ const ptrdiff_t minSizeForJoint = sizeof(AvatarDataPacket::SixByteQuat) + jointBitVectorSize +
+ sizeof(float) + AvatarDataPacket::FAUX_JOINTS_SIZE;
+
auto startSection = destinationBuffer;
// compute maxTranslationDimension before we send any joint data.
@@ -724,6 +726,7 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent
const JointData& data = joints[i];
const JointData& last = lastSentJointData[i];
+ // Note minSizeForJoint is conservative since there isn't a following bit-vector + scale.
if (packetEnd - destinationBuffer >= minSizeForJoint) {
if (!data.translationIsDefaultPose) {
if (sendAll || last.translationIsDefaultPose || (!cullSmallChanges && last.translation != data.translation)
From e4ffc93bc254a4fb52f68ef4770efa730926daee Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Thu, 31 Jan 2019 15:19:05 -0800
Subject: [PATCH 2/3] Revert a cherry-picked workaround accidently committed
This reverts commit 08b21109c1daac786a67490bfef658a359bf47bb.
Stupid git - I never explicitly added this to the branch!
---
libraries/avatars/src/AvatarData.cpp | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp
index c733cfa291..4e95774efb 100755
--- a/libraries/avatars/src/AvatarData.cpp
+++ b/libraries/avatars/src/AvatarData.cpp
@@ -632,11 +632,9 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent
// include jointData if there is room for the most minimal section. i.e. no translations or rotations.
IF_AVATAR_SPACE(PACKET_HAS_JOINT_DATA, AvatarDataPacket::minJointDataSize(numJoints)) {
- // Minimum space required for another rotation joint -
- // size of joint + following translation bit-vector + translation scale + faux joints:
- const ptrdiff_t minSizeForJoint = sizeof(AvatarDataPacket::SixByteQuat) + jointBitVectorSize +
- sizeof(float) + AvatarDataPacket::FAUX_JOINTS_SIZE;
-
+ // Allow for faux joints + translation bit-vector:
+ const ptrdiff_t minSizeForJoint = sizeof(AvatarDataPacket::SixByteQuat)
+ + jointBitVectorSize + AvatarDataPacket::FAUX_JOINTS_SIZE;
auto startSection = destinationBuffer;
// compute maxTranslationDimension before we send any joint data.
@@ -726,7 +724,6 @@ QByteArray AvatarData::toByteArray(AvatarDataDetail dataDetail, quint64 lastSent
const JointData& data = joints[i];
const JointData& last = lastSentJointData[i];
- // Note minSizeForJoint is conservative since there isn't a following bit-vector + scale.
if (packetEnd - destinationBuffer >= minSizeForJoint) {
if (!data.translationIsDefaultPose) {
if (sendAll || last.translationIsDefaultPose || (!cullSmallChanges && last.translation != data.translation)
From 322030fccdf481c524c80da328761a350955d6f3 Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Thu, 31 Jan 2019 16:07:26 -0800
Subject: [PATCH 3/3] Remove unused variable; fix use of qCDebug
---
assignment-client/src/avatars/AvatarMixer.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp
index f885b8110d..801f28c6f5 100644
--- a/assignment-client/src/avatars/AvatarMixer.cpp
+++ b/assignment-client/src/avatars/AvatarMixer.cpp
@@ -435,7 +435,6 @@ void AvatarMixer::handleAvatarKilled(SharedNodePointer avatarNode) {
{ // decrement sessionDisplayNames table and possibly remove
QMutexLocker nodeDataLocker(&avatarNode->getLinkedData()->getMutex());
AvatarMixerClientData* nodeData = dynamic_cast<AvatarMixerClientData*>(avatarNode->getLinkedData());
- const QString& baseDisplayName = nodeData->getBaseDisplayName();
const QString& displayName = nodeData->getAvatar().getSessionDisplayName();
SessionDisplayName exitingDisplayName { displayName };
@@ -446,7 +445,7 @@ void AvatarMixer::handleAvatarKilled(SharedNodePointer avatarNode) {
}
auto displayNameIter = _sessionDisplayNames.find(exitingDisplayName);
if (displayNameIter == _sessionDisplayNames.end()) {
- qCDebug(avatars, "Exiting avatar displayname", displayName, "not found");
+ qCDebug(avatars) << "Exiting avatar displayname" << displayName << "not found";
} else {
_sessionDisplayNames.erase(displayNameIter);
}