fixes and improvments addressing CR comments

This commit is contained in:
Stephen Birarda 2018-08-20 17:47:32 -07:00
parent 4875738a05
commit 3e2d4dc696
12 changed files with 34 additions and 52 deletions

View file

@ -49,7 +49,7 @@ void AvatarMixerClientData::queuePacket(QSharedPointer<ReceivedMessage> message,
_packetQueue.push(message);
}
int AvatarMixerClientData::processPackets(SlaveSharedData* slaveSharedData) {
int AvatarMixerClientData::processPackets(const SlaveSharedData& slaveSharedData) {
int packetsProcessed = 0;
SharedNodePointer node = _packetQueue.node;
assert(_packetQueue.empty() || node);
@ -93,7 +93,8 @@ int AvatarMixerClientData::parseData(ReceivedMessage& message) {
return _avatar->parseDataFromBuffer(message.readWithoutCopy(message.getBytesLeftToRead()));
}
void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, SlaveSharedData* slaveSharedData, Node& sendingNode) {
void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message,
const SlaveSharedData& slaveSharedData, Node& sendingNode) {
// pull the trait version from the message
AvatarTraits::TraitVersion packetTraitVersion;
message.readPrimitive(&packetTraitVersion);
@ -155,9 +156,9 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, Sl
}
}
void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(SlaveSharedData *slaveSharedData, Node& sendingNode,
void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(const SlaveSharedData &slaveSharedData, Node& sendingNode,
AvatarTraits::TraitVersion traitVersion) {
const auto& whitelist = slaveSharedData->skeletonURLWhitelist;
const auto& whitelist = slaveSharedData.skeletonURLWhitelist;
if (!whitelist.isEmpty()) {
bool inWhitelist = false;
@ -179,11 +180,11 @@ void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(SlaveSharedData *sl
if (!inWhitelist) {
// make sure we're not unecessarily overriding the default avatar with the default avatar
if (_avatar->getWireSafeSkeletonModelURL() != slaveSharedData->skeletonReplacementURL) {
if (_avatar->getWireSafeSkeletonModelURL() != slaveSharedData.skeletonReplacementURL) {
// we need to change this avatar's skeleton URL, and send them a traits packet informing them of the change
qDebug() << "Overwriting avatar URL" << _avatar->getWireSafeSkeletonModelURL()
<< "to replacement" << slaveSharedData->skeletonReplacementURL << "for" << sendingNode.getUUID();
_avatar->setSkeletonModelURL(slaveSharedData->skeletonReplacementURL);
<< "to replacement" << slaveSharedData.skeletonReplacementURL << "for" << sendingNode.getUUID();
_avatar->setSkeletonModelURL(slaveSharedData.skeletonReplacementURL);
auto packet = NLPacket::create(PacketType::SetAvatarTraits, -1, true);

View file

@ -116,10 +116,10 @@ public:
QVector<JointData>& getLastOtherAvatarSentJoints(QUuid otherAvatar) { return _lastOtherAvatarSentJoints[otherAvatar]; }
void queuePacket(QSharedPointer<ReceivedMessage> message, SharedNodePointer node);
int processPackets(SlaveSharedData* slaveSharedData); // returns number of packets processed
int processPackets(const SlaveSharedData& slaveSharedData); // returns number of packets processed
void processSetTraitsMessage(ReceivedMessage& message, SlaveSharedData* slaveSharedData, Node& sendingNode);
void checkSkeletonURLAgainstWhitelist(SlaveSharedData* slaveSharedData, Node& sendingNode,
void processSetTraitsMessage(ReceivedMessage& message, const SlaveSharedData& slaveSharedData, Node& sendingNode);
void checkSkeletonURLAgainstWhitelist(const SlaveSharedData& slaveSharedData, Node& sendingNode,
AvatarTraits::TraitVersion traitVersion);
using TraitsCheckTimestamp = std::chrono::steady_clock::time_point;

View file

@ -59,7 +59,7 @@ void AvatarMixerSlave::processIncomingPackets(const SharedNodePointer& node) {
auto nodeData = dynamic_cast<AvatarMixerClientData*>(node->getLinkedData());
if (nodeData) {
_stats.nodesProcessed++;
_stats.packetsProcessed += nodeData->processPackets(_sharedData);
_stats.packetsProcessed += nodeData->processPackets(*_sharedData);
}
auto end = usecTimestampNow();
_stats.processIncomingPacketsElapsedTime += (end - start);
@ -109,19 +109,15 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis
auto traitType = static_cast<AvatarTraits::TraitType>(std::distance(lastReceivedVersions.simpleCBegin(),
simpleReceivedIt));
// we need to double check that this is actually a simple trait type, since the instanced
// trait types are in the simple vector for access efficiency
if (AvatarTraits::isSimpleTrait(traitType)) {
auto lastReceivedVersion = *simpleReceivedIt;
auto& lastSentVersionRef = lastSentVersions[traitType];
auto lastReceivedVersion = *simpleReceivedIt;
auto& lastSentVersionRef = lastSentVersions[traitType];
if (lastReceivedVersions[traitType] > lastSentVersionRef) {
// there is an update to this trait, add it to the traits packet
bytesWritten += sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion);
if (lastReceivedVersions[traitType] > lastSentVersionRef) {
// there is an update to this trait, add it to the traits packet
bytesWritten += sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion);
// update the last sent version
lastSentVersionRef = lastReceivedVersion;
}
// update the last sent version
lastSentVersionRef = lastReceivedVersion;
}
++simpleReceivedIt;

View file

@ -5488,10 +5488,7 @@ void Application::update(float deltaTime) {
// we haven't yet enabled physics. we wait until we think we have all the collision information
// for nearby entities before starting bullet up.
quint64 now = usecTimestampNow();
// Check for flagged EntityData having arrived.
auto entityTreeRenderer = getEntities();
if (isServerlessMode() ||
(entityTreeRenderer && _octreeProcessor.octreeSequenceIsComplete(entityTreeRenderer->getLastOctreeMessageSequence()) )) {
if (isServerlessMode() || _octreeProcessor.isLoadSequenceComplete()) {
// we've received a new full-scene octree stats packet, or it's been long enough to try again anyway
_lastPhysicsCheckTime = now;
_fullSceneCounterAtLastPhysicsCheck = _fullSceneReceivedCounter;

View file

@ -18,7 +18,7 @@ namespace AvatarTraits {
template<typename T, T defaultValue>
class AssociatedTraitValues {
public:
AssociatedTraitValues() : _simpleTypes(TotalTraitTypes, defaultValue) {}
AssociatedTraitValues() : _simpleTypes(FirstInstancedTrait, defaultValue) {}
void insert(TraitType type, T value) { _simpleTypes[type] = value; }
void erase(TraitType type) { _simpleTypes[type] = defaultValue; }

View file

@ -1834,8 +1834,7 @@ qint64 AvatarData::packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice
bytesWritten += destination.writePrimitive(traitType);
if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) {
AvatarTraits::TraitVersion typedVersion = traitVersion;
bytesWritten += destination.writePrimitive(typedVersion);
bytesWritten += destination.writePrimitive(traitVersion);
}
if (traitType == AvatarTraits::SkeletonModelURL) {
@ -1857,8 +1856,7 @@ qint64 AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTr
bytesWritten += destination.writePrimitive(traitType);
if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) {
AvatarTraits::TraitVersion typedVersion = traitVersion;
bytesWritten += destination.writePrimitive(typedVersion);
bytesWritten += destination.writePrimitive(traitVersion);
}
bytesWritten += destination.write(traitInstanceID.toRfc4122());

View file

@ -22,14 +22,15 @@ namespace AvatarTraits {
enum TraitType : int8_t {
NullTrait = -1,
SkeletonModelURL,
AvatarEntity,
FirstInstancedTrait,
AvatarEntity = FirstInstancedTrait,
TotalTraitTypes
};
using TraitInstanceID = QUuid;
inline bool isSimpleTrait(TraitType traitType) {
return traitType == SkeletonModelURL;
return traitType > NullTrait && traitType < FirstInstancedTrait;
}
using TraitVersion = int32_t;
@ -46,8 +47,7 @@ namespace AvatarTraits {
bytesWritten += destination.writePrimitive(traitType);
if (traitVersion > DEFAULT_TRAIT_VERSION) {
AvatarTraits::TraitVersion typedVersion = traitVersion;
bytesWritten += destination.writePrimitive(typedVersion);
bytesWritten += destination.writePrimitive(traitVersion);
}
bytesWritten += destination.write(instanceID.toRfc4122());

View file

@ -68,14 +68,12 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() {
// we double check that it is a simple iterator here
auto traitType = static_cast<AvatarTraits::TraitType>(std::distance(traitStatusesCopy.simpleCBegin(), simpleIt));
if (AvatarTraits::isSimpleTrait(traitType)) {
if (_shouldPerformInitialSend || *simpleIt == Updated) {
if (traitType == AvatarTraits::SkeletonModelURL) {
_owningAvatar->packTrait(traitType, *traitsPacketList);
if (_shouldPerformInitialSend || *simpleIt == Updated) {
if (traitType == AvatarTraits::SkeletonModelURL) {
_owningAvatar->packTrait(traitType, *traitsPacketList);
// keep track of our skeleton version in case we get an override back
_currentSkeletonVersion = _currentTraitVersion;
}
// keep track of our skeleton version in case we get an override back
_currentSkeletonVersion = _currentTraitVersion;
}
}

View file

@ -18,5 +18,3 @@ NodeData::NodeData(const QUuid& nodeID, NetworkPeer::LocalID nodeLocalID) :
{
}
NodeData::~NodeData() {}

View file

@ -26,7 +26,7 @@ class NodeData : public QObject {
Q_OBJECT
public:
NodeData(const QUuid& nodeID = QUuid(), NetworkPeer::LocalID localID = NetworkPeer::NULL_LOCAL_ID);
virtual ~NodeData() = 0;
virtual ~NodeData() = default;
virtual int parseData(ReceivedMessage& message) { return 0; }
const QUuid& getNodeID() const { return _nodeID; }

View file

@ -77,12 +77,6 @@ BasePacket::BasePacket(std::unique_ptr<char[]> data, qint64 size, const HifiSock
}
BasePacket::BasePacket(const BasePacket& other) :
ExtendedIODevice()
{
*this = other;
}
BasePacket& BasePacket::operator=(const BasePacket& other) {
_packetSize = other._packetSize;
_packet = std::unique_ptr<char[]>(new char[_packetSize]);

View file

@ -88,7 +88,7 @@ public:
protected:
BasePacket(qint64 size);
BasePacket(std::unique_ptr<char[]> data, qint64 size, const HifiSockAddr& senderSockAddr);
BasePacket(const BasePacket& other);
BasePacket(const BasePacket& other) { *this = other; }
BasePacket& operator=(const BasePacket& other);
BasePacket(BasePacket&& other);
BasePacket& operator=(BasePacket&& other);