Fix the OOS reported by elexis in #3335, and clean some whitespace and style.

The list of modified entities was thrown out on global visibility
updates (those happen on rejoin) but only in-world entities were
actually updated then, causing problems with garrisoning.

Now the list of modified entities can happen to be large, so replace the
hacky check for infinite loops by a real check.

This was SVN commit r16962.
This commit is contained in:
Nicolas Auvray 2015-08-30 17:42:10 +00:00
parent 45a39e7dfd
commit f47cb2c711
2 changed files with 22 additions and 24 deletions

View File

@ -284,14 +284,14 @@ GarrisonHolder.prototype.PerformGarrison = function(entity)
var cmpAura = Engine.QueryInterface(entity, IID_Auras);
if (cmpAura && cmpAura.HasGarrisonAura())
cmpAura.ApplyGarrisonBonus(this.entity);
cmpAura.ApplyGarrisonBonus(this.entity);
Engine.PostMessage(this.entity, MT_GarrisonedUnitsChanged, { "added" : [entity], "removed": [] });
var cmpUnitAI = Engine.QueryInterface(entity, IID_UnitAI);
if (cmpUnitAI && cmpUnitAI.IsUnderAlert())
Engine.PostMessage(cmpUnitAI.GetAlertRaiser(), MT_UnitGarrisonedAfterAlert, {"holder": this.entity, "unit": entity});
return true;
};
@ -306,7 +306,7 @@ GarrisonHolder.prototype.Eject = function(entity, forced)
// Error: invalid entity ID, usually it's already been ejected
if (entityIndex == -1)
return false; // Fail
// Find spawning location
var cmpFootprint = Engine.QueryInterface(this.entity, IID_Footprint);
var cmpHealth = Engine.QueryInterface(this.entity, IID_Health);
@ -331,10 +331,10 @@ GarrisonHolder.prototype.Eject = function(entity, forced)
return false;
}
}
var cmpNewPosition = Engine.QueryInterface(entity, IID_Position);
this.entities.splice(entityIndex, 1);
for (var vgp of this.visibleGarrisonPoints)
{
if (vgp.entity != entity)
@ -357,15 +357,15 @@ GarrisonHolder.prototype.Eject = function(entity, forced)
var cmpAura = Engine.QueryInterface(entity, IID_Auras);
if (cmpAura && cmpAura.HasGarrisonAura())
cmpAura.RemoveGarrisonBonus(this.entity);
cmpAura.RemoveGarrisonBonus(this.entity);
cmpNewPosition.JumpTo(pos.x, pos.z);
cmpNewPosition.SetHeightOffset(0);
// TODO: what direction should they face in?
Engine.PostMessage(this.entity, MT_GarrisonedUnitsChanged, { "added" : [], "removed": [entity] });
return true;
};
@ -432,7 +432,7 @@ GarrisonHolder.prototype.PerformEject = function(entities, forced)
else
{
var cmpObstruction = Engine.QueryInterface(entity, IID_Obstruction);
failedRadius = cmpObstruction ? cmpObstruction.GetUnitRadius() : 0;
failedRadius = cmpObstruction ? cmpObstruction.GetUnitRadius() : 0;
}
}
}
@ -506,7 +506,7 @@ GarrisonHolder.prototype.UnloadAllByOwner = function(owner, forced)
if (cmpOwnership && cmpOwnership.GetOwner() == owner)
entities.push(entity);
}
return this.PerformEject(entities, forced);
};
@ -530,7 +530,7 @@ GarrisonHolder.prototype.UnloadAllOwn = function(forced)
if (cmpOwnership && cmpOwnership.GetOwner() == owner)
entities.push(entity);
}
return this.PerformEject(entities, forced);
};

View File

@ -1606,16 +1606,13 @@ public:
{
PROFILE("UpdateVisibilityData");
if (m_GlobalVisibilityUpdate)
m_ModifiedEntities.clear();
for (i32 n = 0; n < m_LosTilesPerSide * m_LosTilesPerSide; ++n)
{
for (player_id_t player = 1; player < MAX_LOS_PLAYER_ID + 1; ++player)
{
if (IsVisibilityDirty(m_DirtyVisibility[n], player) || m_GlobalPlayerVisibilityUpdate[player-1] == 1 || m_GlobalVisibilityUpdate)
{
for (auto& ent : m_LosTiles[n])
for (const entity_id_t& ent : m_LosTiles[n])
UpdateVisibility(ent, player);
}
}
@ -1625,16 +1622,17 @@ public:
std::fill(m_GlobalPlayerVisibilityUpdate.begin(), m_GlobalPlayerVisibilityUpdate.end(), 0);
m_GlobalVisibilityUpdate = false;
// Calling UpdateVisibility can modify m_ModifiedEntities, so be careful
// Infinite loops could be triggered by feedback between entities and their mirages
int check = 0;
// Calling UpdateVisibility can modify m_ModifiedEntities, so be careful:
// infinite loops could be triggered by feedback between entities and their mirages.
std::map<entity_id_t, u8> attempts;
while (!m_ModifiedEntities.empty())
{
++check;
ENSURE(check < 1000 && "Possible infinite loop in UpdateVisibilityData");
entity_id_t ent = m_ModifiedEntities.back();
m_ModifiedEntities.pop_back();
++attempts[ent];
ENSURE(attempts[ent] < 100 && "Infinite loop in UpdateVisibilityData");
UpdateVisibility(ent);
}
}
@ -1656,7 +1654,7 @@ public:
if (oldVis == newVis)
return;
itEnts->second.visibilities = (itEnts->second.visibilities & ~(0x3 << 2 * (player - 1))) | (newVis << 2 * (player - 1));
CMessageVisibilityChanged msg(player, ent, oldVis, newVis);