From bde3dd0644c445fd31fe6166df1ac2acb60380eb Mon Sep 17 00:00:00 2001 From: Richard Harrison Date: Thu, 16 Feb 2017 09:16:27 +0100 Subject: [PATCH 1/3] Canvas texture replacement visitor compiler fix. Change to use pointers rather than osg::ref_ptr - based on http://andesengineering.com/OSG_ProducerArticles/RefPointers/RefPointers I think that it is not possible that the scenegraph can be modified between the visitor and the modify, provided that the methods are called after each other like this: ReplaceStaticTextureVisitor visitor(name, new_texture); branch->accept(visitor); visitor.modify_groups(); return visitor.getPlacements(); --- src/Cockpit/od_gauge.cxx | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Cockpit/od_gauge.cxx b/src/Cockpit/od_gauge.cxx index 3bf03b6bd..f6efb0251 100644 --- a/src/Cockpit/od_gauge.cxx +++ b/src/Cockpit/od_gauge.cxx @@ -70,9 +70,16 @@ FGODGauge::~FGODGauge() * Used to remember the located groups that require modification */ typedef struct { - typedef osg::ref_ptr GroupPtr; - GroupPtr parent; - GroupPtr node; + // could be: + //typedef osg::ref_ptr GroupPtr; + //GroupPtr parent; + //GroupPtr node; + // However this gives compile errors on linux; + // so change to use good old fashioned pointers. + // This means that the pointer may become invalid; however provided that this is a short lived operation + // and that modify is called immediately after visit it should work. + osg::Group *parent; + osg::Group *node; int unit; }GroupListItem; @@ -207,6 +214,7 @@ class ReplaceStaticTextureVisitor: * this section of code used to be in the apply method above, however to work this requires modification of the scenegraph nodes * that are currently iterating, so instead the apply method will locate the groups to be modified and when finished then the * nodes can actually be modified safely. Initially found thanks to the debug RTL in MSVC2015 throwing an exception. + * should be called immediately after the visitor to ensure that the groups are still valid and that nothing else has modified these groups. */ void modify_groups() { From 11778f595fba0094af90a1ffaa3166006960b772 Mon Sep 17 00:00:00 2001 From: Richard Harrison Date: Thu, 16 Feb 2017 18:29:44 +0100 Subject: [PATCH 2/3] Fixed exception caused when next is equal to end() This happened whilst running under debug whilst spawning at a new location so it may be an edge case, but still worth checking for --- src/AIModel/AIMultiplayer.cxx | 193 ++++++++++++++++++---------------- 1 file changed, 100 insertions(+), 93 deletions(-) diff --git a/src/AIModel/AIMultiplayer.cxx b/src/AIModel/AIMultiplayer.cxx index 0e563ad19..9fcf17bea 100644 --- a/src/AIModel/AIMultiplayer.cxx +++ b/src/AIModel/AIMultiplayer.cxx @@ -284,104 +284,111 @@ void FGAIMultiplayer::update(double dt) // Interpolation coefficient is between 0 and 1 double intervalStart = prevIt->second.time; double intervalEnd = nextIt->second.time; - double intervalLen = intervalEnd - intervalStart; - double tau = 0.0; - if (intervalLen != 0.0) tau = (tInterp - intervalStart)/intervalLen; - SG_LOG(SG_AI, SG_DEBUG, "Multiplayer vehicle interpolation: [" - << intervalStart << ", " << intervalEnd << "], intervalLen = " - << intervalLen << ", interpolation parameter = " << tau); + /* + * RJH: 2017-02-16 another exception thrown here when running under debug (and hence huge frame delays) + * the value of nextIt was already end(); which I think means that we cannot run the entire next section of code. + */ + if (nextIt != mMotionInfo.end()) { + double intervalLen = intervalEnd - intervalStart; + double tau = 0.0; + if (intervalLen != 0.0) tau = (tInterp - intervalStart) / intervalLen; - // Here we do just linear interpolation on the position - ecPos = interpolate(tau, prevIt->second.position, nextIt->second.position); - ecOrient = interpolate((float)tau, prevIt->second.orientation, - nextIt->second.orientation); - ecLinearVel = interpolate((float)tau, prevIt->second.linearVel, nextIt->second.linearVel); - speed = norm(ecLinearVel) * SG_METER_TO_NM * 3600.0; + SG_LOG(SG_AI, SG_DEBUG, "Multiplayer vehicle interpolation: [" + << intervalStart << ", " << intervalEnd << "], intervalLen = " + << intervalLen << ", interpolation parameter = " << tau); - if (prevIt->second.properties.size() - == nextIt->second.properties.size()) { - std::vector::const_iterator prevPropIt; - std::vector::const_iterator prevPropItEnd; - std::vector::const_iterator nextPropIt; - std::vector::const_iterator nextPropItEnd; - prevPropIt = prevIt->second.properties.begin(); - prevPropItEnd = prevIt->second.properties.end(); - nextPropIt = nextIt->second.properties.begin(); - nextPropItEnd = nextIt->second.properties.end(); - while (prevPropIt != prevPropItEnd) { - PropertyMap::iterator pIt = mPropertyMap.find((*prevPropIt)->id); - //cout << " Setting property..." << (*prevPropIt)->id; - - if (pIt != mPropertyMap.end()) - { - //cout << "Found " << pIt->second->getPath() << ":"; - - int ival; - float val; - /* - * RJH - 2017-01-25 - * During multiplayer operations a series of crashes were encountered that affected all players - * within range of each other and resulting in an exception being thrown at exactly the same moment in time - * (within case props::STRING: ref http://i.imgur.com/y6MBoXq.png) - * Investigation showed that the nextPropIt and prevPropIt were pointing to different properties - * which may be caused due to certain models that have overloaded mp property transmission and - * these craft have their properties truncated due to packet size. However the result of this - * will be different contents in the previous and current packets, so here we protect against - * this by only considering properties where the previous and next id are the same. - * It might be a better solution to search the previous and next lists to locate the matching id's - */ - if (*nextPropIt && (*nextPropIt)->id == (*prevPropIt)->id ) { - switch ((*prevPropIt)->type) { - case props::INT: - case props::BOOL: - case props::LONG: - ival = (int)(0.5 + (1 - tau)*((double)(*prevPropIt)->int_value) + - tau*((double)(*nextPropIt)->int_value)); - pIt->second->setIntValue(ival); - //cout << "Int: " << ival << "\n"; - break; - case props::FLOAT: - case props::DOUBLE: - val = (1 - tau)*(*prevPropIt)->float_value + - tau*(*nextPropIt)->float_value; - //cout << "Flo: " << val << "\n"; - pIt->second->setFloatValue(val); - break; - case props::STRING: - case props::UNSPECIFIED: - //cout << "Str: " << (*nextPropIt)->string_value << "\n"; - pIt->second->setStringValue((*nextPropIt)->string_value); - break; - default: - // FIXME - currently defaults to float values - val = (1 - tau)*(*prevPropIt)->float_value + - tau*(*nextPropIt)->float_value; - //cout << "Unk: " << val << "\n"; - pIt->second->setFloatValue(val); - break; - } - } - else - { - SG_LOG(SG_AI, SG_WARN, "MP packet mismatch during lag interpolation: " << (*prevPropIt)->id << " != " << (*nextPropIt)->id << "\n"); - } + // Here we do just linear interpolation on the position + ecPos = interpolate(tau, prevIt->second.position, nextIt->second.position); + ecOrient = interpolate((float)tau, prevIt->second.orientation, + nextIt->second.orientation); + ecLinearVel = interpolate((float)tau, prevIt->second.linearVel, nextIt->second.linearVel); + speed = norm(ecLinearVel) * SG_METER_TO_NM * 3600.0; + + if (prevIt->second.properties.size() + == nextIt->second.properties.size()) { + std::vector::const_iterator prevPropIt; + std::vector::const_iterator prevPropItEnd; + std::vector::const_iterator nextPropIt; + std::vector::const_iterator nextPropItEnd; + prevPropIt = prevIt->second.properties.begin(); + prevPropItEnd = prevIt->second.properties.end(); + nextPropIt = nextIt->second.properties.begin(); + nextPropItEnd = nextIt->second.properties.end(); + while (prevPropIt != prevPropItEnd) { + PropertyMap::iterator pIt = mPropertyMap.find((*prevPropIt)->id); + //cout << " Setting property..." << (*prevPropIt)->id; + + if (pIt != mPropertyMap.end()) + { + //cout << "Found " << pIt->second->getPath() << ":"; + + int ival; + float val; + /* + * RJH - 2017-01-25 + * During multiplayer operations a series of crashes were encountered that affected all players + * within range of each other and resulting in an exception being thrown at exactly the same moment in time + * (within case props::STRING: ref http://i.imgur.com/y6MBoXq.png) + * Investigation showed that the nextPropIt and prevPropIt were pointing to different properties + * which may be caused due to certain models that have overloaded mp property transmission and + * these craft have their properties truncated due to packet size. However the result of this + * will be different contents in the previous and current packets, so here we protect against + * this by only considering properties where the previous and next id are the same. + * It might be a better solution to search the previous and next lists to locate the matching id's + */ + if (*nextPropIt && (*nextPropIt)->id == (*prevPropIt)->id) { + switch ((*prevPropIt)->type) { + case props::INT: + case props::BOOL: + case props::LONG: + ival = (int)(0.5 + (1 - tau)*((double)(*prevPropIt)->int_value) + + tau*((double)(*nextPropIt)->int_value)); + pIt->second->setIntValue(ival); + //cout << "Int: " << ival << "\n"; + break; + case props::FLOAT: + case props::DOUBLE: + val = (1 - tau)*(*prevPropIt)->float_value + + tau*(*nextPropIt)->float_value; + //cout << "Flo: " << val << "\n"; + pIt->second->setFloatValue(val); + break; + case props::STRING: + case props::UNSPECIFIED: + //cout << "Str: " << (*nextPropIt)->string_value << "\n"; + pIt->second->setStringValue((*nextPropIt)->string_value); + break; + default: + // FIXME - currently defaults to float values + val = (1 - tau)*(*prevPropIt)->float_value + + tau*(*nextPropIt)->float_value; + //cout << "Unk: " << val << "\n"; + pIt->second->setFloatValue(val); + break; + } + } + else + { + SG_LOG(SG_AI, SG_WARN, "MP packet mismatch during lag interpolation: " << (*prevPropIt)->id << " != " << (*nextPropIt)->id << "\n"); + } + } + else + { + SG_LOG(SG_AI, SG_DEBUG, "Unable to find property: " << (*prevPropIt)->id << "\n"); + } + + ++prevPropIt; + ++nextPropIt; + } } - else - { - SG_LOG(SG_AI, SG_DEBUG, "Unable to find property: " << (*prevPropIt)->id << "\n"); - } - - ++prevPropIt; - ++nextPropIt; - } - } - // Now throw away too old data - if (prevIt != mMotionInfo.begin()) - { - --prevIt; - mMotionInfo.erase(mMotionInfo.begin(), prevIt); + // Now throw away too old data + if (prevIt != mMotionInfo.begin()) + { + --prevIt; + mMotionInfo.erase(mMotionInfo.begin(), prevIt); + } } } } else { From cbb7915e491ba07c4fe1091155661fa71b801959 Mon Sep 17 00:00:00 2001 From: Richard Harrison Date: Thu, 16 Feb 2017 18:30:17 +0100 Subject: [PATCH 3/3] Ensure to break out of the loop when an unrecognised incoming MP property id is encountered in a packet. --- src/MultiPlayer/multiplaymgr.cxx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/MultiPlayer/multiplaymgr.cxx b/src/MultiPlayer/multiplaymgr.cxx index c807b984d..823614571 100644 --- a/src/MultiPlayer/multiplaymgr.cxx +++ b/src/MultiPlayer/multiplaymgr.cxx @@ -1418,6 +1418,9 @@ FGMultiplayMgr::ProcessPosMsg(const FGMultiplayMgr::MsgBuf& Msg, SG_LOG(SG_NETWORK, SG_DEBUG, "FGMultiplayMgr::ProcessPosMsg - " "message from " << MsgHdr->Callsign << " has unknown property id " << id); + // At this point the packet must be considered to be unreadable + // as we have no way of knowing the length of this property (it could be a string) + break; } } noprops: