fix(net): xpSpent is a dword on the wire, and we were sending eight bytes
RaiseAttribute, RaiseVital, and RaiseSkill each wrote a 64-bit xpSpent, producing a 24-byte action where the server expects 20. ACE's GameActionRaiseAttribute and its Vital and Skill siblings read message.Payload.ReadUInt32(); holtburger's RaiseAttributeData declares xp_spent: u32 and advances the offset by four. Both oracles agree, and the four extra bytes were tail the server never reads. These three are live-wired, from the character sheet through the command router to SendRaiseAttribute, so this was shipping on every attribute, vital, and skill raise. It has not caused a visible failure because ACE reads the low dword and stops, and a single raise cost has never approached the dword ceiling. That is luck about value ranges, not correctness about layout. Worth noting the shape of the miss: the sibling builder BuildTrainSkill had already been corrected to a 20-byte, 32-bit credits field, and its test is even named U32CreditsNotU64. The same class of bug was found and fixed once in this file and the other three cases were left behind. The parameter stays ulong because the cost comes from 64-bit server XP tables several layers up in the App and Runtime command chain; narrowing that end to end is a separate change and is filed in the audit's open questions. Nothing is lost at the wire: a cost that does not fit in a dword was never expressible here. The existing test asserted the 24-byte shape and is corrected, joined by a theory that sweeps zero, one, a realistic cost, and uint.MaxValue across both remaining builders. Core.Net tests go 655 to 659. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
f416c577d6
commit
f57db35cec
2 changed files with 48 additions and 8 deletions
|
|
@ -16,9 +16,9 @@ public static class CharacterActions
|
||||||
{
|
{
|
||||||
public const uint GameActionEnvelope = 0xF7B1u;
|
public const uint GameActionEnvelope = 0xF7B1u;
|
||||||
|
|
||||||
public const uint RaiseAttributeOpcode = 0x0045u; // u32 attr, u64 xpSpent
|
public const uint RaiseAttributeOpcode = 0x0045u; // u32 attr, u32 xpSpent
|
||||||
public const uint RaiseVitalOpcode = 0x0044u; // u32 vital, u64 xpSpent
|
public const uint RaiseVitalOpcode = 0x0044u; // u32 vital, u32 xpSpent
|
||||||
public const uint RaiseSkillOpcode = 0x0046u; // u32 skillId, u64 xpSpent
|
public const uint RaiseSkillOpcode = 0x0046u; // u32 skillId, u32 xpSpent
|
||||||
public const uint TrainSkillOpcode = 0x0047u; // u32 skillId, u32 credits
|
public const uint TrainSkillOpcode = 0x0047u; // u32 skillId, u32 credits
|
||||||
public const uint ChangeCombatModeOpcode = 0x0053u; // u32 combatMode
|
public const uint ChangeCombatModeOpcode = 0x0053u; // u32 combatMode
|
||||||
|
|
||||||
|
|
@ -74,14 +74,31 @@ public static class CharacterActions
|
||||||
return body;
|
return body;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Envelope + sequence + sub-opcode + id + <b>32-bit</b> xpSpent.
|
||||||
|
///
|
||||||
|
/// <para>The xpSpent field is a dword on the wire, not a qword. ACE's
|
||||||
|
/// <c>GameAction/Actions/GameActionRaiseAttribute.cs</c> (and its Vital and
|
||||||
|
/// Skill siblings) read <c>message.Payload.ReadUInt32()</c>, and
|
||||||
|
/// holtburger's <c>RaiseAttributeData</c> declares <c>xp_spent: u32</c>
|
||||||
|
/// and advances the offset by four. We were writing eight, making the
|
||||||
|
/// message 24 bytes where the server expects 20 and leaving four bytes of
|
||||||
|
/// tail the server never reads.</para>
|
||||||
|
///
|
||||||
|
/// <para>The parameter stays <c>ulong</c> because the cost originates from
|
||||||
|
/// 64-bit server XP tables several layers up; narrowing that chain end to
|
||||||
|
/// end is a separate change. No value is lost here: a cost that does not
|
||||||
|
/// fit in a dword was never expressible on this wire in the first
|
||||||
|
/// place.</para>
|
||||||
|
/// </summary>
|
||||||
private static byte[] BuildAttrOrVital(uint seq, uint sub, uint id, ulong xp)
|
private static byte[] BuildAttrOrVital(uint seq, uint sub, uint id, ulong xp)
|
||||||
{
|
{
|
||||||
byte[] body = new byte[24];
|
byte[] body = new byte[20];
|
||||||
BinaryPrimitives.WriteUInt32LittleEndian(body, GameActionEnvelope);
|
BinaryPrimitives.WriteUInt32LittleEndian(body, GameActionEnvelope);
|
||||||
BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(4), seq);
|
BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(4), seq);
|
||||||
BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(8), sub);
|
BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(8), sub);
|
||||||
BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(12), id);
|
BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(12), id);
|
||||||
BinaryPrimitives.WriteUInt64LittleEndian(body.AsSpan(16), xp);
|
BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(16), (uint)xp);
|
||||||
return body;
|
return body;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -7,17 +7,40 @@ namespace AcDream.Core.Net.Tests.Messages;
|
||||||
|
|
||||||
public sealed class CharacterActionsTests
|
public sealed class CharacterActionsTests
|
||||||
{
|
{
|
||||||
|
/// <summary>
|
||||||
|
/// xpSpent is a dword, not a qword: ACE's GameActionRaiseAttribute reads
|
||||||
|
/// <c>ReadUInt32()</c> and holtburger's RaiseAttributeData declares
|
||||||
|
/// <c>xp_spent: u32</c>. This test previously asserted the 24-byte,
|
||||||
|
/// 64-bit shape — note that its sibling BuildTrainSkill_U32CreditsNotU64
|
||||||
|
/// had already been corrected to 20 bytes for the same class of mistake.
|
||||||
|
/// </summary>
|
||||||
[Fact]
|
[Fact]
|
||||||
public void BuildRaiseAttribute_HasOpcode0x0045AndXp64()
|
public void BuildRaiseAttribute_HasOpcode0x0045AndU32Xp()
|
||||||
{
|
{
|
||||||
byte[] body = CharacterActions.BuildRaiseAttribute(seq: 1, attrId: 5, xpSpent: 12345678);
|
byte[] body = CharacterActions.BuildRaiseAttribute(seq: 1, attrId: 5, xpSpent: 12345678);
|
||||||
Assert.Equal(24, body.Length);
|
Assert.Equal(20, body.Length);
|
||||||
Assert.Equal(CharacterActions.RaiseAttributeOpcode,
|
Assert.Equal(CharacterActions.RaiseAttributeOpcode,
|
||||||
BinaryPrimitives.ReadUInt32LittleEndian(body.AsSpan(8)));
|
BinaryPrimitives.ReadUInt32LittleEndian(body.AsSpan(8)));
|
||||||
Assert.Equal(5u,
|
Assert.Equal(5u,
|
||||||
BinaryPrimitives.ReadUInt32LittleEndian(body.AsSpan(12)));
|
BinaryPrimitives.ReadUInt32LittleEndian(body.AsSpan(12)));
|
||||||
Assert.Equal(12345678u,
|
Assert.Equal(12345678u,
|
||||||
BinaryPrimitives.ReadUInt64LittleEndian(body.AsSpan(16)));
|
BinaryPrimitives.ReadUInt32LittleEndian(body.AsSpan(16)));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData(0u)]
|
||||||
|
[InlineData(1u)]
|
||||||
|
[InlineData(12345678u)]
|
||||||
|
[InlineData(uint.MaxValue)]
|
||||||
|
public void RaiseVitalAndSkill_AlsoWriteU32Xp(uint xp)
|
||||||
|
{
|
||||||
|
byte[] vital = CharacterActions.BuildRaiseVital(seq: 1, vitalId: 1, xpSpent: xp);
|
||||||
|
byte[] skill = CharacterActions.BuildRaiseSkill(seq: 1, skillId: 8, xpSpent: xp);
|
||||||
|
|
||||||
|
Assert.Equal(20, vital.Length);
|
||||||
|
Assert.Equal(20, skill.Length);
|
||||||
|
Assert.Equal(xp, BinaryPrimitives.ReadUInt32LittleEndian(vital.AsSpan(16)));
|
||||||
|
Assert.Equal(xp, BinaryPrimitives.ReadUInt32LittleEndian(skill.AsSpan(16)));
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue