From 5a8ecdffabbe80e12480a4eaf707e19432db0fea Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 28 Feb 2019 17:37:56 -0800 Subject: [PATCH 1/7] EntityStats can have embedded EntityData --- assignment-client/src/avatars/AvatarMixer.cpp | 13 +++++++++++++ .../src/avatars/AvatarMixerClientData.cpp | 6 +++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 67bc9b4cf7..6384ad2b40 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -976,7 +976,20 @@ void AvatarMixer::handleOctreePacket(QSharedPointer message, Sh switch (packetType) { case PacketType::OctreeStats: + { // Ignore stats, but may have a different Entity packet appended. + OctreeHeadlessViewer::parseOctreeStats(message, senderNode); + const auto piggyBackedSizeWithHeader = message->getBytesLeftToRead(); + if (piggyBackedSizeWithHeader > 0) { + // pull out the piggybacked packet and create a new QSharedPointer for it + auto buffer = std::unique_ptr(new char[piggyBackedSizeWithHeader]); + memcpy(buffer.get(), message->getRawMessage() + message->getPosition(), piggyBackedSizeWithHeader); + + auto newPacket = NLPacket::fromReceivedPacket(std::move(buffer), piggyBackedSizeWithHeader, message->getSenderSockAddr()); + auto newMessage = QSharedPointer::create(*newPacket); + handleOctreePacket(newMessage, senderNode); + } break; + } case PacketType::EntityData: _entityViewer.processDatagram(*message, senderNode); diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index e24d48a9ed..a63a76829b 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -145,9 +145,9 @@ int AvatarMixerClientData::parseData(ReceivedMessage& message, const SlaveShared FindPriorityZone findPriorityZone { newPosition, false } ; entityTree.recurseTreeWithOperation(&FindPriorityZone::operation, &findPriorityZone); _avatar->setPriorityAvatar(findPriorityZone.isInPriorityZone); - //if (findPriorityZone.isInPriorityZone) { - // qCWarning(avatars) << "Avatar" << _avatar->getSessionDisplayName() << "in hero zone"; - //} + if (findPriorityZone.isInPriorityZone) { + qCWarning(avatars) << "Avatar" << _avatar->getSessionDisplayName() << "in hero zone"; + } #endif } From 64a2025bcdf2c427484edf6d27788092011f0418 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Thu, 28 Feb 2019 18:40:53 -0800 Subject: [PATCH 2/7] Renaming in main loop to make source/destination clear Also comment clean-up, etc --- .../src/avatars/AvatarMixerSlave.cpp | 123 +++++++++--------- 1 file changed, 62 insertions(+), 61 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index a38553ddeb..ac18268860 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -320,21 +320,21 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) _stats.nodesBroadcastedTo++; - AvatarMixerClientData* nodeData = reinterpret_cast(destinationNode->getLinkedData()); + AvatarMixerClientData* destinationNodeData = reinterpret_cast(destinationNode->getLinkedData()); - nodeData->resetInViewStats(); + destinationNodeData->resetInViewStats(); - const AvatarData& avatar = nodeData->getAvatar(); + const AvatarData& avatar = destinationNodeData->getAvatar(); glm::vec3 myPosition = avatar.getClientGlobalPosition(); // reset the internal state for correct random number distribution distribution.reset(); // Estimate number to sort on number sent last frame (with min. of 20). - const int numToSendEst = std::max(int(nodeData->getNumAvatarsSentLastFrame() * 2.5f), 20); + const int numToSendEst = std::max(int(destinationNodeData->getNumAvatarsSentLastFrame() * 2.5f), 20); // reset the number of sent avatars - nodeData->resetNumAvatarsSentLastFrame(); + destinationNodeData->resetNumAvatarsSentLastFrame(); // keep track of outbound data rate specifically for avatar data int numAvatarDataBytes = 0; @@ -353,8 +353,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // When this is true, the AvatarMixer will send Avatar data to a client // about avatars they've ignored or that are out of view - bool PALIsOpen = nodeData->getRequestsDomainListData(); - bool PALWasOpen = nodeData->getPrevRequestsDomainListData(); + bool PALIsOpen = destinationNodeData->getRequestsDomainListData(); + bool PALWasOpen = destinationNodeData->getPrevRequestsDomainListData(); // When this is true, the AvatarMixer will send Avatar data to a client about avatars that have ignored them bool getsAnyIgnored = PALIsOpen && destinationNode->getCanKick(); @@ -368,7 +368,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) AABox nodeBox = computeBubbleBox(avatar, MY_AVATAR_BUBBLE_EXPANSION_FACTOR); // prepare to sort - const auto& cameraViews = nodeData->getViewFrustums(); + const auto& cameraViews = destinationNodeData->getViewFrustums(); using AvatarPriorityQueue = PrioritySortUtil::PriorityQueue; // Keep two independent queues, one for heroes and one for the riff-raff. @@ -391,47 +391,47 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) continue; } - auto avatarNode = otherNodeRaw; + auto sourceAvatarNode = otherNodeRaw; - bool shouldIgnore = false; + bool sendAvatar = true; // We will consider this source avatar for sending. // We ignore other nodes for a couple of reasons: // 1) ignore bubbles and ignore specific node // 2) the node hasn't really updated it's frame data recently, this can // happen if for example the avatar is connected on a desktop and sending // updates at ~30hz. So every 3 frames we skip a frame. - assert(avatarNode); // we can't have gotten here without the avatarData being a valid key in the map + assert(sourceAvatarNode); // we can't have gotten here without the avatarData being a valid key in the map - const AvatarMixerClientData* avatarClientNodeData = reinterpret_cast(avatarNode->getLinkedData()); - assert(avatarClientNodeData); // we can't have gotten here without avatarNode having valid data + const AvatarMixerClientData* sourceAvatarNodeData = reinterpret_cast(sourceAvatarNode->getLinkedData()); + assert(sourceAvatarNodeData); // we can't have gotten here without sourceAvatarNode having valid data quint64 startIgnoreCalculation = usecTimestampNow(); // make sure we have data for this avatar, that it isn't the same node, // and isn't an avatar that the viewing node has ignored // or that has ignored the viewing node - if ((destinationNode->isIgnoringNodeWithID(avatarNode->getUUID()) && !PALIsOpen) - || (avatarNode->isIgnoringNodeWithID(destinationNode->getUUID()) && !getsAnyIgnored)) { - shouldIgnore = true; + if ((destinationNode->isIgnoringNodeWithID(sourceAvatarNode->getUUID()) && !PALIsOpen) + || (sourceAvatarNode->isIgnoringNodeWithID(destinationNode->getUUID()) && !getsAnyIgnored)) { + sendAvatar = false; } else { // Check to see if the space bubble is enabled // Don't bother with these checks if the other avatar has their bubble enabled and we're gettingAnyIgnored - if (nodeData->isIgnoreRadiusEnabled() || (avatarClientNodeData->isIgnoreRadiusEnabled() && !getsAnyIgnored)) { + if (destinationNodeData->isIgnoreRadiusEnabled() || (sourceAvatarNodeData->isIgnoreRadiusEnabled() && !getsAnyIgnored)) { // Perform the collision check between the two bounding boxes - AABox otherNodeBox = avatarClientNodeData->getAvatar().getDefaultBubbleBox(); + AABox otherNodeBox = sourceAvatarNodeData->getAvatar().getDefaultBubbleBox(); if (nodeBox.touches(otherNodeBox)) { - nodeData->ignoreOther(destinationNode, avatarNode); - shouldIgnore = !getsAnyIgnored; + destinationNodeData->ignoreOther(destinationNode, sourceAvatarNode); + sendAvatar = getsAnyIgnored; } } // Not close enough to ignore - if (!shouldIgnore) { - nodeData->removeFromRadiusIgnoringSet(avatarNode->getUUID()); + if (sendAvatar) { + destinationNodeData->removeFromRadiusIgnoringSet(sourceAvatarNode->getUUID()); } } - if (!shouldIgnore) { - AvatarDataSequenceNumber lastSeqToReceiver = nodeData->getLastBroadcastSequenceNumber(avatarNode->getLocalID()); - AvatarDataSequenceNumber lastSeqFromSender = avatarClientNodeData->getLastReceivedSequenceNumber(); + if (sendAvatar) { + AvatarDataSequenceNumber lastSeqToReceiver = destinationNodeData->getLastBroadcastSequenceNumber(sourceAvatarNode->getLocalID()); + AvatarDataSequenceNumber lastSeqFromSender = sourceAvatarNodeData->getLastReceivedSequenceNumber(); // FIXME - This code does appear to be working. But it seems brittle. // It supports determining if the frame of data for this "other" @@ -445,12 +445,12 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // or that somehow we haven't sent if (lastSeqToReceiver == lastSeqFromSender && lastSeqToReceiver != 0) { ++numAvatarsHeldBack; - shouldIgnore = true; + sendAvatar = false; } else if (lastSeqFromSender == 0) { - // We have have not yet recieved any data about this avatar. Ignore it for now + // We have have not yet received any data about this avatar. Ignore it for now // This is important for Agent scripts that are not avatar // so that they don't appear to be an avatar at the origin - shouldIgnore = true; + sendAvatar = false; } else if (lastSeqFromSender - lastSeqToReceiver > 1) { // this is a skip - we still send the packet but capture the presence of the skip so we see it happening ++numAvatarsWithSkippedFrames; @@ -459,13 +459,13 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) quint64 endIgnoreCalculation = usecTimestampNow(); _stats.ignoreCalculationElapsedTime += (endIgnoreCalculation - startIgnoreCalculation); - if (!shouldIgnore) { + if (sendAvatar) { // sort this one for later - const MixerAvatar* avatarNodeData = avatarClientNodeData->getConstAvatarData(); - auto lastEncodeTime = nodeData->getLastOtherAvatarEncodeTime(avatarNode->getLocalID()); + const MixerAvatar* avatarNodeData = sourceAvatarNodeData->getConstAvatarData(); + auto lastEncodeTime = destinationNodeData->getLastOtherAvatarEncodeTime(sourceAvatarNode->getLocalID()); avatarPriorityQueues[avatarNodeData->getPriorityAvatar() ? kHero : kNonhero].push( - SortableAvatar(avatarNodeData, avatarNode, lastEncodeTime)); + SortableAvatar(avatarNodeData, sourceAvatarNode, lastEncodeTime)); } // If Avatar A's PAL WAS open but is no longer open, AND @@ -475,18 +475,18 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // will be sent when it doesn't need to be (but where it _should_ be OK to send). // However, it's less heavy-handed than using `shouldIgnore`. if (PALWasOpen && !PALIsOpen && - (destinationNode->isIgnoringNodeWithID(avatarNode->getUUID()) || - avatarNode->isIgnoringNodeWithID(destinationNode->getUUID()))) { + (destinationNode->isIgnoringNodeWithID(sourceAvatarNode->getUUID()) || + sourceAvatarNode->isIgnoringNodeWithID(destinationNode->getUUID()))) { // ...send a Kill Packet to Node A, instructing Node A to kill Avatar B, // then have Node A cleanup the killed Node B. auto packet = NLPacket::create(PacketType::KillAvatar, NUM_BYTES_RFC4122_UUID + sizeof(KillAvatarReason), true); - packet->write(avatarNode->getUUID().toRfc4122()); + packet->write(sourceAvatarNode->getUUID().toRfc4122()); packet->writePrimitive(KillAvatarReason::AvatarIgnored); nodeList->sendPacket(std::move(packet), *destinationNode); - nodeData->cleanupKilledNode(avatarNode->getUUID(), avatarNode->getLocalID()); + destinationNodeData->cleanupKilledNode(sourceAvatarNode->getUUID(), sourceAvatarNode->getLocalID()); } - nodeData->setPrevRequestsDomainListData(PALIsOpen); + destinationNodeData->setPrevRequestsDomainListData(PALIsOpen); } // loop through our sorted avatars and allocate our bandwidth to them accordingly @@ -501,13 +501,14 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) int numAvatarsSent = 0; auto identityPacketList = NLPacketList::create(PacketType::AvatarIdentity, QByteArray(), true, true); + // Loop over two priorities - hero avatars then everyone else: for (PriorityVariants currentVariant = kHero; currentVariant <= kNonhero; ++((int&)currentVariant)) { const auto& sortedAvatarVector = avatarPriorityQueues[currentVariant].getSortedVector(numToSendEst); for (const auto& sortedAvatar : sortedAvatarVector) { - const Node* otherNode = sortedAvatar.getNode(); + const Node* sourceNode = sortedAvatar.getNode(); auto lastEncodeForOther = sortedAvatar.getTimestamp(); - assert(otherNode); // we can't have gotten here without the avatarData being a valid key in the map + assert(sourceNode); // we can't have gotten here without the avatarData being a valid key in the map AvatarData::AvatarDataDetail detail = AvatarData::NoData; @@ -533,31 +534,31 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) auto startAvatarDataPacking = chrono::high_resolution_clock::now(); - const AvatarMixerClientData* otherNodeData = reinterpret_cast(otherNode->getLinkedData()); - const MixerAvatar* otherAvatar = otherNodeData->getConstAvatarData(); + const AvatarMixerClientData* sourceNodeData = reinterpret_cast(sourceNode->getLinkedData()); + const MixerAvatar* sourceAvatar = sourceNodeData->getConstAvatarData(); // Typically all out-of-view avatars but such avatars' priorities will rise with time: - bool isLowerPriority = sortedAvatar.getPriority() <= OUT_OF_VIEW_THRESHOLD; + bool isLowerPriority = sortedAvatar.getPriority() <= OUT_OF_VIEW_THRESHOLD; // XXX: hero handling? if (isLowerPriority) { detail = PALIsOpen ? AvatarData::PALMinimum : AvatarData::MinimumData; - nodeData->incrementAvatarOutOfView(); + destinationNodeData->incrementAvatarOutOfView(); } else if (!overBudget) { detail = distribution(generator) < AVATAR_SEND_FULL_UPDATE_RATIO ? AvatarData::SendAllData : AvatarData::CullSmallData; - nodeData->incrementAvatarInView(); + destinationNodeData->incrementAvatarInView(); // If the time that the mixer sent AVATAR DATA about Avatar B to Avatar A is BEFORE OR EQUAL TO // the time that Avatar B flagged an IDENTITY DATA change, send IDENTITY DATA about Avatar B to Avatar A. - if (otherAvatar->hasProcessedFirstIdentity() - && nodeData->getLastBroadcastTime(otherNode->getLocalID()) <= otherNodeData->getIdentityChangeTimestamp()) { - identityBytesSent += sendIdentityPacket(*identityPacketList, otherNodeData, *destinationNode); + if (sourceAvatar->hasProcessedFirstIdentity() + && destinationNodeData->getLastBroadcastTime(sourceNode->getLocalID()) <= sourceNodeData->getIdentityChangeTimestamp()) { + identityBytesSent += sendIdentityPacket(*identityPacketList, sourceNodeData, *destinationNode); // remember the last time we sent identity details about this other node to the receiver - nodeData->setLastBroadcastTime(otherNode->getLocalID(), usecTimestampNow()); + destinationNodeData->setLastBroadcastTime(sourceNode->getLocalID(), usecTimestampNow()); } } - QVector& lastSentJointsForOther = nodeData->getLastOtherAvatarSentJoints(otherNode->getLocalID()); + QVector& lastSentJointsForOther = destinationNodeData->getLastOtherAvatarSentJoints(sourceNode->getLocalID()); const bool distanceAdjust = true; const bool dropFaceTracking = false; @@ -566,7 +567,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) do { auto startSerialize = chrono::high_resolution_clock::now(); - QByteArray bytes = otherAvatar->toByteArray(detail, lastEncodeForOther, lastSentJointsForOther, + QByteArray bytes = sourceAvatar->toByteArray(detail, lastEncodeForOther, lastSentJointsForOther, sendStatus, dropFaceTracking, distanceAdjust, myPosition, &lastSentJointsForOther, avatarSpaceAvailable); auto endSerialize = chrono::high_resolution_clock::now(); @@ -587,17 +588,17 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) if (detail != AvatarData::NoData) { _stats.numOthersIncluded++; - if (otherAvatar->getPriorityAvatar()) { + if (sourceAvatar->getPriorityAvatar()) { _stats.numHeroesIncluded++; } // increment the number of avatars sent to this receiver - nodeData->incrementNumAvatarsSentLastFrame(); + destinationNodeData->incrementNumAvatarsSentLastFrame(); // set the last sent sequence number for this sender on the receiver - nodeData->setLastBroadcastSequenceNumber(otherNode->getLocalID(), - otherNodeData->getLastReceivedSequenceNumber()); - nodeData->setLastOtherAvatarEncodeTime(otherNode->getLocalID(), usecTimestampNow()); + destinationNodeData->setLastBroadcastSequenceNumber(sourceNode->getLocalID(), + sourceNodeData->getLastReceivedSequenceNumber()); + destinationNodeData->setLastOtherAvatarEncodeTime(sourceNode->getLocalID(), usecTimestampNow()); } auto endAvatarDataPacking = chrono::high_resolution_clock::now(); @@ -606,7 +607,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) if (!overBudget) { // use helper to add any changed traits to our packet list - traitBytesSent += addChangedTraitsToBulkPacket(nodeData, otherNodeData, *traitsPacketList); + traitBytesSent += addChangedTraitsToBulkPacket(destinationNodeData, sourceNodeData, *traitsPacketList); } numAvatarsSent++; remainingAvatars--; @@ -619,8 +620,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) } } - if (nodeData->getNumAvatarsSentLastFrame() > numToSendEst) { - qCWarning(avatars) << "More avatars sent than upper estimate" << nodeData->getNumAvatarsSentLastFrame() + if (destinationNodeData->getNumAvatarsSentLastFrame() > numToSendEst) { + qCWarning(avatars) << "More avatars sent than upper estimate" << destinationNodeData->getNumAvatarsSentLastFrame() << " / " << numToSendEst; } @@ -651,12 +652,12 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) } // record the bytes sent for other avatar data in the AvatarMixerClientData - nodeData->recordSentAvatarData(numAvatarDataBytes, traitBytesSent); + destinationNodeData->recordSentAvatarData(numAvatarDataBytes, traitBytesSent); // record the number of avatars held back this frame - nodeData->recordNumOtherAvatarStarves(numAvatarsHeldBack); - nodeData->recordNumOtherAvatarSkips(numAvatarsWithSkippedFrames); + destinationNodeData->recordNumOtherAvatarStarves(numAvatarsHeldBack); + destinationNodeData->recordNumOtherAvatarSkips(numAvatarsWithSkippedFrames); quint64 endPacketSending = usecTimestampNow(); _stats.packetSendingElapsedTime += (endPacketSending - startPacketSending); From d391d4af389ca10f3211dbbe0772fc6d517dbe67 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 1 Mar 2019 10:08:22 -0800 Subject: [PATCH 3/7] Debugging + tweaks --- assignment-client/src/avatars/AvatarMixerSlave.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index ac18268860..d57d3a5011 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -522,6 +522,9 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) _stats.overBudgetAvatars++; detail = AvatarData::PALMinimum; } else { + if (currentVariant == kHero) { + qCWarning(avatars) << "Overbudget break with hero avatars!" << destinationNode->getUUID().toString(); + } _stats.overBudgetAvatars += remainingAvatars; break; } @@ -538,7 +541,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) const MixerAvatar* sourceAvatar = sourceNodeData->getConstAvatarData(); // Typically all out-of-view avatars but such avatars' priorities will rise with time: - bool isLowerPriority = sortedAvatar.getPriority() <= OUT_OF_VIEW_THRESHOLD; // XXX: hero handling? + bool isLowerPriority = currentVariant != kHero && sortedAvatar.getPriority() <= OUT_OF_VIEW_THRESHOLD; // XXX: hero handling? if (isLowerPriority) { detail = PALIsOpen ? AvatarData::PALMinimum : AvatarData::MinimumData; From 3cc932db96d6f2f37d3f8d3774a23b68351489a9 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 1 Mar 2019 12:43:39 -0800 Subject: [PATCH 4/7] More debugging for hero issues --- .../src/avatars/AvatarMixerSlave.cpp | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index d57d3a5011..54eefadda1 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -325,7 +325,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) destinationNodeData->resetInViewStats(); const AvatarData& avatar = destinationNodeData->getAvatar(); - glm::vec3 myPosition = avatar.getClientGlobalPosition(); + glm::vec3 destinationPosition = avatar.getClientGlobalPosition(); // reset the internal state for correct random number distribution distribution.reset(); @@ -355,6 +355,9 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // about avatars they've ignored or that are out of view bool PALIsOpen = destinationNodeData->getRequestsDomainListData(); bool PALWasOpen = destinationNodeData->getPrevRequestsDomainListData(); + if (PALIsOpen) { + qCWarning(avatars) << "PAL is open:" << avatar.getSessionDisplayName(); + } // When this is true, the AvatarMixer will send Avatar data to a client about avatars that have ignored them bool getsAnyIgnored = PALIsOpen && destinationNode->getCanKick(); @@ -365,7 +368,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // compute node bounding box const float MY_AVATAR_BUBBLE_EXPANSION_FACTOR = 4.0f; // magic number determined emperically - AABox nodeBox = computeBubbleBox(avatar, MY_AVATAR_BUBBLE_EXPANSION_FACTOR); + AABox destinationNodeBox = computeBubbleBox(avatar, MY_AVATAR_BUBBLE_EXPANSION_FACTOR); // prepare to sort const auto& cameraViews = destinationNodeData->getViewFrustums(); @@ -417,8 +420,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // Don't bother with these checks if the other avatar has their bubble enabled and we're gettingAnyIgnored if (destinationNodeData->isIgnoreRadiusEnabled() || (sourceAvatarNodeData->isIgnoreRadiusEnabled() && !getsAnyIgnored)) { // Perform the collision check between the two bounding boxes - AABox otherNodeBox = sourceAvatarNodeData->getAvatar().getDefaultBubbleBox(); - if (nodeBox.touches(otherNodeBox)) { + AABox sourceNodeBox = sourceAvatarNodeData->getAvatar().getDefaultBubbleBox(); + if (destinationNodeBox.touches(sourceNodeBox)) { destinationNodeData->ignoreOther(destinationNode, sourceAvatarNode); sendAvatar = getsAnyIgnored; } @@ -456,6 +459,13 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) ++numAvatarsWithSkippedFrames; } } + { + if (sourceAvatarNodeData->getConstAvatarData()->getPriorityAvatar() && !sendAvatar) { + qCWarning(avatars) << "Hero avatar dropped:" << sourceAvatarNodeData->getConstAvatarData()->getSessionDisplayName() + << "lastSeqToReceiver =" << destinationNodeData->getLastBroadcastSequenceNumber(sourceAvatarNode->getLocalID()) + << "lastSeqFromSender = " << sourceAvatarNodeData->getLastReceivedSequenceNumber(); + } + } quint64 endIgnoreCalculation = usecTimestampNow(); _stats.ignoreCalculationElapsedTime += (endIgnoreCalculation - startIgnoreCalculation); @@ -571,7 +581,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) do { auto startSerialize = chrono::high_resolution_clock::now(); QByteArray bytes = sourceAvatar->toByteArray(detail, lastEncodeForOther, lastSentJointsForOther, - sendStatus, dropFaceTracking, distanceAdjust, myPosition, + sendStatus, dropFaceTracking, distanceAdjust, destinationPosition, &lastSentJointsForOther, avatarSpaceAvailable); auto endSerialize = chrono::high_resolution_clock::now(); _stats.toByteArrayElapsedTime += From 83c9381575eafe3aa43fcfbbf60249506359c6f7 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 1 Mar 2019 17:25:46 -0800 Subject: [PATCH 5/7] Convert avatarPriority to trivalued (inherit, crowd, hero) Also tweaks from original reviewer comments. --- .../src/avatars/AvatarMixerClientData.cpp | 28 ++++++----- .../src/avatars/AvatarMixerSlave.cpp | 6 +-- assignment-client/src/avatars/MixerAvatar.h | 6 +-- .../entities/src/EntityItemProperties.cpp | 46 +++++++++++++------ libraries/entities/src/EntityItemProperties.h | 2 +- libraries/entities/src/ZoneEntityItem.cpp | 10 ++-- libraries/entities/src/ZoneEntityItem.h | 7 ++- .../system/assets/data/createAppTooltips.json | 2 +- scripts/system/edit.js | 2 +- scripts/system/html/js/entityProperties.js | 3 +- 10 files changed, 69 insertions(+), 43 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index a63a76829b..cef4383aee 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -91,22 +91,26 @@ namespace { struct FindPriorityZone { glm::vec3 position; bool isInPriorityZone { false }; + float zoneVolume { std::numeric_limits::max() }; + static bool operation(const OctreeElementPointer& element, void* extraData) { auto findPriorityZone = static_cast(extraData); if (element->getAACube().contains(findPriorityZone->position)) { const EntityTreeElementPointer entityTreeElement = static_pointer_cast(element); entityTreeElement->forEachEntity([&findPriorityZone](EntityItemPointer item) { if (item->getType() == EntityTypes::Zone - && item->contains(findPriorityZone->position) - && static_pointer_cast(item)->getAvatarPriority()) { - findPriorityZone->isInPriorityZone = true; + && item->contains(findPriorityZone->position)) { + auto zoneItem = static_pointer_cast(item); + if (zoneItem->getAvatarPriority() != COMPONENT_MODE_INHERIT) { + float volume = zoneItem->getVolumeEstimate(); + if (volume < findPriorityZone->zoneVolume) { // Smaller volume wins + findPriorityZone->isInPriorityZone = zoneItem->getAvatarPriority() == COMPONENT_MODE_ENABLED; + findPriorityZone->zoneVolume = volume; + } + } } }); - if (findPriorityZone->isInPriorityZone) { - return false; // For now, stop at first hit. - } else { - return true; - } + return true; // Keep recursing } else { // Position isn't within this subspace, so end recursion. return false; } @@ -144,10 +148,10 @@ int AvatarMixerClientData::parseData(ReceivedMessage& message, const SlaveShared EntityTree& entityTree = *slaveSharedData.entityTree; FindPriorityZone findPriorityZone { newPosition, false } ; entityTree.recurseTreeWithOperation(&FindPriorityZone::operation, &findPriorityZone); - _avatar->setPriorityAvatar(findPriorityZone.isInPriorityZone); - if (findPriorityZone.isInPriorityZone) { - qCWarning(avatars) << "Avatar" << _avatar->getSessionDisplayName() << "in hero zone"; - } + _avatar->setHasPriority(findPriorityZone.isInPriorityZone); + //if (findPriorityZone.isInPriorityZone) { + // qCWarning(avatars) << "Avatar" << _avatar->getSessionDisplayName() << "in hero zone"; + //} #endif } diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 54eefadda1..80600a53ee 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -460,7 +460,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) } } { - if (sourceAvatarNodeData->getConstAvatarData()->getPriorityAvatar() && !sendAvatar) { + if (sourceAvatarNodeData->getConstAvatarData()->getHasPriority() && !sendAvatar) { qCWarning(avatars) << "Hero avatar dropped:" << sourceAvatarNodeData->getConstAvatarData()->getSessionDisplayName() << "lastSeqToReceiver =" << destinationNodeData->getLastBroadcastSequenceNumber(sourceAvatarNode->getLocalID()) << "lastSeqFromSender = " << sourceAvatarNodeData->getLastReceivedSequenceNumber(); @@ -474,7 +474,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) const MixerAvatar* avatarNodeData = sourceAvatarNodeData->getConstAvatarData(); auto lastEncodeTime = destinationNodeData->getLastOtherAvatarEncodeTime(sourceAvatarNode->getLocalID()); - avatarPriorityQueues[avatarNodeData->getPriorityAvatar() ? kHero : kNonhero].push( + avatarPriorityQueues[avatarNodeData->getHasPriority() ? kHero : kNonhero].push( SortableAvatar(avatarNodeData, sourceAvatarNode, lastEncodeTime)); } @@ -601,7 +601,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) if (detail != AvatarData::NoData) { _stats.numOthersIncluded++; - if (sourceAvatar->getPriorityAvatar()) { + if (sourceAvatar->getHasPriority()) { _stats.numHeroesIncluded++; } diff --git a/assignment-client/src/avatars/MixerAvatar.h b/assignment-client/src/avatars/MixerAvatar.h index 4781fdee1b..2444bd541c 100644 --- a/assignment-client/src/avatars/MixerAvatar.h +++ b/assignment-client/src/avatars/MixerAvatar.h @@ -19,11 +19,11 @@ class MixerAvatar : public AvatarData { public: - bool getPriorityAvatar() const { return _bPriorityAvatar; } - void setPriorityAvatar(bool bPriorityAvatar) { _bPriorityAvatar = bPriorityAvatar; } + bool getHasPriority() const { return _bHasPriority; } + void setHasPriority(bool bPriorityAvatar) { _bHasPriority = bPriorityAvatar; } private: - bool _bPriorityAvatar { false }; + bool _bHasPriority { false }; }; using MixerAvatarSharedPointer = std::shared_ptr; diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index a7cba157ae..ac9b55df4b 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -225,6 +225,15 @@ QString EntityItemProperties::getBloomModeAsString() const { return getComponentModeAsString(_bloomMode); } +namespace { + const QStringList AVATAR_PRIORITIES_AS_STRING + { "inherit", "crowd", "hero" }; +} + +QString EntityItemProperties::getAvatarPriorityAsString() const { + return AVATAR_PRIORITIES_AS_STRING.value(_avatarPriority); +} + std::array::const_iterator EntityItemProperties::findComponent(const QString& mode) { return std::find_if(COMPONENT_MODES.begin(), COMPONENT_MODES.end(), [&](const ComponentPair& pair) { return (pair.second == mode); @@ -249,6 +258,15 @@ void EntityItemProperties::setBloomModeFromString(const QString& bloomMode) { } } +void EntityItemProperties::setAvatarPriorityFromString(QString const& avatarPriority) { + auto result = AVATAR_PRIORITIES_AS_STRING.indexOf(avatarPriority); + + if (result != -1) { + _avatarPriority = result; + _avatarPriorityChanged = true; + } +} + QString EntityItemProperties::getKeyLightModeAsString() const { return getComponentModeAsString(_keyLightMode); } @@ -616,12 +634,12 @@ EntityPropertyFlags EntityItemProperties::getChangedProperties() const { CHECK_PROPERTY_CHANGE(PROP_FLYING_ALLOWED, flyingAllowed); CHECK_PROPERTY_CHANGE(PROP_GHOSTING_ALLOWED, ghostingAllowed); CHECK_PROPERTY_CHANGE(PROP_FILTER_URL, filterURL); - CHECK_PROPERTY_CHANGE(PROP_AVATAR_PRIORITY, avatarPriority); CHECK_PROPERTY_CHANGE(PROP_KEY_LIGHT_MODE, keyLightMode); CHECK_PROPERTY_CHANGE(PROP_AMBIENT_LIGHT_MODE, ambientLightMode); CHECK_PROPERTY_CHANGE(PROP_SKYBOX_MODE, skyboxMode); CHECK_PROPERTY_CHANGE(PROP_HAZE_MODE, hazeMode); CHECK_PROPERTY_CHANGE(PROP_BLOOM_MODE, bloomMode); + CHECK_PROPERTY_CHANGE(PROP_AVATAR_PRIORITY, avatarPriority); // Polyvox CHECK_PROPERTY_CHANGE(PROP_VOXEL_VOLUME_SIZE, voxelVolumeSize); @@ -1422,8 +1440,10 @@ EntityPropertyFlags EntityItemProperties::getChangedProperties() const { * zone. It is periodically executed for each entity in the zone. It can, for example, be used to not allow changes to * certain properties.
* - * @property {boolean} avatarPriority=false - If true avatars within this zone will have their movements distributed to other - * clients with priority over other avatars. Use, for example, on a performance stage with a few presenters. + * @property {string} avatarPriority="inherit" - Configures the update priority of contained avatars to other clients.
+ * "inherit": Priority from enclosing zones is unchanged.
+ * "crowd": Priority in this zone is the normal priority.
+ * "hero": Avatars in this zone will have an increased update priority *
  *
  * function filter(properties) {
@@ -1753,13 +1773,13 @@ QScriptValue EntityItemProperties::copyToScriptValue(QScriptEngine* engine, bool
         COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_FLYING_ALLOWED, flyingAllowed);
         COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_GHOSTING_ALLOWED, ghostingAllowed);
         COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_FILTER_URL, filterURL);
-        COPY_PROPERTY_TO_QSCRIPTVALUE(PROP_AVATAR_PRIORITY, avatarPriority);
 
         COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER(PROP_KEY_LIGHT_MODE, keyLightMode, getKeyLightModeAsString());
         COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER(PROP_AMBIENT_LIGHT_MODE, ambientLightMode, getAmbientLightModeAsString());
         COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER(PROP_SKYBOX_MODE, skyboxMode, getSkyboxModeAsString());
         COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER(PROP_HAZE_MODE, hazeMode, getHazeModeAsString());
         COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER(PROP_BLOOM_MODE, bloomMode, getBloomModeAsString());
+        COPY_PROPERTY_TO_QSCRIPTVALUE_GETTER(PROP_AVATAR_PRIORITY, avatarPriority, getAvatarPriorityAsString());
     }
 
     // Web only
@@ -2114,12 +2134,12 @@ void EntityItemProperties::copyFromScriptValue(const QScriptValue& object, bool
     COPY_PROPERTY_FROM_QSCRIPTVALUE(flyingAllowed, bool, setFlyingAllowed);
     COPY_PROPERTY_FROM_QSCRIPTVALUE(ghostingAllowed, bool, setGhostingAllowed);
     COPY_PROPERTY_FROM_QSCRIPTVALUE(filterURL, QString, setFilterURL);
-    COPY_PROPERTY_FROM_QSCRIPTVALUE(avatarPriority, bool, setAvatarPriority);
     COPY_PROPERTY_FROM_QSCRIPTVALUE_ENUM(keyLightMode, KeyLightMode);
     COPY_PROPERTY_FROM_QSCRIPTVALUE_ENUM(ambientLightMode, AmbientLightMode);
     COPY_PROPERTY_FROM_QSCRIPTVALUE_ENUM(skyboxMode, SkyboxMode);
     COPY_PROPERTY_FROM_QSCRIPTVALUE_ENUM(hazeMode, HazeMode);
     COPY_PROPERTY_FROM_QSCRIPTVALUE_ENUM(bloomMode, BloomMode);
+    COPY_PROPERTY_FROM_QSCRIPTVALUE_ENUM(avatarPriority, AvatarPriority);
 
     // Polyvox
     COPY_PROPERTY_FROM_QSCRIPTVALUE(voxelVolumeSize, vec3, setVoxelVolumeSize);
@@ -2393,12 +2413,12 @@ void EntityItemProperties::merge(const EntityItemProperties& other) {
     COPY_PROPERTY_IF_CHANGED(flyingAllowed);
     COPY_PROPERTY_IF_CHANGED(ghostingAllowed);
     COPY_PROPERTY_IF_CHANGED(filterURL);
-    COPY_PROPERTY_IF_CHANGED(avatarPriority);
     COPY_PROPERTY_IF_CHANGED(keyLightMode);
     COPY_PROPERTY_IF_CHANGED(ambientLightMode);
     COPY_PROPERTY_IF_CHANGED(skyboxMode);
     COPY_PROPERTY_IF_CHANGED(hazeMode);
     COPY_PROPERTY_IF_CHANGED(bloomMode);
+    COPY_PROPERTY_IF_CHANGED(avatarPriority);
 
     // Polyvox
     COPY_PROPERTY_IF_CHANGED(voxelVolumeSize);
@@ -2778,12 +2798,12 @@ bool EntityItemProperties::getPropertyInfo(const QString& propertyName, EntityPr
         ADD_PROPERTY_TO_MAP(PROP_FLYING_ALLOWED, FlyingAllowed, flyingAllowed, bool);
         ADD_PROPERTY_TO_MAP(PROP_GHOSTING_ALLOWED, GhostingAllowed, ghostingAllowed, bool);
         ADD_PROPERTY_TO_MAP(PROP_FILTER_URL, FilterURL, filterURL, QString);
-        ADD_PROPERTY_TO_MAP(PROP_AVATAR_PRIORITY, AvatarPriority, avatarPriority, bool);
         ADD_PROPERTY_TO_MAP(PROP_KEY_LIGHT_MODE, KeyLightMode, keyLightMode, uint32_t);
         ADD_PROPERTY_TO_MAP(PROP_AMBIENT_LIGHT_MODE, AmbientLightMode, ambientLightMode, uint32_t);
         ADD_PROPERTY_TO_MAP(PROP_SKYBOX_MODE, SkyboxMode, skyboxMode, uint32_t);
         ADD_PROPERTY_TO_MAP(PROP_HAZE_MODE, HazeMode, hazeMode, uint32_t);
         ADD_PROPERTY_TO_MAP(PROP_BLOOM_MODE, BloomMode, bloomMode, uint32_t);
+        ADD_PROPERTY_TO_MAP(PROP_AVATAR_PRIORITY, AvatarPriority, avatarPriority, uint32_t);
 
         // Polyvox
         ADD_PROPERTY_TO_MAP(PROP_VOXEL_VOLUME_SIZE, VoxelVolumeSize, voxelVolumeSize, vec3);
@@ -3184,7 +3204,7 @@ OctreeElement::AppendState EntityItemProperties::encodeEntityEditPacket(PacketTy
                 APPEND_ENTITY_PROPERTY(PROP_SKYBOX_MODE, (uint32_t)properties.getSkyboxMode());
                 APPEND_ENTITY_PROPERTY(PROP_HAZE_MODE, (uint32_t)properties.getHazeMode());
                 APPEND_ENTITY_PROPERTY(PROP_BLOOM_MODE, (uint32_t)properties.getBloomMode());
-                APPEND_ENTITY_PROPERTY(PROP_AVATAR_PRIORITY, properties.getAvatarPriority());
+                APPEND_ENTITY_PROPERTY(PROP_AVATAR_PRIORITY, (uint32_t)properties.getAvatarPriority());
             }
 
             if (properties.getType() == EntityTypes::PolyVox) {
@@ -3641,13 +3661,13 @@ bool EntityItemProperties::decodeEntityEditPacket(const unsigned char* data, int
         READ_ENTITY_PROPERTY_TO_PROPERTIES(PROP_FLYING_ALLOWED, bool, setFlyingAllowed);
         READ_ENTITY_PROPERTY_TO_PROPERTIES(PROP_GHOSTING_ALLOWED, bool, setGhostingAllowed);
         READ_ENTITY_PROPERTY_TO_PROPERTIES(PROP_FILTER_URL, QString, setFilterURL);
-        READ_ENTITY_PROPERTY_TO_PROPERTIES(PROP_AVATAR_PRIORITY, bool, setAvatarPriority);
 
         READ_ENTITY_PROPERTY_TO_PROPERTIES(PROP_KEY_LIGHT_MODE, uint32_t, setKeyLightMode);
         READ_ENTITY_PROPERTY_TO_PROPERTIES(PROP_AMBIENT_LIGHT_MODE, uint32_t, setAmbientLightMode);
         READ_ENTITY_PROPERTY_TO_PROPERTIES(PROP_SKYBOX_MODE, uint32_t, setSkyboxMode);
         READ_ENTITY_PROPERTY_TO_PROPERTIES(PROP_HAZE_MODE, uint32_t, setHazeMode);
         READ_ENTITY_PROPERTY_TO_PROPERTIES(PROP_BLOOM_MODE, uint32_t, setBloomMode);
+        READ_ENTITY_PROPERTY_TO_PROPERTIES(PROP_AVATAR_PRIORITY, uint32_t, setAvatarPriority);
     }
 
     if (properties.getType() == EntityTypes::PolyVox) {
@@ -4022,13 +4042,13 @@ void EntityItemProperties::markAllChanged() {
     _bloom.markAllChanged();
     _flyingAllowedChanged = true;
     _ghostingAllowedChanged = true;
-    _avatarPriorityChanged = true;
     _filterURLChanged = true;
     _keyLightModeChanged = true;
     _ambientLightModeChanged = true;
     _skyboxModeChanged = true;
     _hazeModeChanged = true;
     _bloomModeChanged = true;
+    _avatarPriorityChanged = true;
 
     // Polyvox
     _voxelVolumeSizeChanged = true;
@@ -4608,9 +4628,6 @@ QList EntityItemProperties::listChangedProperties() {
     if (filterURLChanged()) {
         out += "filterURL";
     }
-    if (avatarPriorityChanged()) {
-        out += "avatarPriority";
-    }
     if (keyLightModeChanged()) {
         out += "keyLightMode";
     }
@@ -4626,6 +4643,9 @@ QList EntityItemProperties::listChangedProperties() {
     if (bloomModeChanged()) {
         out += "bloomMode";
     }
+    if (avatarPriorityChanged()) {
+        out += "avatarPriority";
+    }
 
     // Polyvox
     if (voxelVolumeSizeChanged()) {
diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h
index ff5204efe2..75b2b2aec3 100644
--- a/libraries/entities/src/EntityItemProperties.h
+++ b/libraries/entities/src/EntityItemProperties.h
@@ -315,12 +315,12 @@ public:
     DEFINE_PROPERTY(PROP_FLYING_ALLOWED, FlyingAllowed, flyingAllowed, bool, ZoneEntityItem::DEFAULT_FLYING_ALLOWED);
     DEFINE_PROPERTY(PROP_GHOSTING_ALLOWED, GhostingAllowed, ghostingAllowed, bool, ZoneEntityItem::DEFAULT_GHOSTING_ALLOWED);
     DEFINE_PROPERTY(PROP_FILTER_URL, FilterURL, filterURL, QString, ZoneEntityItem::DEFAULT_FILTER_URL);
-    DEFINE_PROPERTY(PROP_AVATAR_PRIORITY, AvatarPriority, avatarPriority, bool, ZoneEntityItem::DEFAULT_AVATAR_PRIORITY);
     DEFINE_PROPERTY_REF_ENUM(PROP_KEY_LIGHT_MODE, KeyLightMode, keyLightMode, uint32_t, (uint32_t)COMPONENT_MODE_INHERIT);
     DEFINE_PROPERTY_REF_ENUM(PROP_SKYBOX_MODE, SkyboxMode, skyboxMode, uint32_t, (uint32_t)COMPONENT_MODE_INHERIT);
     DEFINE_PROPERTY_REF_ENUM(PROP_AMBIENT_LIGHT_MODE, AmbientLightMode, ambientLightMode, uint32_t, (uint32_t)COMPONENT_MODE_INHERIT);
     DEFINE_PROPERTY_REF_ENUM(PROP_HAZE_MODE, HazeMode, hazeMode, uint32_t, (uint32_t)COMPONENT_MODE_INHERIT);
     DEFINE_PROPERTY_REF_ENUM(PROP_BLOOM_MODE, BloomMode, bloomMode, uint32_t, (uint32_t)COMPONENT_MODE_INHERIT);
+    DEFINE_PROPERTY_REF_ENUM(PROP_AVATAR_PRIORITY, AvatarPriority, avatarPriority, uint32_t, (uint32_t)COMPONENT_MODE_INHERIT);
 
     // Polyvox
     DEFINE_PROPERTY_REF(PROP_VOXEL_VOLUME_SIZE, VoxelVolumeSize, voxelVolumeSize, glm::vec3, PolyVoxEntityItem::DEFAULT_VOXEL_VOLUME_SIZE);
diff --git a/libraries/entities/src/ZoneEntityItem.cpp b/libraries/entities/src/ZoneEntityItem.cpp
index 83619caa3c..a6b88ca7c2 100644
--- a/libraries/entities/src/ZoneEntityItem.cpp
+++ b/libraries/entities/src/ZoneEntityItem.cpp
@@ -65,13 +65,13 @@ EntityItemProperties ZoneEntityItem::getProperties(const EntityPropertyFlags& de
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(flyingAllowed, getFlyingAllowed);
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(ghostingAllowed, getGhostingAllowed);
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(filterURL, getFilterURL);
-    COPY_ENTITY_PROPERTY_TO_PROPERTIES(avatarPriority, getAvatarPriority);
 
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(keyLightMode, getKeyLightMode);
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(ambientLightMode, getAmbientLightMode);
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(skyboxMode, getSkyboxMode);
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(hazeMode, getHazeMode);
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(bloomMode, getBloomMode);
+    COPY_ENTITY_PROPERTY_TO_PROPERTIES(avatarPriority, getAvatarPriority);
 
     return properties;
 }
@@ -194,7 +194,7 @@ int ZoneEntityItem::readEntitySubclassDataFromBuffer(const unsigned char* data,
     READ_ENTITY_PROPERTY(PROP_SKYBOX_MODE, uint32_t, setSkyboxMode);
     READ_ENTITY_PROPERTY(PROP_HAZE_MODE, uint32_t, setHazeMode);
     READ_ENTITY_PROPERTY(PROP_BLOOM_MODE, uint32_t, setBloomMode);
-    READ_ENTITY_PROPERTY(PROP_AVATAR_PRIORITY, bool, setAvatarPriority);
+    READ_ENTITY_PROPERTY(PROP_AVATAR_PRIORITY, uint32_t, setAvatarPriority);
 
     return bytesRead;
 }
@@ -476,8 +476,10 @@ bool ZoneEntityItem::matchesJSONFilters(const QJsonObject& jsonFilters) const {
 
     static const QString AVATAR_PRIORITY_PROPERTY = "avatarPriority";
 
-    if (jsonFilters.contains(AVATAR_PRIORITY_PROPERTY)) {
-        return (jsonFilters[AVATAR_PRIORITY_PROPERTY].toBool() == _avatarPriority);
+    // If set ignore only priority-inherit zones:
+    if (jsonFilters.contains(AVATAR_PRIORITY_PROPERTY) && jsonFilters[AVATAR_PRIORITY_PROPERTY].toBool()
+        && _avatarPriority != COMPONENT_MODE_INHERIT) {
+        return true;
     }
 
     // Chain to base:
diff --git a/libraries/entities/src/ZoneEntityItem.h b/libraries/entities/src/ZoneEntityItem.h
index a3e668b6f6..1afcfcba47 100644
--- a/libraries/entities/src/ZoneEntityItem.h
+++ b/libraries/entities/src/ZoneEntityItem.h
@@ -98,8 +98,8 @@ public:
     QString getFilterURL() const;
     void setFilterURL(const QString url); 
 
-    bool getAvatarPriority() const { return _avatarPriority; }
-    void setAvatarPriority(bool value) { _avatarPriority = value; }
+    uint32_t getAvatarPriority() const { return _avatarPriority; }
+    void setAvatarPriority(uint32_t value) { _avatarPriority = value; }
 
     bool keyLightPropertiesChanged() const { return _keyLightPropertiesChanged; }
     bool ambientLightPropertiesChanged() const { return _ambientLightPropertiesChanged; }
@@ -130,7 +130,6 @@ public:
     static const bool DEFAULT_FLYING_ALLOWED;
     static const bool DEFAULT_GHOSTING_ALLOWED;
     static const QString DEFAULT_FILTER_URL;
-    static const bool DEFAULT_AVATAR_PRIORITY = false;
 
 protected:
     KeyLightPropertyGroup _keyLightProperties;
@@ -156,7 +155,7 @@ protected:
     QString _filterURL { DEFAULT_FILTER_URL };
 
     // Avatar-updates priority
-    bool _avatarPriority { DEFAULT_AVATAR_PRIORITY };
+    uint32_t _avatarPriority { COMPONENT_MODE_INHERIT };
 
     // Dirty flags turn true when either keylight properties is changing values.
     bool _keyLightPropertiesChanged { false };
diff --git a/scripts/system/assets/data/createAppTooltips.json b/scripts/system/assets/data/createAppTooltips.json
index 7e5f2c8659..7201cdecad 100644
--- a/scripts/system/assets/data/createAppTooltips.json
+++ b/scripts/system/assets/data/createAppTooltips.json
@@ -135,7 +135,7 @@
         "tooltip": "The radius of bloom. The higher the value, the larger the bloom."
     },
     "avatarPriority": {
-        "tooltip":  "Avatars in this zone will have a higher update priority."
+        "tooltip":  "Alter Avatars' update priorities."
     },
     "modelURL": {
         "tooltip": "A mesh model from an FBX or OBJ file."
diff --git a/scripts/system/edit.js b/scripts/system/edit.js
index 67a789c266..2c3785217c 100644
--- a/scripts/system/edit.js
+++ b/scripts/system/edit.js
@@ -383,7 +383,7 @@ const DEFAULT_ENTITY_PROPERTIES = {
         },
         shapeType: "box",
         bloomMode: "inherit",
-        avatarPriority: false
+        avatarPriority: "inherit"
     },
     Model: {
         collisionShape: "none",
diff --git a/scripts/system/html/js/entityProperties.js b/scripts/system/html/js/entityProperties.js
index 404ded6ae2..b78aee4ad5 100644
--- a/scripts/system/html/js/entityProperties.js
+++ b/scripts/system/html/js/entityProperties.js
@@ -430,7 +430,8 @@ const GROUPS = [
             },
             {
                 label: "Avatar Priority",
-                type: "bool",
+                type: "dropdown",
+                options: { inherit: "Inherit", crowd: "Crowd", hero: "Hero" },
                 propertyID: "avatarPriority",
             },
 

From 122b62a5b82fded9d66450dc6f6b3f251c4ea91b Mon Sep 17 00:00:00 2001
From: Simon Walton 
Date: Mon, 4 Mar 2019 17:32:18 -0800
Subject: [PATCH 6/7] Remove some debug output

---
 assignment-client/src/avatars/AvatarMixerSlave.cpp | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp
index 80600a53ee..b52bf03471 100644
--- a/assignment-client/src/avatars/AvatarMixerSlave.cpp
+++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp
@@ -459,13 +459,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node)
                 ++numAvatarsWithSkippedFrames;
             }
         }
-        {
-            if (sourceAvatarNodeData->getConstAvatarData()->getHasPriority() && !sendAvatar) {
-                qCWarning(avatars) << "Hero avatar dropped:" << sourceAvatarNodeData->getConstAvatarData()->getSessionDisplayName()
-                    << "lastSeqToReceiver =" << destinationNodeData->getLastBroadcastSequenceNumber(sourceAvatarNode->getLocalID())
-                    << "lastSeqFromSender = " << sourceAvatarNodeData->getLastReceivedSequenceNumber();
-            }
-        }
+
         quint64 endIgnoreCalculation = usecTimestampNow();
         _stats.ignoreCalculationElapsedTime += (endIgnoreCalculation - startIgnoreCalculation);
 

From 743d1a58e2c4d07fa26f61b1512190263225fa42 Mon Sep 17 00:00:00 2001
From: Simon Walton 
Date: Tue, 5 Mar 2019 09:34:29 -0800
Subject: [PATCH 7/7] Style tweaks from review

---
 assignment-client/src/avatars/AvatarMixerSlave.cpp | 6 ------
 assignment-client/src/avatars/MixerAvatar.h        | 6 +++---
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp
index b52bf03471..e59c81f4b7 100644
--- a/assignment-client/src/avatars/AvatarMixerSlave.cpp
+++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp
@@ -355,9 +355,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node)
     // about avatars they've ignored or that are out of view
     bool PALIsOpen = destinationNodeData->getRequestsDomainListData();
     bool PALWasOpen = destinationNodeData->getPrevRequestsDomainListData();
-    if (PALIsOpen) {
-        qCWarning(avatars) << "PAL is open:" << avatar.getSessionDisplayName();
-    }
 
     // When this is true, the AvatarMixer will send Avatar data to a client about avatars that have ignored them
     bool getsAnyIgnored = PALIsOpen && destinationNode->getCanKick();
@@ -526,9 +523,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node)
                     _stats.overBudgetAvatars++;
                     detail = AvatarData::PALMinimum;
                 } else {
-                    if (currentVariant == kHero) {
-                        qCWarning(avatars) << "Overbudget break with hero avatars!" << destinationNode->getUUID().toString();
-                    }
                     _stats.overBudgetAvatars += remainingAvatars;
                     break;
                 }
diff --git a/assignment-client/src/avatars/MixerAvatar.h b/assignment-client/src/avatars/MixerAvatar.h
index 2444bd541c..4c3ded4582 100644
--- a/assignment-client/src/avatars/MixerAvatar.h
+++ b/assignment-client/src/avatars/MixerAvatar.h
@@ -19,11 +19,11 @@
 
 class MixerAvatar : public AvatarData {
 public:
-    bool getHasPriority() const { return  _bHasPriority; }
-    void setHasPriority(bool bPriorityAvatar) { _bHasPriority = bPriorityAvatar; }
+    bool getHasPriority() const { return  _hasPriority; }
+    void setHasPriority(bool hasPriority) { _hasPriority = hasPriority; }
 
 private:
-    bool _bHasPriority { false };
+    bool _hasPriority { false };
 };
 
 using MixerAvatarSharedPointer = std::shared_ptr;