refactor: extract buildForceAck from 5 duplicated force-ACK blocks

All five force-ACK handlers (speed, root, flag, collision-height,
knockback) repeated the same ~25-line GUID+counter+movementInfo+coord-
conversion+send sequence. Extracted into buildForceAck() which returns
a ready-to-send packet with the movement payload already written.

This also fixes a transport coordinate conversion bug: the collision-
height handler was the only one that omitted the ONTRANSPORT check,
causing position desync when riding boats/zeppelins. buildForceAck
handles transport coords uniformly for all callers.

Net -80 lines.
This commit is contained in:
Kelsi 2026-03-29 19:08:42 -07:00
parent 5fcb30be1a
commit 7462fdd41f
2 changed files with 53 additions and 157 deletions

View file

@ -179,6 +179,7 @@ private:
void handleOtherPlayerMovement(network::Packet& packet);
void handleMoveSetSpeed(network::Packet& packet);
void handleForceRunSpeedChange(network::Packet& packet);
network::Packet buildForceAck(Opcode ackOpcode, uint32_t counter);
void handleForceSpeedChange(network::Packet& packet, const char* name, Opcode ackOpcode, float* speedStorage);
void handleForceMoveRootState(network::Packet& packet, bool rooted);
void handleMoveKnockBack(network::Packet& packet);

View file

@ -762,6 +762,46 @@ void MovementHandler::dismount() {
// Force Speed / Root / Flag Change Handlers
// ============================================================
// Shared force-ACK packet builder. All server-forced movement changes (speed,
// root, flag, collision-height, knockback) require the same ACK structure:
// GUID + counter + movement payload with server-space coordinates. Centralised
// here so transport coordinate conversion can't diverge between handlers.
network::Packet MovementHandler::buildForceAck(Opcode ackOpcode, uint32_t counter) {
network::Packet ack(wireOpcode(ackOpcode));
const bool legacyGuid =
isActiveExpansion("classic") || isActiveExpansion("tbc") || isActiveExpansion("turtle");
if (legacyGuid) {
ack.writeUInt64(owner_.playerGuid);
} else {
ack.writePackedGuid(owner_.playerGuid);
}
ack.writeUInt32(counter);
MovementInfo wire = movementInfo;
wire.time = nextMovementTimestampMs();
if (wire.hasFlag(MovementFlags::ONTRANSPORT)) {
wire.transportTime = wire.time;
wire.transportTime2 = wire.time;
}
glm::vec3 serverPos = core::coords::canonicalToServer(glm::vec3(wire.x, wire.y, wire.z));
wire.x = serverPos.x;
wire.y = serverPos.y;
wire.z = serverPos.z;
if (wire.hasFlag(MovementFlags::ONTRANSPORT)) {
glm::vec3 serverTransport =
core::coords::canonicalToServer(glm::vec3(wire.transportX, wire.transportY, wire.transportZ));
wire.transportX = serverTransport.x;
wire.transportY = serverTransport.y;
wire.transportZ = serverTransport.z;
}
if (owner_.packetParsers_) {
owner_.packetParsers_->writeMovementPayload(ack, wire);
} else {
MovementPacket::writeMovementPayload(ack, wire);
}
return ack;
}
void MovementHandler::handleForceSpeedChange(network::Packet& packet, const char* name,
Opcode ackOpcode, float* speedStorage) {
const bool fscTbcLike = isClassicLikeExpansion() || isActiveExpansion("tbc");
@ -790,39 +830,7 @@ void MovementHandler::handleForceSpeedChange(network::Packet& packet, const char
}
if (owner_.socket) {
network::Packet ack(wireOpcode(ackOpcode));
const bool legacyGuidAck =
isActiveExpansion("classic") || isActiveExpansion("tbc") || isActiveExpansion("turtle");
if (legacyGuidAck) {
ack.writeUInt64(owner_.playerGuid);
} else {
ack.writePackedGuid(owner_.playerGuid);
}
ack.writeUInt32(counter);
MovementInfo wire = movementInfo;
wire.time = nextMovementTimestampMs();
if (wire.hasFlag(MovementFlags::ONTRANSPORT)) {
wire.transportTime = wire.time;
wire.transportTime2 = wire.time;
}
glm::vec3 serverPos = core::coords::canonicalToServer(glm::vec3(wire.x, wire.y, wire.z));
wire.x = serverPos.x;
wire.y = serverPos.y;
wire.z = serverPos.z;
if (wire.hasFlag(MovementFlags::ONTRANSPORT)) {
glm::vec3 serverTransport =
core::coords::canonicalToServer(glm::vec3(wire.transportX, wire.transportY, wire.transportZ));
wire.transportX = serverTransport.x;
wire.transportY = serverTransport.y;
wire.transportZ = serverTransport.z;
}
if (owner_.packetParsers_) {
owner_.packetParsers_->writeMovementPayload(ack, wire);
} else {
MovementPacket::writeMovementPayload(ack, wire);
}
auto ack = buildForceAck(ackOpcode, counter);
ack.writeFloat(newSpeed);
owner_.socket->send(ack);
}
@ -863,41 +871,9 @@ void MovementHandler::handleForceMoveRootState(network::Packet& packet, bool roo
}
if (!owner_.socket) return;
uint16_t ackWire = wireOpcode(rooted ? Opcode::CMSG_FORCE_MOVE_ROOT_ACK
: Opcode::CMSG_FORCE_MOVE_UNROOT_ACK);
if (ackWire == 0xFFFF) return;
network::Packet ack(ackWire);
const bool legacyGuidAck =
isActiveExpansion("classic") || isActiveExpansion("tbc") || isActiveExpansion("turtle");
if (legacyGuidAck) {
ack.writeUInt64(owner_.playerGuid);
} else {
ack.writePackedGuid(owner_.playerGuid);
}
ack.writeUInt32(counter);
MovementInfo wire = movementInfo;
wire.time = nextMovementTimestampMs();
if (wire.hasFlag(MovementFlags::ONTRANSPORT)) {
wire.transportTime = wire.time;
wire.transportTime2 = wire.time;
}
glm::vec3 serverPos = core::coords::canonicalToServer(glm::vec3(wire.x, wire.y, wire.z));
wire.x = serverPos.x;
wire.y = serverPos.y;
wire.z = serverPos.z;
if (wire.hasFlag(MovementFlags::ONTRANSPORT)) {
glm::vec3 serverTransport =
core::coords::canonicalToServer(glm::vec3(wire.transportX, wire.transportY, wire.transportZ));
wire.transportX = serverTransport.x;
wire.transportY = serverTransport.y;
wire.transportZ = serverTransport.z;
}
if (owner_.packetParsers_) owner_.packetParsers_->writeMovementPayload(ack, wire);
else MovementPacket::writeMovementPayload(ack, wire);
owner_.socket->send(ack);
Opcode ackOp = rooted ? Opcode::CMSG_FORCE_MOVE_ROOT_ACK : Opcode::CMSG_FORCE_MOVE_UNROOT_ACK;
if (wireOpcode(ackOp) == 0xFFFF) return;
owner_.socket->send(buildForceAck(ackOp, counter));
}
void MovementHandler::handleForceMoveFlagChange(network::Packet& packet, const char* name,
@ -922,40 +898,8 @@ void MovementHandler::handleForceMoveFlagChange(network::Packet& packet, const c
}
if (!owner_.socket) return;
uint16_t ackWire = wireOpcode(ackOpcode);
if (ackWire == 0xFFFF) return;
network::Packet ack(ackWire);
const bool legacyGuidAck =
isActiveExpansion("classic") || isActiveExpansion("tbc") || isActiveExpansion("turtle");
if (legacyGuidAck) {
ack.writeUInt64(owner_.playerGuid);
} else {
ack.writePackedGuid(owner_.playerGuid);
}
ack.writeUInt32(counter);
MovementInfo wire = movementInfo;
wire.time = nextMovementTimestampMs();
if (wire.hasFlag(MovementFlags::ONTRANSPORT)) {
wire.transportTime = wire.time;
wire.transportTime2 = wire.time;
}
glm::vec3 serverPos = core::coords::canonicalToServer(glm::vec3(wire.x, wire.y, wire.z));
wire.x = serverPos.x;
wire.y = serverPos.y;
wire.z = serverPos.z;
if (wire.hasFlag(MovementFlags::ONTRANSPORT)) {
glm::vec3 serverTransport =
core::coords::canonicalToServer(glm::vec3(wire.transportX, wire.transportY, wire.transportZ));
wire.transportX = serverTransport.x;
wire.transportY = serverTransport.y;
wire.transportZ = serverTransport.z;
}
if (owner_.packetParsers_) owner_.packetParsers_->writeMovementPayload(ack, wire);
else MovementPacket::writeMovementPayload(ack, wire);
owner_.socket->send(ack);
if (wireOpcode(ackOpcode) == 0xFFFF) return;
owner_.socket->send(buildForceAck(ackOpcode, counter));
}
void MovementHandler::handleMoveSetCollisionHeight(network::Packet& packet) {
@ -972,28 +916,11 @@ void MovementHandler::handleMoveSetCollisionHeight(network::Packet& packet) {
if (guid != owner_.playerGuid) return;
if (!owner_.socket) return;
uint16_t ackWire = wireOpcode(Opcode::CMSG_MOVE_SET_COLLISION_HGT_ACK);
if (ackWire == 0xFFFF) return;
network::Packet ack(ackWire);
const bool legacyGuidAck = isActiveExpansion("classic") || isActiveExpansion("tbc") || isActiveExpansion("turtle");
if (legacyGuidAck) {
ack.writeUInt64(owner_.playerGuid);
} else {
ack.writePackedGuid(owner_.playerGuid);
}
ack.writeUInt32(counter);
MovementInfo wire = movementInfo;
wire.time = nextMovementTimestampMs();
glm::vec3 serverPos = core::coords::canonicalToServer(glm::vec3(wire.x, wire.y, wire.z));
wire.x = serverPos.x;
wire.y = serverPos.y;
wire.z = serverPos.z;
if (owner_.packetParsers_) owner_.packetParsers_->writeMovementPayload(ack, wire);
else MovementPacket::writeMovementPayload(ack, wire);
if (wireOpcode(Opcode::CMSG_MOVE_SET_COLLISION_HGT_ACK) == 0xFFFF) return;
// buildForceAck now handles transport coordinate conversion, fixing the
// previous omission that caused desync when riding boats/zeppelins.
auto ack = buildForceAck(Opcode::CMSG_MOVE_SET_COLLISION_HGT_ACK, counter);
ack.writeFloat(height);
owner_.socket->send(ack);
}
@ -1020,40 +947,8 @@ void MovementHandler::handleMoveKnockBack(network::Packet& packet) {
}
if (!owner_.socket) return;
uint16_t ackWire = wireOpcode(Opcode::CMSG_MOVE_KNOCK_BACK_ACK);
if (ackWire == 0xFFFF) return;
network::Packet ack(ackWire);
const bool legacyGuidAck =
isActiveExpansion("classic") || isActiveExpansion("tbc") || isActiveExpansion("turtle");
if (legacyGuidAck) {
ack.writeUInt64(owner_.playerGuid);
} else {
ack.writePackedGuid(owner_.playerGuid);
}
ack.writeUInt32(counter);
MovementInfo wire = movementInfo;
wire.time = nextMovementTimestampMs();
if (wire.hasFlag(MovementFlags::ONTRANSPORT)) {
wire.transportTime = wire.time;
wire.transportTime2 = wire.time;
}
glm::vec3 serverPos = core::coords::canonicalToServer(glm::vec3(wire.x, wire.y, wire.z));
wire.x = serverPos.x;
wire.y = serverPos.y;
wire.z = serverPos.z;
if (wire.hasFlag(MovementFlags::ONTRANSPORT)) {
glm::vec3 serverTransport =
core::coords::canonicalToServer(glm::vec3(wire.transportX, wire.transportY, wire.transportZ));
wire.transportX = serverTransport.x;
wire.transportY = serverTransport.y;
wire.transportZ = serverTransport.z;
}
if (owner_.packetParsers_) owner_.packetParsers_->writeMovementPayload(ack, wire);
else MovementPacket::writeMovementPayload(ack, wire);
owner_.socket->send(ack);
if (wireOpcode(Opcode::CMSG_MOVE_KNOCK_BACK_ACK) == 0xFFFF) return;
owner_.socket->send(buildForceAck(Opcode::CMSG_MOVE_KNOCK_BACK_ACK, counter));
}
// ============================================================