Fix -WSign warning in battle_transition

[AZero13]

src/battle_transition.c:1199:16: warning: passing 'u16[960]' (aka 'unsigned short[960]') to parameter of type 's16 *' (aka 'short *') converts between pointers to integer types with different sign [-Wpointer-sign]
1199 | SetSinWave(gScanlineEffectRegBuffers[1], sTransitionData->cameraX, 0, 2, 0, 160);

[AZero13]

Ping

[ketsuban]

I feel like this PR lacks context. Can you explain why it's correct for the type of the first argument to SetSinWave to be u16 * rather than for the type of gScanlineEffectRegBuffers to be s16 **?

[AZero13]

I feel like this PR lacks context. Can you explain why it's correct for the type of the first argument to SetSinWave to be u16 * rather than for the type of gScanlineEffectRegBuffers to be s16 **?

Because the pointer we pass as the first argument is a u16 pointer. It just fixes a clang warning. Not sure if it is technically UB. But I feel like if clang is warning about it, it probably is.

[Kurausukun]

As far as I know, changing the type pointed to between signed and unsigned is not UB by itself, but dereferencing it would probably fall under conversion between signed and unsigned, where the typical rules apply: conversion from signed to unsigned is always well-defined, conversion from unsigned to signed is well-defined if the value can be represented by the signed type, otherwise it is implementation-defined.

[mrgriffin]

imo gScanlineEffectRegBuffers aren't really u16 or s16, they're a bunch of bytes that are interpreted based on the hardware registers they're DMAd to and thus could be u16s, s16s, u32s, s32s, or even something like struct { u16 _; s16_; } if you're doing 32-bit copies to a 16-bit unsigned register followed by a 16-bit signed register.

I suppose it's debatable whether any hardware registers are signed or not, but LCD I/O BG Rotation/Scaling are clearly 2s complement, which I'd say translates to signed in C on the GBA:

Normal signed 32bit values may be written to above registers.

I definitely don't think it makes sense to consider the output of SetSinWave as u16 (unless the sinAdd-amplitude combination makes all values >= 0, but that's not a precondition of this function afaict).