Bug: Link battle LCG is effectively only nine bytes

[SnowyMouse]

LCG is ten bytes, but only nine are actually ever used. The tenth byte is never read! As such, it is functionally identical to Gen 1's LCG.

I think it might be a holdover from Gen 1. In fact, the check to see if we've run out of bytes in the seed is actually the same code as pokered.

pokecrystal/engine/battle/core.asm

Lines 6903 to 6907 in 99af3f4

	cp 10 - 1 ; Exclude last value. See the closing comment
	ld a, [hl]
	pop bc
	pop hl
	ret c

One could assume that this tenth byte is used for cycling PRNG, but this is also not so. If you scroll down below, you'll see this code:

pokecrystal/engine/battle/core.asm

Lines 6915 to 6937 in 99af3f4

	; Reset count to 0
	xor a
	ld [wLinkBattleRNCount], a
	ld hl, wLinkBattleRNs
	ld b, 10 ; number of seeds
	; Generate next number in the sequence for each seed
	; a[n+1] = (a[n] * 5 + 1) % 256
	.loop
	; get last #
	ld a, [hl]
	; a * 5 + 1
	ld c, a
	add a
	add a
	add c
	inc a
	; update #
	ld [hli], a
	dec b
	jr nz, .loop

We do not use the value from the previous iteration, and hl is incremented at the end of the loop (hli increments hl after write, not before). As such, it works exactly the same as Gen 1's PRNG. In fact Gen 1's code is exactly the same, besides b being set to 10 instead of 9.

Therefore, the game generates a tenth byte but never uses it. I'm guessing the fix is probably to just change cp 10 - 1 to just cp 10 (or even better, cp SERIAL_RNS_LENGTH).

[Rangi42]

Gen 1 also has a 10-byte PRNG buffer (SERIAL_RNS_LENGTH is 10 there too). So one "fix" here could be to change the cp 10 - 1 to cp 10, but another could be to change the ld b, 10 to ld b, 9 like Gen 1 had. Also -- what would the consequence of either "fix" be? Does this cause an observable bug, e.g. repeated or inconsistent RNG values?

[SnowyMouse]

I think the better fix would be changing ld b, 10 to ld b, 9.

I only suggested using cp 10 because it looked like it might've been intentional by the developers to use a tenth byte for some bizarre reason. Gen 1-2's LCG is technically nine independent LCGs, and since it just uses random numbers as the seed, repeats are bound to happen, and not only that, but happen very predictably. Adding more bytes raises this probability.

Changing ld b, 10 to ld b, 9 will also not break cartridge compatibility (again, that 10th byte wasn't getting used!), so there is also points for that, too.