diff --git a/source/simulation2/components/CCmpPosition.cpp b/source/simulation2/components/CCmpPosition.cpp index 5d05b4a012..9731ffc85a 100644 --- a/source/simulation2/components/CCmpPosition.cpp +++ b/source/simulation2/components/CCmpPosition.cpp @@ -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::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); } /** diff --git a/source/simulation2/components/tests/test_Position.h b/source/simulation2/components/tests/test_Position.h index 3c5ba2b52a..0b0c65f2e6 100644 --- a/source/simulation2/components/tests/test_Position.h +++ b/source/simulation2/components/tests/test_Position.h @@ -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(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(CID_Position, "upright0false"); + + 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;