diff --git a/Pkmds.Core/Extensions/SaveFileExtensions.cs b/Pkmds.Core/Extensions/SaveFileExtensions.cs index 2ff3253c..d8b95aec 100644 --- a/Pkmds.Core/Extensions/SaveFileExtensions.cs +++ b/Pkmds.Core/Extensions/SaveFileExtensions.cs @@ -19,6 +19,16 @@ public static class SaveFileExtensions /// public static void CompactParty(this SaveFile sav) { + // LGPE (SAV7b) stores the party as a pointer list into unified storage, maintained by + // PKHeX itself — there are no interstitial gaps to remove. Such saves can also report a + // PartyCount larger than the number of populated pointers; reading or writing a slot whose + // pointer is the SLOT_EMPTY sentinel throws ArgumentOutOfRangeException (issues #942–#948). + // Nothing to compact, so leave it alone. + if (sav is SAV7b) + { + return; + } + var nonBlank = new List(PartySize); for (var i = 0; i < PartySize; i++) { @@ -40,6 +50,92 @@ public static void CompactParty(this SaveFile sav) } } + /// + /// Reads a party slot, returning instead of throwing when the slot + /// cannot be read. Mainline saves return a blank Pokémon for empty slots, but LGPE (SAV7b) + /// stores the party as a pointer list and throws + /// when the pointer is the SLOT_EMPTY sentinel — which happens on saves whose party-count + /// field over-reports the number of populated slots (issues #942–#948). + /// + public static PKM? TryGetPartySlot(this SaveFile sav, int index) + { + if (index < 0 || index >= PartySize) + { + return null; + } + + try + { + return sav.GetPartySlotAtIndex(index); + } + catch (ArgumentOutOfRangeException) + { + // LGPE pointer-list slot points at the SLOT_EMPTY sentinel; treat as no Pokémon. + return null; + } + } + + /// + /// The number of leading party slots that can actually be read without throwing. For most + /// saves this equals . For LGPE (SAV7b) the stored count can + /// exceed the number of populated pointers, so this walks the reported slots and stops at the + /// first one that is unreadable or empty. + /// + public static int GetSafePartyCount(this SaveFile sav) + { + var reported = sav.PartyCount; + if (sav is not SAV7b) + { + return reported; + } + + var count = 0; + for (var i = 0; i < reported && i < PartySize; i++) + { + if (sav.TryGetPartySlot(i) is not { Species: > 0 }) + { + break; + } + + count++; + } + + return count; + } + + /// + /// Writes a Pokémon to a party slot, returning instead of throwing + /// when the slot cannot be written. On LGPE (SAV7b) a slot whose pointer is the SLOT_EMPTY + /// sentinel cannot be written through the index API (PKHeX dereferences the pointer first and + /// throws), so callers should treat as "this slot is not writable". + /// + public static bool TrySetPartySlot(this SaveFile sav, PKM pokemon, int index) + { + if (index < 0 || index >= PartySize) + { + return false; + } + + // On LGPE the slot must already point at a real storage entry; an empty pointer can't be + // written via SetPartySlotAtIndex (it throws while resolving the offset). + if (sav is SAV7b && sav.TryGetPartySlot(index) is not { Species: > 0 }) + { + return false; + } + + try + { + sav.SetPartySlotAtIndex(pokemon, index); + return true; + } + catch (ArgumentOutOfRangeException) + { + // Defensive: a malformed pointer list could still resolve to an out-of-range offset + // even after the guard above. Honor the "never throw" contract and report failure. + return false; + } + } + /// /// For Gen 1/2 saves — whose box storage is a packed list, not a grid — collects non-blank /// slots in and rewrites them contiguously starting at slot 0. No-op diff --git a/Pkmds.Rcl/Components/MainTabPages/EncounterDatabaseTab.razor.cs b/Pkmds.Rcl/Components/MainTabPages/EncounterDatabaseTab.razor.cs index 162dfb25..e4eeb59d 100644 --- a/Pkmds.Rcl/Components/MainTabPages/EncounterDatabaseTab.razor.cs +++ b/Pkmds.Rcl/Components/MainTabPages/EncounterDatabaseTab.razor.cs @@ -180,12 +180,16 @@ private async Task EnsureTargetSlotSelectedAsync() if (hasSelectedSlot) { - if (AppService.EditFormPokemon?.Species == 0) + // No occupant loaded (null) or an empty slot (Species 0) means there's nothing to + // overwrite — proceed without prompting. Guarding the null case fixes a + // NullReferenceException when generating into a slot with no loaded occupant, e.g. the + // first generate after loading a save with nothing selected (issue #949). + if (AppService.EditFormPokemon is not { Species: > 0 } occupant) { return true; } - var occupantName = SafeNameLookup.Species(AppService.EditFormPokemon!.Species); + var occupantName = SafeNameLookup.Species(occupant.Species); var confirmed = await DialogService.ShowMessageBoxAsync( "Overwrite Pokémon?", $"The selected slot contains {occupantName}. Overwrite it?", diff --git a/Pkmds.Rcl/Components/MainTabPages/TrainerInfoTab.razor.cs b/Pkmds.Rcl/Components/MainTabPages/TrainerInfoTab.razor.cs index 24a861fc..791d5f3d 100644 --- a/Pkmds.Rcl/Components/MainTabPages/TrainerInfoTab.razor.cs +++ b/Pkmds.Rcl/Components/MainTabPages/TrainerInfoTab.razor.cs @@ -538,8 +538,8 @@ private static IEnumerable GetExtraCurrencies(SaveFile saveF uint.MaxValue); break; - // Gen 9 SV / ZA currencies live inside their respective TrainerInfoSav9*Section - // components so they sit next to the rest of the gen-specific fields. + // Gen 9 SV / ZA currencies live inside their respective TrainerInfoSav9*Section + // components so they sit next to the rest of the gen-specific fields. } } diff --git a/Pkmds.Rcl/Components/PartyGrid.razor b/Pkmds.Rcl/Components/PartyGrid.razor index d7e1430d..52a95297 100644 --- a/Pkmds.Rcl/Components/PartyGrid.razor +++ b/Pkmds.Rcl/Components/PartyGrid.razor @@ -8,8 +8,10 @@ @for (var i = 0; i < 6; i++) { var slotNum = i; + @* LGPE (SAV7b) saves can report more party slots than are populated; reading a + phantom slot throws (issues #942–#948). TryGetPartySlot returns null instead. *@ var pkm = i < saveFile.PartyCount - ? saveFile.PartyData[slotNum] + ? saveFile.TryGetPartySlot(slotNum) : null; 0, IsEgg: false }) { count++; @@ -519,9 +520,21 @@ private async Task HandleFileDropAsync(string[] fileNames) // leaving a gap. if (IsPartySlot) { - saveFile.SetPartySlotAtIndex(pokemon, SlotNumber); - saveFile.CompactParty(); - RefreshService.RefreshPartyState(); + // TrySetPartySlot returns false for an LGPE (SAV7b) party slot whose pointer is the + // SLOT_EMPTY sentinel — writing there throws in PKHeX and corrupts the in-memory + // party count (issues #942–#948). Let's Go has no standalone party buffer, so an + // empty party slot can't be written through the index API. + if (saveFile.TrySetPartySlot(pokemon, SlotNumber)) + { + saveFile.CompactParty(); + RefreshService.RefreshPartyState(); + } + else + { + Snackbar.Add( + "That party slot can't be edited for Let's Go saves. Use the box instead.", + Severity.Warning); + } } else if (BoxNumber.HasValue) { diff --git a/Pkmds.Rcl/Services/AppService.cs b/Pkmds.Rcl/Services/AppService.cs index 65ed44c4..0e770b06 100644 --- a/Pkmds.Rcl/Services/AppService.cs +++ b/Pkmds.Rcl/Services/AppService.cs @@ -265,9 +265,19 @@ public void SavePokemon(PKM? pokemon) switch (selectedPokemonType) { case SelectedPokemonType.Party: - AppState.SaveFile.SetPartySlotAtIndex(pokemon, partySlot); + // TrySetPartySlot returns false for an unwritable LGPE (SAV7b) slot — one whose + // pointer is the SLOT_EMPTY sentinel because the save over-reports its party count + // (issues #944–#948). Writing there throws in PKHeX, so skip rather than crash. + // Return early so we don't snapshot a "saved" baseline for a write that never + // happened — that would hide the user's unsaved changes. + if (!AppState.SaveFile.TrySetPartySlot(pokemon, partySlot)) + { + return; + } + // If the edited slot was past PartyCount (e.g. HaX mode editing an empty slot) - // the write would leave a gap; party is always a packed list, so compact. + // the write would leave a gap; party is always a packed list, so compact + // (no-op for LGPE). AppState.SaveFile.CompactParty(); // Let's Go games store Pokémon in a unified storage system @@ -820,54 +830,54 @@ private static Task ImportWB8ToBDSP(SAV8BS saveFile, WB8 gift, out bool isSucces return Task.CompletedTask; case WB8.GiftType.Item: - { - var items = saveFile.Items; - var addedAny = false; - for (var i = 0; i < 7; i++) { - var itemId = (ushort)gift.GetItem(i); - var quantity = gift.GetQuantity(i); - if (itemId == 0) - break; - items.SetItemQuantity(itemId, items.GetItemQuantity(itemId) + quantity); - addedAny = true; + var items = saveFile.Items; + var addedAny = false; + for (var i = 0; i < 7; i++) + { + var itemId = (ushort)gift.GetItem(i); + var quantity = gift.GetQuantity(i); + if (itemId == 0) + break; + items.SetItemQuantity(itemId, items.GetItemQuantity(itemId) + quantity); + addedAny = true; + } + isSuccessful = addedAny; + resultsMessage = addedAny + ? "The Mystery Gift items have been added to your bag." + : "No items found in this Mystery Gift."; + return Task.CompletedTask; } - isSuccessful = addedAny; - resultsMessage = addedAny - ? "The Mystery Gift items have been added to your bag." - : "No items found in this Mystery Gift."; - return Task.CompletedTask; - } default: - { - // For Clothing, Money, BP, etc.: populate a RecvData8b slot in MysteryBlock8b. - // The in-game Pokémon Delivery Man reads these slots to grant gifts. - var records = saveFile.MysteryRecords; - var emptySlot = -1; - for (var i = 0; i < MysteryBlock8b.RecvDataMax; i++) { - if (records.GetReceived(i).Ticks == 0) + // For Clothing, Money, BP, etc.: populate a RecvData8b slot in MysteryBlock8b. + // The in-game Pokémon Delivery Man reads these slots to grant gifts. + var records = saveFile.MysteryRecords; + var emptySlot = -1; + for (var i = 0; i < MysteryBlock8b.RecvDataMax; i++) { - emptySlot = i; - break; + if (records.GetReceived(i).Ticks == 0) + { + emptySlot = i; + break; + } } - } - if (emptySlot == -1) - { - isSuccessful = false; - resultsMessage = "No empty Mystery Gift slots available in the save file."; + if (emptySlot == -1) + { + isSuccessful = false; + resultsMessage = "No empty Mystery Gift slots available in the save file."; + return Task.CompletedTask; + } + var slot = records.GetReceived(emptySlot); + slot.Timestamp = DateTime.UtcNow; + slot.DeliveryID = (ushort)gift.CardID; + slot.DataType = (byte)gift.CardType; + slot.TextID = (ushort)gift.CardTitleIndex; + isSuccessful = true; + resultsMessage = "The Mystery Gift has been queued. Visit the Pokémon Delivery Man to claim it."; return Task.CompletedTask; } - var slot = records.GetReceived(emptySlot); - slot.Timestamp = DateTime.UtcNow; - slot.DeliveryID = (ushort)gift.CardID; - slot.DataType = (byte)gift.CardType; - slot.TextID = (ushort)gift.CardTitleIndex; - isSuccessful = true; - resultsMessage = "The Mystery Gift has been queued. Visit the Pokémon Delivery Man to claim it."; - return Task.CompletedTask; - } } } diff --git a/Pkmds.Tests/LgpePartySafetyTests.cs b/Pkmds.Tests/LgpePartySafetyTests.cs new file mode 100644 index 00000000..eb2255d4 --- /dev/null +++ b/Pkmds.Tests/LgpePartySafetyTests.cs @@ -0,0 +1,83 @@ +namespace Pkmds.Tests; + +/// +/// Regression tests for LGPE (SAV7b) party handling. Let's Go stores the party as a pointer list +/// into unified storage; an empty slot holds the SLOT_EMPTY sentinel, and reading or writing such +/// a slot via the index API throws . Saves in the wild +/// (emulator / JKSM dumps) can report a larger than the number of +/// populated pointer slots, which crashed the party grid, the editor save path, and CompactParty +/// (issues #942–#948). The safe extension helpers must tolerate that state instead of throwing. +/// +public class LgpePartySafetyTests +{ + /// + /// Builds a Let's Go save whose reported party count exceeds the number of populated pointer + /// slots — the malformed state of the user-reported saves. The pointers stay at SLOT_EMPTY, so + /// every reported slot throws when read/written through PKHeX's index API. + /// + private static SAV7b CreateLgpeWithOverReportedParty(int reportedCount) + { + var sav = new SAV7b(); + sav.Storage.PartyCount = reportedCount; + return sav; + } + + [Fact] + public void RawGetPartySlotAtIndex_PhantomSlot_Throws() + { + // Documents the underlying PKHeX behavior the safe helpers shield callers from. + var sav = CreateLgpeWithOverReportedParty(2); + + Action act = () => sav.GetPartySlotAtIndex(0); + + act.Should().Throw(); + } + + [Fact] + public void TryGetPartySlot_PhantomSlot_ReturnsNullInsteadOfThrowing() + { + var sav = CreateLgpeWithOverReportedParty(2); + + sav.TryGetPartySlot(0).Should().BeNull(); + sav.TryGetPartySlot(5).Should().BeNull(); + } + + [Fact] + public void GetSafePartyCount_NoPopulatedSlots_ReturnsZero() + { + var sav = CreateLgpeWithOverReportedParty(6); + + sav.GetSafePartyCount().Should().Be(0); + } + + [Fact] + public void CompactParty_Lgpe_DoesNotThrow() + { + var sav = CreateLgpeWithOverReportedParty(6); + + Action act = sav.CompactParty; + + act.Should().NotThrow(); + } + + [Fact] + public void TrySetPartySlot_PhantomSlot_ReturnsFalseAndDoesNotThrow() + { + var sav = CreateLgpeWithOverReportedParty(2); + var pkm = new PB7 { Species = (ushort)Species.Pikachu }; + + var result = true; + Action act = () => result = sav.TrySetPartySlot(pkm, 1); + + act.Should().NotThrow(); + result.Should().BeFalse(); + } + + [Fact] + public void GetSafePartyCount_NonLgpeSave_EqualsPartyCount() + { + var sav = new SAV8SWSH(); + + sav.GetSafePartyCount().Should().Be(sav.PartyCount); + } +}