diff --git a/Source/quests.cpp b/Source/quests.cpp index 1bc5d5e7cea..a0283d0018b 100644 --- a/Source/quests.cpp +++ b/Source/quests.cpp @@ -104,30 +104,6 @@ const char *const QuestTriggerNames[5] = { N_(/* TRANSLATORS: Quest Map*/ "A Dark Passage"), N_(/* TRANSLATORS: Quest Map*/ "Unholy Altar") }; -/** - * A quest group containing the three quests the Butcher, - * Ogden's Sign and Gharbad the Weak, which ensures that exactly - * two of these three quests appear in any single player game. - */ -int QuestGroup1[3] = { Q_BUTCHER, Q_LTBANNER, Q_GARBUD }; -/** - * A quest group containing the three quests Halls of the Blind, - * the Magic Rock and Valor, which ensures that exactly two of - * these three quests appear in any single player game. - */ -int QuestGroup2[3] = { Q_BLIND, Q_ROCK, Q_BLOOD }; -/** - * A quest group containing the three quests Black Mushroom, - * Zhar the Mad and Anvil of Fury, which ensures that exactly - * two of these three quests appear in any single player game. - */ -int QuestGroup3[3] = { Q_MUSHROOM, Q_ZHAR, Q_ANVIL }; -/** - * A quest group containing the two quests Lachdanan and Warlord - * of Blood, which ensures that exactly one of these two quests - * appears in any single player game. - */ -int QuestGroup4[2] = { Q_VEIL, Q_WARLORD }; /** * @brief There is no reason to run this, the room has already had a proper sector assigned @@ -274,7 +250,7 @@ void InitQuests() } if (!UseMultiplayerQuests() && *sgOptions.Gameplay.randomizeQuests) { - // Quests are set from the seed used to generate level 16. + // Quests are set from the seed used to generate level 15. InitialiseQuestPools(DungeonSeeds[15], Quests); } @@ -301,25 +277,22 @@ void InitialiseQuestPools(uint32_t seed, Quest quests[]) DiabloGenerator rng(seed); quests[rng.pickRandomlyAmong({ Q_SKELKING, Q_PWATER })]._qactive = QUEST_NOTAVAIL; - // using int and not size_t here to detect negative values from GenerateRnd - int randomIndex = rng.generateRnd(sizeof(QuestGroup1) / sizeof(*QuestGroup1)); - - if (randomIndex >= 0) - quests[QuestGroup1[randomIndex]]._qactive = QUEST_NOTAVAIL; - - randomIndex = rng.generateRnd(sizeof(QuestGroup2) / sizeof(*QuestGroup2)); - if (randomIndex >= 0) - quests[QuestGroup2[randomIndex]]._qactive = QUEST_NOTAVAIL; + if (seed == 988045466) { + // If someone starts a new game at 1977-12-28 19:44:42 UTC or 2087-02-18 22:43:02 UTC + // vanilla Diablo ends up reading QuestGroup1[-2] here. Due to the way the data segment + // is laid out (at least in 1.09) this ends up reading the address of the string + // "A Dark Passage" and trying to write to Quests[*8] which lands in read-only memory. + // The proper result would've been to mark The Butcher unavailable but instead nothing happens. + rng.discardRandomValues(1); + } else { + quests[rng.pickRandomlyAmong({ Q_BUTCHER, Q_LTBANNER, Q_GARBUD })]._qactive = QUEST_NOTAVAIL; + } - randomIndex = rng.generateRnd(sizeof(QuestGroup3) / sizeof(*QuestGroup3)); - if (randomIndex >= 0) - quests[QuestGroup3[randomIndex]]._qactive = QUEST_NOTAVAIL; + quests[rng.pickRandomlyAmong({ Q_BLIND, Q_ROCK, Q_BLOOD })]._qactive = QUEST_NOTAVAIL; - randomIndex = rng.generateRnd(sizeof(QuestGroup4) / sizeof(*QuestGroup4)); + quests[rng.pickRandomlyAmong({ Q_MUSHROOM, Q_ZHAR, Q_ANVIL })]._qactive = QUEST_NOTAVAIL; - // always true, QuestGroup4 has two members - if (randomIndex >= 0) - quests[QuestGroup4[randomIndex]]._qactive = QUEST_NOTAVAIL; + quests[rng.pickRandomlyAmong({ Q_VEIL, Q_WARLORD })]._qactive = QUEST_NOTAVAIL; } void CheckQuests() diff --git a/test/quests_test.cpp b/test/quests_test.cpp index 1aef016b867..42f3e36340b 100644 --- a/test/quests_test.cpp +++ b/test/quests_test.cpp @@ -31,16 +31,31 @@ TEST(QuestTest, SinglePlayerBadPools) EXPECT_EQ(Quests[Q_SKELKING]._qactive, QUEST_NOTAVAIL) << "Skeleton King quest is deactivated with 'bad' seed"; ResetQuests(); + // Having this seed for level 15 requires starting a game at 1977-12-28 07:44:42 PM or 2087-02-18 10:43:02 PM + // Given Diablo was released in 1996 and it's currently 2024 this will never have been naturally hit. It's not + // possible to hit this case on a pre-NT kernel windows system but it may be possible on macos or winnt? InitialiseQuestPools(988045466, Quests); EXPECT_THAT(GetActiveFlagsForSlice({ Q_BUTCHER, Q_LTBANNER, Q_GARBUD }), ::testing::Each(QUEST_INIT)) << "All quests in pool 2 remain active with 'bad' seed"; ResetQuests(); + // This seed can only be reached by editing a save file or modifying the game. Given quest state (including + // availability) is saved as part of the game state there's no vanilla compatibility concerns here. InitialiseQuestPools(4203210069U, Quests); - EXPECT_THAT(GetActiveFlagsForSlice({ Q_BLIND, Q_ROCK, Q_BLOOD }), ::testing::Each(QUEST_INIT)) << "All quests in pool 3 remain active with 'bad' seed"; + // If we wanted to retain the behaviour that vanilla Diablo would've done we should instead deactivate + // Quests[QuestGroup2[-2]]. This would hit QuestGroup1[1] (Ogden's Sign), however that's already marked + // as unavailable with this seed due to the previous quest group's roll. + EXPECT_EQ(Quests[Q_LTBANNER]._qactive, QUEST_NOTAVAIL) << "Ogden's Sign should be deactivated with 'bad' seed"; + EXPECT_EQ(Quests[Q_BLIND]._qactive, QUEST_NOTAVAIL) << "Halls of the Blind should also be deactivated with 'bad' seed"; ResetQuests(); + // This seed can only be reached by editing a save file or modifying the game. Given quest state (including + // availability) is saved as part of the game state there's no vanilla compatibility concerns here. InitialiseQuestPools(2557708932U, Quests); - EXPECT_THAT(GetActiveFlagsForSlice({ Q_MUSHROOM, Q_ZHAR, Q_ANVIL }), ::testing::Each(QUEST_INIT)) << "All quests in pool 4 remain active with 'bad' seed"; + // If we wanted to retain the behaviour that vanilla Diablo would've done we should instead deactivate + // Quests[QuestGroup3[-2]]. This would hit QuestGroup2[1] (Magic Rock), however that's already marked + // as unavailable with this seed due to the previous quest group's roll. + EXPECT_EQ(Quests[Q_ROCK]._qactive, QUEST_NOTAVAIL) << "Magic Rock should be deactivated with 'bad' seed"; + EXPECT_EQ(Quests[Q_MUSHROOM]._qactive, QUEST_NOTAVAIL) << "Black Mushroom should also be deactivated with 'bad' seed"; ResetQuests(); InitialiseQuestPools(1272442071, Quests);