fix(eff_ob): the packet names an entity and switches an effect on or off

[denislauri1999]

The model has ExtraSpace, MapX, MapY, Unknown, EffectId. The client
reads something else.

Its handler takes the first character of field 0 as the entity kind, field 1
as that entity's id, field 2 as a switch, and field 3 as the effect:

A   := StrToIntDef(field[1], -1)        the entity id
c   := field[0][1]                      the kind - first character only
obj := <lookup>(c, A)
B   := StrToIntDef(field[2], 0)         the switch
E   := StrToIntDef(field[3], 0xFFFF)    the effect
if B = 1:  attach E to obj
if B = 0:  detach whatever obj is carrying
otherwise: nothing

No map coordinate is read anywhere in it.

What was tested

In game, with a positive control so the result cannot be a coincidence - effect
4430 is a ring of magenta arcs and it is unmistakable:

sent	seen
eff_ob 1 2 1 4430	the ring appears around the player
eff_ob 1 2 0 4430	nothing
eff_ob 79 115 0 4430	nothing
eff_ob -1 -1 0 4430	nothing
eff 1 2 4430 (control, other route)	the ring appears

dotnet build: 0 warnings. dotnet test: 127/127. The serializer corpus
snapshot was edited by hand, two lines, so the diff stays two lines.

What this contradicts, and why I still think it is right

SerializeEffObPacketMatchesTrace pinned eff_ob -1 -1 0 4269, and that line
really is in a packet capture - four times. I replaced the assertion rather than
keep it, so this is worth stating plainly.

That capture is of an older protocol. On the modern client the line does
nothing: the double space puts an empty field in front, so everything shifts by
one and field 2 reads -1, which is neither 1 nor 0, so no branch runs. Even
without the shift, -1 names no entity and the lookup fails first. It cannot be
made to draw anything, which is what the fourth row of the table above shows.

If the intent is to keep the model faithful to that capture rather than to the
client that reads it, say so and I will close this.

[coderabbitai]

Warning

Review limit reached

Next included review available in 50 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b870013f-54a4-427b-9931-96202fed8543

📥 Commits

Reviewing files that changed from the base of the PR and between 5caf2c1 and b17b324.

📒 Files selected for processing (3)

• src/NosCore.Packets/ServerPackets/Map/EffObPacket.cs
• test/NosCore.Packets.Tests/SerializerCorpusTests.ExpressionTreeSerializerOutputIsPinnedForEveryPacket.verified.txt
• test/NosCore.Packets.Tests/SerializerTest.cs

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}