From b82356a249f00b6678bd161d6f88eb366bf2f39f Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Tue, 17 May 2016 15:02:04 -0700 Subject: [PATCH] AvatarMixer: Clients will show incompatible version dialog For this to work, the server needs to send an empty AvatarIdentity packet back to the sender when it receives a packet mismatch error. This AvatarIdentity packet will be different then what the client expects and will trigger the incompatible version dialog. Previously, the avatar-mixer was just silently dropping incoming mismatched version packets. Causing the client to never get a response, and thus never showing the incompatible version dialog. --- assignment-client/src/avatars/AvatarMixer.cpp | 16 ++++++++++++++++ assignment-client/src/avatars/AvatarMixer.h | 3 ++- libraries/networking/src/LimitedNodeList.cpp | 7 ++++--- libraries/networking/src/LimitedNodeList.h | 4 +++- libraries/networking/src/udt/PacketHeaders.cpp | 4 +++- libraries/networking/src/udt/PacketHeaders.h | 2 +- 6 files changed, 29 insertions(+), 7 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index a109934d10..610c9bcc40 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -45,6 +45,9 @@ AvatarMixer::AvatarMixer(ReceivedMessage& message) : packetReceiver.registerListener(PacketType::AvatarData, this, "handleAvatarDataPacket"); packetReceiver.registerListener(PacketType::AvatarIdentity, this, "handleAvatarIdentityPacket"); packetReceiver.registerListener(PacketType::KillAvatar, this, "handleKillAvatarPacket"); + + auto nodeList = DependencyManager::get(); + connect(nodeList.data(), &NodeList::packetVersionMismatch, this, &AvatarMixer::handlePacketVersionMismatch); } AvatarMixer::~AvatarMixer() { @@ -509,6 +512,19 @@ void AvatarMixer::domainSettingsRequestComplete() { _broadcastThread.start(); } +void AvatarMixer::handlePacketVersionMismatch(PacketType type, const HifiSockAddr& senderSockAddr, const QUuid& senderUUID) { + // if this client is using packet versions we don't expect. + if ((type == PacketTypeEnum::Value::AvatarIdentity || type == PacketTypeEnum::Value::AvatarData) && !senderUUID.isNull()) { + // Echo an empty AvatarIdentity packet back to that client. + // This should trigger a version mismatch dialog on their side. + auto nodeList = DependencyManager::get(); + auto node = nodeList->nodeWithUUID(senderUUID); + if (node) { + auto poisonPacket = NLPacket::create(PacketType::AvatarIdentity, 0); + nodeList->sendPacket(std::move(poisonPacket), *node); + } + } +} void AvatarMixer::parseDomainServerSettings(const QJsonObject& domainSettings) { const QString AVATAR_MIXER_SETTINGS_KEY = "avatar_mixer"; diff --git a/assignment-client/src/avatars/AvatarMixer.h b/assignment-client/src/avatars/AvatarMixer.h index c7761a2cba..d1a9249c83 100644 --- a/assignment-client/src/avatars/AvatarMixer.h +++ b/assignment-client/src/avatars/AvatarMixer.h @@ -38,7 +38,8 @@ private slots: void handleAvatarIdentityPacket(QSharedPointer message, SharedNodePointer senderNode); void handleKillAvatarPacket(QSharedPointer message); void domainSettingsRequestComplete(); - + void handlePacketVersionMismatch(PacketType type, const HifiSockAddr& senderSockAddr, const QUuid& senderUUID); + private: void broadcastAvatarData(); void parseDomainServerSettings(const QJsonObject& domainSettings); diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 2c10d0627e..9efe51183e 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -176,9 +176,10 @@ bool LimitedNodeList::packetVersionMatch(const udt::Packet& packet) { bool hasBeenOutput = false; QString senderString; + const HifiSockAddr& senderSockAddr = packet.getSenderSockAddr(); + QUuid sourceID; if (NON_SOURCED_PACKETS.contains(headerType)) { - const HifiSockAddr& senderSockAddr = packet.getSenderSockAddr(); hasBeenOutput = versionDebugSuppressMap.contains(senderSockAddr, headerType); if (!hasBeenOutput) { @@ -186,7 +187,7 @@ bool LimitedNodeList::packetVersionMatch(const udt::Packet& packet) { senderString = QString("%1:%2").arg(senderSockAddr.getAddress().toString()).arg(senderSockAddr.getPort()); } } else { - QUuid sourceID = NLPacket::sourceIDInHeader(packet); + sourceID = NLPacket::sourceIDInHeader(packet); hasBeenOutput = sourcedVersionDebugSuppressMap.contains(sourceID, headerType); @@ -201,7 +202,7 @@ bool LimitedNodeList::packetVersionMatch(const udt::Packet& packet) { << senderString << "sent" << qPrintable(QString::number(headerVersion)) << "but" << qPrintable(QString::number(versionForPacketType(headerType))) << "expected."; - emit packetVersionMismatch(headerType); + emit packetVersionMismatch(headerType, senderSockAddr, sourceID); } return false; diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 0cbe9668b3..2ab8aaab39 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -236,7 +236,9 @@ public slots: signals: void dataSent(quint8 channelType, int bytes); - void packetVersionMismatch(PacketType type); + + // QUuid might be zero for non-sourced packet types. + void packetVersionMismatch(PacketType type, const HifiSockAddr& senderSockAddr, const QUuid& senderUUID); void uuidChanged(const QUuid& ownerUUID, const QUuid& oldUUID); void nodeAdded(SharedNodePointer); diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index e4aab94090..81984521f8 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -48,9 +48,11 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::EntityEdit: case PacketType::EntityData: return VERSION_LIGHT_HAS_FALLOFF_RADIUS; + case PacketType::AvatarIdentity: case PacketType::AvatarData: case PacketType::BulkAvatarData: - return static_cast(AvatarMixerPacketVersion::SoftAttachmentSupport); + case PacketType::KillAvatar: + return static_cast(AvatarMixerPacketVersion::AbsoluteSixByteRotations); case PacketType::ICEServerHeartbeat: return 18; // ICE Server Heartbeat signing case PacketType::AssetGetInfo: diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index e0d854ab71..4c2141dff4 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -175,7 +175,7 @@ const PacketVersion VERSION_LIGHT_HAS_FALLOFF_RADIUS = 57; enum class AvatarMixerPacketVersion : PacketVersion { TranslationSupport = 17, SoftAttachmentSupport, - AbsoluteFortyEightBitRotations + AbsoluteSixByteRotations }; #endif // hifi_PacketHeaders_h