Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lua, quest] Converts BRD unlock and BRD AF1 to IF #6682

Open
wants to merge 1 commit into
base: base
Choose a base branch
from

Conversation

CriticalXI
Copy link
Contributor

I affirm:

  • I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • I have read and understood the Contributing Guide and the Code of Conduct.
  • I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

  • Converts Path of the Bard and Painful Memory BRD quests to interaction framework

Path of the Bard: https://www.youtube.com/watch?v=ZAJAjoMJFdo
Painful Memory: https://www.youtube.com/watch?v=QXsns9eag10

Steps to test these changes

  • Complete A Minstrel in Despair quest either manually or through GM commands
  • Speak with Song Runes in Valkurm Dunes to complete Path of the Bard quest
  • Set yourself to BRD 40 to start Painful Memory quest and follow the quest steps

return quest:progressEvent(138)
elseif quest:getVar(player, 'initialCS') == 1 then
return quest:progressEvent(137)
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onTrigger = function(player, npc)
    local initialCS = quest:getVar(player, 'initialCS')
    if initialCS == 0 then
        return quest:progressEvent(138)
    elseif initialCS == 1 then
        return quest:progressEvent(137)
    end
end,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjusted to the example

return quest:noAction()
elseif
quest:getVar(player, 'nmKilled') == 1
then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theres only 1 condition here. Also, as above, store the variable in a local var instead of fetching it.

@CriticalXI CriticalXI force-pushed the brd_af1_painful_memory branch 4 times, most recently from 65a66d8 to a7019cb Compare January 15, 2025 17:59
player:hasKeyItem(xi.ki.MERTAIRES_BRACELET) and
npcUtil.popFromQM(player, npc, ID.mob.TROS, { claim = true, hide = 0 }) and
nmKilled == 0
then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As written, there is a bug with this part of the code. If the player delays checking the Waters of Oblivion until after Tros has fully despawned, Tros will then respawn and agro onto the player while they are in the final cutscene.

if
    nmKilled == 0 and
    player:hasKeyItem(xi.ki.MERTAIRES_BRACELET) and
    npcUtil.popFromQM(player, npc, ID.mob.TROS, { claim = true, hide = 0 })
then

Swap the NM killed check to the top (or at least above the NM spawn) to prevent this from happening. Since you want to make sure all checks are passed before spawning the NM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll keep that in mind for future quests. Got it adjusted, along with the NPC commands below.

-----------------------------------
-- Log ID: 3, Quest ID: 63
-- Mertaire: !gotoid 17780764
-- Waters_of_Oblivion: !gotoid 17457347
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swap these !gotoid checks to !pos this way if ids ever shift this won't cause issues or need to be updated in the future.

-- -- Path of the Bard
-- -----------------------------------
-- -- Log ID: 3, Quest ID: 20
-- -- Song Runes: !gotoid 17199695
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This !gotoid as well.

@CriticalXI CriticalXI force-pushed the brd_af1_painful_memory branch 2 times, most recently from e62c79a to 7034107 Compare January 17, 2025 17:41
@CriticalXI CriticalXI force-pushed the brd_af1_painful_memory branch from 7034107 to 9a34126 Compare January 28, 2025 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants