mirror of
https://gitea.wildfiregames.com/0ad/0ad
synced 2026-06-16 05:13:58 -07:00
Don't send redundant PositionChanged messages
UnitAI constantly calls FaceTowardsTarget while attacking or gathering, broadcasting an MT_PositionChanged message each time even when the entity already faces the target. With many units fighting, that floods the message bus (range manager, obstruction, minimap, AI proxy, ...) every turn. This was attempted in080599442fby returning early from TurnTo, but MoveAndTurnTo relies on TurnTo to advertise the movement, so units walking in a straight line stopped advertising their position and LOS broke (#6844), which led to the revert in3fb7319df7. Instead, deduplicate in AdvertisePositionChanges itself: remember the data of the last message sent and skip the message when it wouldn't change anything. Since the comparison covers the whole message data, a movement with an unchanged angle is still advertised. Add a regression test covering both #7654 and the #6844 scenario. Fixes: #7654 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
917275d6cb
commit
fc6a908775
2 changed files with 129 additions and 12 deletions
|
|
@ -116,6 +116,14 @@ public:
|
|||
|
||||
bool m_EnabledMessageInterpolate;
|
||||
|
||||
// The data of the last CMessagePositionChanged sent, to avoid sending redundant
|
||||
// messages (e.g. when UnitAI turns an entity towards a target it already faces).
|
||||
// Derived state: it always matches the current position when messages are up to
|
||||
// date, so Deserialize recomputes it instead of serializing it.
|
||||
bool m_LastAdvertisedInWorld;
|
||||
entity_pos_t m_LastAdvertisedX, m_LastAdvertisedZ;
|
||||
entity_angle_t m_LastAdvertisedRotY;
|
||||
|
||||
static std::string GetSchema()
|
||||
{
|
||||
return
|
||||
|
|
@ -174,6 +182,11 @@ public:
|
|||
m_ActorFloating = false;
|
||||
|
||||
m_EnabledMessageInterpolate = false;
|
||||
|
||||
// Match the message that would be sent while out of the world.
|
||||
m_LastAdvertisedInWorld = false;
|
||||
m_LastAdvertisedX = m_LastAdvertisedZ = entity_pos_t::Zero();
|
||||
m_LastAdvertisedRotY = entity_angle_t::Zero();
|
||||
}
|
||||
|
||||
void Deinit() override
|
||||
|
|
@ -278,6 +291,13 @@ public:
|
|||
UpdateXZRotation();
|
||||
|
||||
UpdateMessageSubscriptions();
|
||||
|
||||
// No message is sent during deserialization (subscribers restore their own
|
||||
// state), so the last advertised data is the current position.
|
||||
m_LastAdvertisedInWorld = m_InWorld;
|
||||
m_LastAdvertisedX = m_InWorld ? m_X : entity_pos_t::Zero();
|
||||
m_LastAdvertisedZ = m_InWorld ? m_Z : entity_pos_t::Zero();
|
||||
m_LastAdvertisedRotY = m_InWorld ? m_RotY : entity_angle_t::Zero();
|
||||
}
|
||||
|
||||
void Deserialized()
|
||||
|
|
@ -905,7 +925,7 @@ private:
|
|||
* - m_X, m_Z
|
||||
* - m_RotY
|
||||
*/
|
||||
void AdvertisePositionChanges() const
|
||||
void AdvertisePositionChanges()
|
||||
{
|
||||
for (std::set<entity_id_t>::const_iterator it = m_Turrets.begin(); it != m_Turrets.end(); ++it)
|
||||
{
|
||||
|
|
@ -913,16 +933,28 @@ private:
|
|||
if (cmpPosition)
|
||||
cmpPosition->UpdateTurretPosition();
|
||||
}
|
||||
if (m_InWorld)
|
||||
{
|
||||
CMessagePositionChanged msg(GetEntityId(), true, m_X, m_Z, m_RotY);
|
||||
GetSimContext().GetComponentManager().PostMessage(GetEntityId(), msg);
|
||||
}
|
||||
else
|
||||
{
|
||||
CMessagePositionChanged msg(GetEntityId(), false, entity_pos_t::Zero(), entity_pos_t::Zero(), entity_angle_t::Zero());
|
||||
GetSimContext().GetComponentManager().PostMessage(GetEntityId(), msg);
|
||||
}
|
||||
|
||||
const entity_pos_t x = m_InWorld ? m_X : entity_pos_t::Zero();
|
||||
const entity_pos_t z = m_InWorld ? m_Z : entity_pos_t::Zero();
|
||||
const entity_angle_t rotY = m_InWorld ? m_RotY : entity_angle_t::Zero();
|
||||
|
||||
// Don't send a message if the advertised position didn't actually change,
|
||||
// e.g. when UnitAI keeps facing an entity towards a target it already faces
|
||||
// (#7654). The check covers the whole message data, not just the angle, so
|
||||
// MoveAndTurnTo with an unchanged angle still advertises the movement (#6844).
|
||||
if (m_InWorld == m_LastAdvertisedInWorld &&
|
||||
x == m_LastAdvertisedX &&
|
||||
z == m_LastAdvertisedZ &&
|
||||
rotY == m_LastAdvertisedRotY)
|
||||
return;
|
||||
|
||||
m_LastAdvertisedInWorld = m_InWorld;
|
||||
m_LastAdvertisedX = x;
|
||||
m_LastAdvertisedZ = z;
|
||||
m_LastAdvertisedRotY = rotY;
|
||||
|
||||
CMessagePositionChanged msg(GetEntityId(), m_InWorld, x, z, rotY);
|
||||
GetSimContext().GetComponentManager().PostMessage(GetEntityId(), msg);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
/* Copyright (C) 2025 Wildfire Games.
|
||||
/* Copyright (C) 2026 Wildfire Games.
|
||||
* This file is part of 0 A.D.
|
||||
*
|
||||
* 0 A.D. is free software: you can redistribute it and/or modify
|
||||
|
|
@ -58,6 +58,39 @@ public:
|
|||
}
|
||||
};
|
||||
|
||||
class MockPositionChangedListener : public IComponent
|
||||
{
|
||||
public:
|
||||
DEFAULT_MOCK_COMPONENT()
|
||||
|
||||
JS::HandleValue GetJSInstance() const override
|
||||
{
|
||||
return JS::UndefinedHandleValue;
|
||||
}
|
||||
|
||||
bool NewJSObject(const ScriptInterface&, JS::MutableHandleObject) const override
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
int m_Count = 0;
|
||||
bool m_LastInWorld = false;
|
||||
entity_pos_t m_LastX, m_LastZ;
|
||||
entity_angle_t m_LastRotY;
|
||||
|
||||
void HandleMessage(const CMessage& msg, bool /*global*/) override
|
||||
{
|
||||
if (msg.GetType() != MT_PositionChanged)
|
||||
return;
|
||||
const CMessagePositionChanged& msgData = static_cast<const CMessagePositionChanged&>(msg);
|
||||
++m_Count;
|
||||
m_LastInWorld = msgData.inWorld;
|
||||
m_LastX = msgData.x;
|
||||
m_LastZ = msgData.z;
|
||||
m_LastRotY = msgData.a;
|
||||
}
|
||||
};
|
||||
|
||||
class TestCmpPosition : public CxxTest::TestSuite
|
||||
{
|
||||
public:
|
||||
|
|
@ -213,6 +246,58 @@ public:
|
|||
TS_ASSERT_EQUALS(cmp->GetPosition(), fixedvec(100, 122, 200));
|
||||
}
|
||||
|
||||
void test_advertise_position_changes()
|
||||
{
|
||||
CXeromycesEngine xeromycesEngine;
|
||||
ComponentTestHelper test(*g_ScriptContext);
|
||||
|
||||
MockTerrain terrain;
|
||||
test.AddMock(SYSTEM_ENTITY, IID_Terrain, terrain);
|
||||
|
||||
ICmpPosition* cmp = test.Add<ICmpPosition>(CID_Position, "<Anchor>upright</Anchor><Altitude>0</Altitude><Floating>false</Floating>");
|
||||
|
||||
MockPositionChangedListener listener;
|
||||
test.GetSimContext().GetComponentManager().DynamicSubscriptionNonsync(MT_PositionChanged, &listener, true);
|
||||
|
||||
// Moving out of the world while already out of it doesn't send a message.
|
||||
cmp->MoveOutOfWorld();
|
||||
TS_ASSERT_EQUALS(listener.m_Count, 0);
|
||||
|
||||
// Entering the world does.
|
||||
cmp->JumpTo(entity_pos_t::FromInt(100), entity_pos_t::FromInt(200));
|
||||
TS_ASSERT_EQUALS(listener.m_Count, 1);
|
||||
TS_ASSERT(listener.m_LastInWorld);
|
||||
TS_ASSERT_EQUALS(listener.m_LastX, entity_pos_t::FromInt(100));
|
||||
TS_ASSERT_EQUALS(listener.m_LastZ, entity_pos_t::FromInt(200));
|
||||
|
||||
// Turning to a new angle sends a message.
|
||||
cmp->TurnTo(entity_angle_t::FromInt(1));
|
||||
TS_ASSERT_EQUALS(listener.m_Count, 2);
|
||||
TS_ASSERT_EQUALS(listener.m_LastRotY, entity_angle_t::FromInt(1));
|
||||
|
||||
// Turning to the angle the entity already faces doesn't (#7654).
|
||||
cmp->TurnTo(entity_angle_t::FromInt(1));
|
||||
TS_ASSERT_EQUALS(listener.m_Count, 2);
|
||||
|
||||
// Moving without turning still advertises the movement (#6844).
|
||||
cmp->MoveAndTurnTo(entity_pos_t::FromInt(110), entity_pos_t::FromInt(200), entity_angle_t::FromInt(1));
|
||||
TS_ASSERT_EQUALS(listener.m_Count, 3);
|
||||
TS_ASSERT_EQUALS(listener.m_LastX, entity_pos_t::FromInt(110));
|
||||
TS_ASSERT_EQUALS(listener.m_LastZ, entity_pos_t::FromInt(200));
|
||||
TS_ASSERT_EQUALS(listener.m_LastRotY, entity_angle_t::FromInt(1));
|
||||
|
||||
// Moving to the position the entity already has doesn't send a message.
|
||||
cmp->MoveTo(entity_pos_t::FromInt(110), entity_pos_t::FromInt(200));
|
||||
TS_ASSERT_EQUALS(listener.m_Count, 3);
|
||||
|
||||
// Leaving the world sends an out-of-world message, but only once.
|
||||
cmp->MoveOutOfWorld();
|
||||
TS_ASSERT_EQUALS(listener.m_Count, 4);
|
||||
TS_ASSERT(!listener.m_LastInWorld);
|
||||
cmp->MoveOutOfWorld();
|
||||
TS_ASSERT_EQUALS(listener.m_Count, 4);
|
||||
}
|
||||
|
||||
void test_serialize()
|
||||
{
|
||||
CXeromycesEngine xeromycesEngine;
|
||||
|
|
|
|||
Loading…
Reference in a new issue