[Quest] A Generous General in IF #6598
Conversation
| ----------------------------------- | ||
| local quest = Quest:new(xi.questLog.OTHER_AREAS, xi.quest.id.otherAreas.A_GENEROUS_GENERAL) | ||
|
|
||
| local spawnedMobs = |
There was a problem hiding this comment.
This seems like we could procedurally generate this as opposed to defining a table, but would need to look into the function you're using to call this
There was a problem hiding this comment.
Something like
for i = 0, 6 do
table.insert(spawnedMobs, oldtonID.mob.GENEROUS_GENERAL_OFFSET + i)
end
I just saw other methods defining tables so I assumed that was the accepted pattern
There was a problem hiding this comment.
I think my issue here is that its only used once. I'm fine with it, but feel like the spawning logic could be simplified overall and iterated
There was a problem hiding this comment.
Gotcha - plopped this line in and got rid of the table at the top of the file, does that look good to you?
| player:getQuestStatus(xi.questLog.OTHER_AREAS, xi.quest.id.otherAreas.AN_AFFABLE_ADAMANTKING) ~= xi.questStatus.QUEST_ACCEPTED and | ||
| player:getQuestStatus(xi.questLog.OTHER_AREAS, xi.quest.id.otherAreas.AN_UNDERSTANDING_OVERLORD) ~= xi.questStatus.QUEST_ACCEPTED and | ||
| player:getQuestStatus(xi.questLog.OTHER_AREAS, xi.quest.id.otherAreas.A_MORAL_MANIFEST) ~= xi.questStatus.QUEST_ACCEPTED and | ||
| player:getVar('BstHeadGearQuest_Conquest') < NextConquestTally() and |
There was a problem hiding this comment.
If this variable is unique to this quest, then it should follow standard quest vars, at absolute minimum, it should be an expiring charvar with a check against 0
There was a problem hiding this comment.
All of the Beast headgear quests share the same time block of conquest tally, so if you start one headgear quest you can't start another unless you cancel the one you're currently on. Trying to find the source I saw that bit of info in though..
There was a problem hiding this comment.
(unless the quest is cancelled)
There was a problem hiding this comment.
Can't find the source I had seen this in so moving this over to a quest var
| function(player, prevZone) | ||
| local pos = player:getPos() | ||
| if | ||
| pos.x >= -323.0 and |
There was a problem hiding this comment.
These conditions are checked multiple times. Should move these coordinate checks to their own function for reusability
| if option == 0 then | ||
| quest:begin(player) -- begin quest | ||
| elseif option == 1 then | ||
| player:setVar('BstHeadGearQuest_Conquest', NextConquestTally()) -- if player declines they must wait until next conquest tally |
There was a problem hiding this comment.
See above comment, set it to 1 with an expiry of NextConquestTally()
There was a problem hiding this comment.
I had no idea this was a thing, that's awesome
| elseif prog == 1 then | ||
| return quest:event(771):importantOnce() -- reminder to get buffalo hide and sheep leather or cancel quest | ||
| elseif | ||
| prog >= 2 and |
There was a problem hiding this comment.
Loose condition here, what happens if partsWait == 0 and prog == 2?
There was a problem hiding this comment.
Hmm,
partsWait is set to VanaMidnight() at the same time (right after) prog is set to 2 with event 772.
partsWait is only set to 0 in event 775, if player prog is 2, then it is set to 3. So we don't hit partsWait == 0 and prog == 2.
Are you seeing a possibility of partsWait not being set to VanaMidnight or being reset to 0 that I might be missing?
| [771] = function(player, csid, option, npc) | ||
| if option == 100 then | ||
| player:messageSpecial(southernSanDoriaID.text.QUEST_CANCELLED) | ||
| quest:setVar(player, 'Prog', 0) -- reset progress |
There was a problem hiding this comment.
There are other quest vars here that may not be cleaned up, call quest:cleanup(player)
| if quest:complete(player) then | ||
| player:confirmTrade() | ||
| player:setVar('BstHeadGearQuest_Conquest', NextConquestTally()) | ||
| player:setCharVar('generousGeneralZone', 1) |
There was a problem hiding this comment.
This seems very much associated with this quest, and should be a questVar
a14e086 to
ce35daa
Compare
Co-authored-by: jamesbradleym <jamesbradleym@gmail.com> Co-authored-by: hookstar <carl.hooker@gmail.com>
ce35daa to
189f346
Compare
I affirm:
What does this pull request do?
Allows players to accept and complete the quest A Generous General.
https://www.youtube.com/watch?v=IG5QqGUeXXY
https://www.youtube.com/watch?v=d-2v1wO24ps
https://www.dropbox.com/s/wgz4mom3g8yuiy0/A%20Generous%20General%20Unlock%20and%20Turn%20In.zip?dl=0
AirSkyBoat#3497
Steps to test these changes
Zone in to Oldton Movalpolos from S Gusta (!pos 566 -12 685 106), with minimum level 60, and no other beastmen hat quest active. (If you decline, you must wait until the next conquest tally)
Go to Faulpie in S Sandoria to get Goblin Coif Cutting by trading in a Buffalo Hide, Sheep Leather, and 10,000 gil. Wait until next Vana day.
Return to Oldton Movalpolos from S Gusta (!pos 566 -12 685 106) with Goblin Coif equipped.
Go to Iron Box !pos -141 7 157 11
Kill Goblin Preceptor - Quest complete
Zone in and out of Oldton Movalpolos with Choplix's Coif equipped for additional CS and gold beast coin reward
Notes
Add'l Resources:
https://ffxiclopedia.fandom.com/wiki/A_Generous_General%3F
https://youtu.be/l6FMewhQxEs
https://www.dropbox.com/s/z1d2sr4blmlr0ms/Declining%20A%20Generous%20General%20-%20choplix%20coif%20quest.rar?dl=0
https://ffxi.allakhazam.com/db/quests.html?fquest=658