Merge pull request #37 from birarda/entity-script-server

fix interest list checks, remove unecessary sets

domain-server/src/DomainServer.cpp

@@ -855,6 +855,44 @@ void DomainServer::processListRequestPacket(QSharedPointer<ReceivedMessage> mess
     sendDomainListToNode(sendingNode, message->getSenderSockAddr());
 }
 
+bool DomainServer::isInInterestSet(const SharedNodePointer& nodeA, const SharedNodePointer& nodeB) {
+    auto nodeAData = static_cast<DomainServerNodeData*>(nodeA->getLinkedData());
+    auto nodeBData = static_cast<DomainServerNodeData*>(nodeB->getLinkedData());
+
+    // if we have no linked data for node A then B can't possibly be in the interest set
+    if (!nodeAData) {
+        return false;
+    }
+
+    // first check if the general interest set A contains the type for B
+    if (nodeAData->getNodeInterestSet().contains(nodeB->getType())) {
+        // given that there is a match in the general interest set, do any special checks
+
+        // (1/19/17) Agents only need to connect to Entity Script Servers to perform administrative tasks
+        // related to entity server scripts. Only agents with rez permissions should be doing that, so
+        // if the agent does not have those permissions, we do not want them and the server to incur the
+        // overhead of connecting to one another. Additionally we exclude agents that do not care about the
+        // Entity Script Server and won't attempt to connect to it.
+
+        bool isAgentWithoutRights = nodeA->getType() == NodeType::Agent
+            && nodeB->getType() == NodeType::EntityScriptServer
+            && !nodeA->getCanRez() && !nodeA->getCanRezTmp();
+
+        if (isAgentWithoutRights) {
+            return false;
+        }
+
+        bool isScriptServerForIneffectiveAgent =
+            (nodeA->getType() == NodeType::EntityScriptServer && nodeB->getType() == NodeType::Agent)
+            && ((nodeBData && !nodeBData->getNodeInterestSet().contains(NodeType::EntityScriptServer))
+                || (!nodeB->getCanRez() && !nodeB->getCanRezTmp()));
+
+        return !isScriptServerForIneffectiveAgent;
+    } else {
+        return false;
+    }
+}
+
 unsigned int DomainServer::countConnectedUsers() {
     unsigned int result = 0;
     auto nodeList = DependencyManager::get<LimitedNodeList>();
@@ -954,40 +992,18 @@ void DomainServer::sendDomainListToNode(const SharedNodePointer& node, const Hif
         if (nodeData->isAuthenticated()) {
             // if this authenticated node has any interest types, send back those nodes as well
             limitedNodeList->eachNode([&](const SharedNodePointer& otherNode) {
-                if (otherNode->getUUID() != node->getUUID()
+                if (otherNode->getUUID() != node->getUUID() && isInInterestSet(node, otherNode)) {
-                    && nodeInterestSet.contains(otherNode->getType())) {
+                    // since we're about to add a node to the packet we start a segment
+                    domainListPackets->startSegment();
 
-                    // (1/19/17) Agents only need to connect to Entity Script Servers to perform administrative tasks
+                    // don't send avatar nodes to other avatars, that will come from avatar mixer
-                    // related to entity server scripts. Only agents with rez permissions should be doing that, so
+                    domainListStream << *otherNode.data();
-                    // if the agent does not have those permissions, we do not want them and the server to incur the
-                    // overhead of connecting to one another. Additionally we exclude agents that do not care about the
-                    // Entity Script Server and won't attempt to connect to it.
-                    auto otherNodeData = static_cast<DomainServerNodeData*>(otherNode->getLinkedData());
 
-                    bool isAgentWithoutRights = node->getType() == NodeType::Agent
+                    // pack the secret that these two nodes will use to communicate with each other
-                        && otherNode->getType() == NodeType::EntityScriptServer
+                    domainListStream << connectionSecretForNodes(node, otherNode);
-                        && !node->getCanRez() && !node->getCanRezTmp();
 
-                    bool isScriptServerForIneffectiveAgent =
+                    // we've added the node we wanted so end the segment now
-                        (node->getType() == NodeType::EntityScriptServer && otherNode->getType() == NodeType::Agent)
+                    domainListPackets->endSegment();
-                        && (!otherNodeData->getNodeInterestSet().contains(NodeType::EntityServer)
-                        || (!otherNode->getCanRez() && !otherNode->getCanRezTmp()));
-
-                    bool shouldNotConnect = isAgentWithoutRights || isScriptServerForIneffectiveAgent;
-
-                    if (!shouldNotConnect) {
-                        // since we're about to add a node to the packet we start a segment
-                        domainListPackets->startSegment();
-
-                        // don't send avatar nodes to other avatars, that will come from avatar mixer
-                        domainListStream << *otherNode.data();
-
-                        // pack the secret that these two nodes will use to communicate with each other
-                        domainListStream << connectionSecretForNodes(node, otherNode);
-
-                        // we've added the node we wanted so end the segment now
-                        domainListPackets->endSegment();
-                    }
                 }
             });
         }
@@ -1038,8 +1054,7 @@ void DomainServer::broadcastNewNode(const SharedNodePointer& addedNode) {
         [&](const SharedNodePointer& node)->bool {
             if (node->getLinkedData() && node->getActiveSocket() && node != addedNode) {
                 // is the added Node in this node's interest list?
-                DomainServerNodeData* nodeData = dynamic_cast<DomainServerNodeData*>(node->getLinkedData());
+                return isInInterestSet(node, addedNode);
-                return nodeData->getNodeInterestSet().contains(addedNode->getType());
             } else {
                 return false;
             }
@@ -2389,7 +2404,6 @@ void DomainServer::processNodeDisconnectRequestPacket(QSharedPointer<ReceivedMes
 }
 
 void DomainServer::handleKillNode(SharedNodePointer nodeToKill) {
-    auto nodeType = nodeToKill->getType();
     auto limitedNodeList = DependencyManager::get<LimitedNodeList>();
     const QUuid& nodeUUID = nodeToKill->getUUID();
 
@@ -2401,10 +2415,9 @@ void DomainServer::handleKillNode(SharedNodePointer nodeToKill) {
     removedNodePacket->write(nodeUUID.toRfc4122());
 
     // broadcast out the DomainServerRemovedNode message
-    limitedNodeList->eachMatchingNode([&nodeType](const SharedNodePointer& otherNode) -> bool {
+    limitedNodeList->eachMatchingNode([this, &nodeToKill](const SharedNodePointer& otherNode) -> bool {
         // only send the removed node packet to nodes that care about the type of node this was
-        auto nodeLinkedData = dynamic_cast<DomainServerNodeData*>(otherNode->getLinkedData());
+        return isInInterestSet(otherNode, nodeToKill);
-        return (nodeLinkedData != nullptr) && nodeLinkedData->getNodeInterestSet().contains(nodeType);
     }, [&limitedNodeList](const SharedNodePointer& otherNode){
         limitedNodeList->sendUnreliablePacket(*removedNodePacket, *otherNode);
     });

domain-server/src/DomainServer.h

@@ -132,6 +132,8 @@ private:
 
     void sendDomainListToNode(const SharedNodePointer& node, const HifiSockAddr& senderSockAddr);
 
+    bool isInInterestSet(const SharedNodePointer& nodeA, const SharedNodePointer& nodeB);
+
     QUuid connectionSecretForNodes(const SharedNodePointer& nodeA, const SharedNodePointer& nodeB);
     void broadcastNewNode(const SharedNodePointer& node);
 

libraries/entities/src/EntityTreeElement.cpp

@@ -309,16 +309,12 @@ OctreeElement::AppendState EntityTreeElement::appendElementData(OctreePacketData
                     bool entityMatchesFilters = entity->matchesJSONFilters(jsonFilters);
 
                     if (entityMatchesFilters) {
-                        // we should include this entity unless it has already been excluded
-                        includeThisEntity = true;
-
                         // make sure this entity is in the set of entities sent last frame
                         entityNodeData->insertEntitySentLastFrame(entity->getID());
 
                     } else {
                         // we might include this entity if it matched in the previous frame
                         if (entityNodeData->sentEntityLastFrame(entity->getID())) {
-                            includeThisEntity = true;
 
                             entityNodeData->removeEntitySentLastFrame(entity->getID());
                         } else {