From f86bb98ccdef11c56c305057273890ac52bb19d6 Mon Sep 17 00:00:00 2001 From: Jon Wolverton Date: Thu, 26 Aug 2021 17:33:29 -0400 Subject: [PATCH] Collection quest ignoring items (#13401) * update tests to check for issue #12664 * rewrite processCollectionQuest to allocate quest items proportionally Removes need to check for excess items, fixing issue #12664 * move test for this bug to the correct context section & remove redundant test * refactor processCollectionQuest more * tweak confusing comments --- test/api/unit/models/group.test.js | 40 ++++++++++-------- website/server/models/group.js | 66 +++++++++++++----------------- 2 files changed, 53 insertions(+), 53 deletions(-) diff --git a/test/api/unit/models/group.test.js b/test/api/unit/models/group.test.js index c681a63273..59efba1134 100644 --- a/test/api/unit/models/group.test.js +++ b/test/api/unit/models/group.test.js @@ -488,20 +488,6 @@ describe('Group Model', () => { expect(party.quest.progress.collect.soapBars).to.eq(5); }); - it('does not drop an item if not need when on a collection quest', async () => { - party.quest.key = 'dilatoryDistress1'; - party.quest.active = false; - await party.startQuest(questLeader); - party.quest.progress.collect.fireCoral = 20; - await party.save(); - - await Group.processQuestProgress(participatingMember, progress); - - party = await Group.findOne({ _id: party._id }); - - expect(party.quest.progress.collect.fireCoral).to.eq(20); - }); - it('sends a chat message about progress', async () => { await Group.processQuestProgress(participatingMember, progress); @@ -538,8 +524,8 @@ describe('Group Model', () => { }); }); - describe('collection quests with multiple items', () => { - it('sends a chat message if no progress is made on quest with multiple items', async () => { + describe('collection quests with multiple item types', () => { + it('sends a chat message if no progress is made', async () => { progress.collectedItems = 0; party.quest.key = 'dilatoryDistress1'; party.quest.active = false; @@ -609,6 +595,28 @@ describe('Group Model', () => { }); }); + it('does not drop items when an item type becomes full', async () => { + progress.collectedItems = 20; + party.quest.key = 'dilatoryDistress1'; + party.quest.active = false; + + await party.startQuest(questLeader); + party.quest.progress.collect.fireCoral = 19; + await party.save(); + + await Group.processQuestProgress(participatingMember, progress); + + party = await Group.findOne({ _id: party._id }); + + // There is a very small chance (~1 in 500K) that blueFins will + // be 19 or 20 due to randomness and not any bug. In these cases, this + // test doesn't actually verify anything, but it's rare enough that it + // shouldn't be a problem, and to make it deterministic would require + // stubbing out methods in implementation-specific ways. + expect(party.quest.progress.collect.fireCoral).to.be.within(19, 20); + expect(party.quest.progress.collect.blueFins).to.be.within(19, 20); + }); + it('sends message about victory', async () => { progress.collectedItems = 500; diff --git a/website/server/models/group.js b/website/server/models/group.js index 76fcbec1de..38b913c139 100644 --- a/website/server/models/group.js +++ b/website/server/models/group.js @@ -1141,32 +1141,26 @@ schema.methods._processCollectionQuest = async function processCollectionQuest ( const group = this; const quest = questScrolls[group.quest.key]; const itemsFound = {}; - - const possibleItemKeys = Object.keys(quest.collect) - .filter(key => group.quest.progress.collect[key] < quest.collect[key].count); - - const possibleItemsToCollect = possibleItemKeys.reduce((accumulator, current, index) => { - accumulator[possibleItemKeys[index]] = quest.collect[current]; - return accumulator; - }, {}); - - _.times(progress.collectedItems, () => { - const item = shared.randomVal(possibleItemsToCollect, { key: true }); - - if (group.quest.progress.collect[item] < quest.collect[item].count) { - if (!itemsFound[item]) { - itemsFound[item] = 0; - } - itemsFound[item] += 1; - group.quest.progress.collect[item] += 1; - } + Object.keys(quest.collect).forEach(item => { + itemsFound[item] = 0; }); - // Add 0 for all items not found - Object.keys(this.quest.progress.collect).forEach(item => { - if (!itemsFound[item]) { - itemsFound[item] = 0; + // Create an array of item names, one item name per item that still needs to + // be collected so that items are found proportionally to how many are needed. + const remainingItems = [].concat(...Object.keys(quest.collect).map(item => { + let count = quest.collect[item].count - (group.quest.progress.collect[item] || 0); + if (count < 0) { // This could only happen if there's a bug, but just in case. + count = 0; } + return Array(count).fill(item); + })); + + // slice() will grab only what is available even if requested slice is larger + // than the array, so we don't need to worry about overfilling quest items. + const collectedItems = _.shuffle(remainingItems).slice(0, progress.collectedItems); + collectedItems.forEach(item => { + itemsFound[item] += 1; + group.quest.progress.collect[item] += 1; }); let foundText = _.reduce(itemsFound, (m, v, k) => { @@ -1186,23 +1180,21 @@ schema.methods._processCollectionQuest = async function processCollectionQuest ( }); group.markModified('quest.progress.collect'); - // Still needs completing - const needsCompleted = _.find(quest.collect, (v, k) => group.quest.progress.collect[k] < v.count); + const promises = [group.save(), foundChat.save()]; - if (needsCompleted) { - return Promise.all([group.save(), foundChat.save()]); + const questFinished = collectedItems.length === remainingItems.length; + if (questFinished) { + await group.finishQuest(quest); + const allItemsFoundChat = group.sendChat({ + message: `\`${shared.i18n.t('chatItemQuestFinish', 'en')}\``, + info: { + type: 'all_items_found', + }, + }); + + promises.push(allItemsFoundChat.save()); } - await group.finishQuest(quest); - const allItemsFoundChat = group.sendChat({ - message: `\`${shared.i18n.t('chatItemQuestFinish', 'en')}\``, - info: { - type: 'all_items_found', - }, - }); - - const promises = [group.save(), foundChat.save(), allItemsFoundChat.save()]; - return Promise.all(promises); };