Sim/COB: avoid UB in float-to-short angle casts (fixes arm64/x86 desync)

[tomjn]

What

Casting a floating-point heading/pitch expression directly to short is undefined behaviour when the truncated value does not fit in short, and this is genuinely reached in the COB angle math: RAD2TAANG = COBSCALE_HALF / pi with COBSCALE_HALF = 32768, so heading * RAD2TAANG equals +-32768 at heading = +-pi — one past short's 32767 maximum.

This routes the conversion through int first (short(int(expr))) at every affected site, which is well-defined: float -> int truncates toward zero (the magnitudes here always fit int), then int -> short performs the intended 16-bit TA-angle wraparound.

Why it matters (determinism)

Because the direct float -> short is UB, arm64 and x86 produced different values at the angle extremes, which was observed as an arm64/x86 multiplayer desync.

This is synced simulation code, so the fix must not change results on the platform that was already correct. It doesn't: x86 already lowers short(float) as float -> int -> short (cvttss2si into a 32-bit register, then narrow), so making the int() step explicit leaves the x86 result unchanged while pinning arm64 (whose fcvtzs saturated differently) to that same value.

Sites

Fixed in rts/Sim/Units/Scripts/CobInstance.cpp:

• WindChanged (SetDirection)
• StartBuilding (heading + pitch)
• AimWeapon (heading + pitch)

Siblings checked and deliberately left untouched (not the same bug):

• UnitScript.cpp:1058 casts asin(...) * RAD2TAANG — range +-16384, always fits short, no UB.
• UnitScript.cpp:1095/1099 already route through int.
• CobThread.cpp has no such casts.

Testing

• Built engine-headless cleanly (CobInstance compiles into the sim).
• Verified by reasoning that the int() step reproduces x86's existing lowering, so the already-correct platform is unaffected.
• A true determinism check requires cross-arch runs; cross-arch sync validation is recommended before merge.

Origin / credit

The fix was discussed in PR #2991's review thread (credit to BambaDamba); it was never committed there. Implemented here from that description and verified independently.

AI disclosure

Implemented with Claude Code (Anthropic) from a written plan. Reasoning, scope decisions, and the build verification were reviewed by a human (per AI_POLICY.md).

[bruno-dasilva]

wtf float to short can be UB?? absolutely nutty

[sprunk]

This sounds like something that requires a comment. Perhaps add some sort of inline short FloatHeadingToShort(float)

[n-morales]

Floating point to integral conversions can be UB if the truncated value is not representable in the destination type.

It would be better IMO to limit the range on this value before converting to short and not do the float->int->short conversion because that is extremely confusing.

[sprunk]

I sort of agree with n-morales that it would be good to come up with something less confusing than daisy chaining static casts, but since a comment exists I'm not too unhappy about it.