kwsch / PKHeX

Pokémon Save File Editor
https://projectpokemon.org/pkhex/
Other
3.72k stars 700 forks source link

Pokemon XD & Re-Rolling Non-Shadows #2751

Closed ijuintekka closed 4 years ago

ijuintekka commented 4 years ago

Describe the problem When Pokémon XD generates a team for an NPC, it can re-roll any of its non-shadow pokemon based on the player’s shiny value even though they are normally uncatchable by the player.

This can result in producing shadow pokemon with a PID/IV combination normally unreachable by re-rolling the shadow alone. PKHeX curently flags shadows generated in this manner as illegal.

To Reproduce For example, in the XD Mawile encounter:

An encounter seed of B2E6D1FE will generate the following pokemon and PIDs under normal circumstances:

Loudred: 4C3005E8 Girafarig: 8C917600 Mawile: F56635C5 (20,31,8,17,2,0)

or

Loudred: 4C3005E8 Girafarig: 8C917600 Mawile: E1FF26C2 (20,31,8,17,2,0) (re-rolled)

But by the player having a shiny value that matches Girafarig, 8018 in this case, it will instead re-roll that pokemon, skipping over the other results.

Loudred: 4C3005E8 Girafarig: D28DE40E (re-rolled 64 times to next viable match) Mawile: 049F2F05 (31,30,29,23,27,31)

The re-rolled Girafarig and Mawile cannot normally be reached and so aren’t checked for. 303 - MAWILE - 8E25049F2F05.zip

kwsch commented 4 years ago

Here's the logic that unrolls from PIDIV -> origin seed: https://github.com/kwsch/PKHeX/blob/5d63235dee92236a77b5729abcdda2c8ebd29196/PKHeX.Core/Legality/RNG/Locks/TeamLockResult.cs#L137-L159

TSV is only set when it's searching an XD mon, so maybe just changing:

if (current.Shadow && sv == TSV) // shiny shadow mon, only ever true for XD.
{
    // This interrupt is ignored! The result is shiny.
}

to

if (sv == TSV) // XD shiny checks all opponent PKM, even non-shadow.
{
    // This interrupt is ignored! The result is shiny.
}

That should cover the behavior you've seen, and should resolve the issue...

ijuintekka commented 4 years ago

This fixes the issue in generation 3-5 saves, but the "Invalid Encounter Type PID" error remains when the pokemon is loaded in generation 6, 7 and 8 saves. Perhaps because the shiny value is calculated differently? tests1

kwsch commented 4 years ago

Yeah.

https://github.com/kwsch/PKHeX/blob/f54bc7bf6fa665688c27e807544dabb3bf1f022a/PKHeX.Core/Legality/RNG/Locks/LockFinder.cs#L13

https://github.com/kwsch/PKHeX/blob/c301ce88ab42269420edabe5a90725c2f072ac8c/PKHeX.Core/PKM/PK5.cs#L269 https://github.com/kwsch/PKHeX/blob/c301ce88ab42269420edabe5a90725c2f072ac8c/PKHeX.Core/PKM/Shared/G6PKM.cs#L38 https://github.com/kwsch/PKHeX/blob/c301ce88ab42269420edabe5a90725c2f072ac8c/PKHeX.Core/PKM/PK8.cs#L82

I think just inlining the logic to do the (a^b)>>3 in LockFinder.cs would resolve the behavior.

Somewhat related: I wonder if there are other calls to TSV that are sensitive like this (I don't think so, but I wouldn't be surprised). Edit: found one, but it's checking the correlation using the >>4 format which is fine. https://github.com/kwsch/PKHeX/blob/9c28ffacc0e79d6cc6962442bf959bc384912688/PKHeX.Core/Legality/Verifiers/PIDVerifier.cs#L116

So this usage (CXD shadow locks) is the only place where this is an issue!

edit2 note to self: can probably remove this check, as we already calc it when building up the team list - https://github.com/kwsch/PKHeX/blob/5d63235dee92236a77b5729abcdda2c8ebd29196/PKHeX.Core/Legality/RNG/Locks/TeamLockResult.cs#L207-L209

ijuintekka commented 4 years ago

Wonderful! Thank you very much for looking at this so quickly.