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] [sql] [Quest] Two Horn the Savage Implementation #7102

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

Conversation

slashtangent
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?

Adds the quest Two Horn the Savage

Steps to test these changes

  1. Zone into Aht Urhgan Whitegate !zone 50
  2. Talk to Milazahn !pos -58.1921 0.000 1.9746
  3. Trade 1k Gil to Cacaroon !pos -71.1544 0.000 -86.0422
  4. Talk to Cacaroon to receive onetime per zon dialogue
  5. Zone to test dialogue replayes
  6. Zone to Mamook !zone 65
  7. Activate the Viscous Liquid at E-8 !pos -262.437 5.130 -141.241
  8. Activate Viscous Liquid again to spawn NM
  9. Kill the NM
  10. Activate the Viscous Liquid
  11. Zone into Aht Urhgan Whitegate !zone 50
  12. Talk to Cacaroon to test once per zone dialogue !pos -71.1544 0.000 -86.0422
  13. Talk to Milazahn to end quest !pos -58.1921 0.000 1.9746

Sources

  • Quest Captures

https://youtu.be/o_aWAEJjxvs
Capture logs broke part way through. Logging out breaks the capture suite.
https://drive.google.com/drive/folders/1D0uhQ1VqkoKJtqEmj9Fu-6BrcyH0mtjJ?usp=sharing

NM Kill + Finish Quest (Packets Only)
https://1drv.ms/u/c/87e665e10555c452/Ebv7ERqzu1xErLiYf_Rci7YBVgLBkL6SV7L5wMBVRUYzfA?e=BZEFgB

Quest: Two-Horn the Savage
*Includes partial capture of Rat Race as one of the wikis had erroneous info that was checked.
https://www.youtube.com/watch?v=Yy_7jWLgrOg
https://drive.google.com/drive/u/1/folders/1P7hHf2obpp0n9yc9PczOLmxfncSKniR2

  • NM Captures

NM HP Capture
https://youtu.be/ydD0venpAKY
https://1drv.ms/u/c/87e665e10555c452/Ef_HDjh22A9AmQM-VvBh55wB1eNhXTdzIt81N6PBKL5Kkw?e=iLLclv

Immunities + BLU + MDB MDT BDT Testing
https://youtu.be/cmEC9NJfZeo
https://1drv.ms/u/c/87e665e10555c452/EU7_ieBAn6tBhj_5sG7efOYB5HIDKZuyomSnAHZb-V883g?e=fpIdhX

Mamool Ja NM testing regen + kiting + axe throw mobskill
https://youtu.be/nv-GKqkq9Qk
https://1drv.ms/u/c/87e665e10555c452/EYNdkvMle59ImZ0oKssglr4BgMn9VgBHtZAQ0UL5o9iI9w?e=5lnkz5

NM - Mamool Ja mobskill list test (From Two Horn the Savage Quest)
https://youtu.be/jLnJ8OUaugc
https://1drv.ms/u/c/87e665e10555c452/EWSExBX57INCmSoz6I5ymm4Bwdp72RUdfOVgoeoFG0mm0A?e=SbN2P4

@slashtangent slashtangent force-pushed the Two-Horn-the-Savage branch 2 times, most recently from 9b14bdf to 67dd71e Compare February 20, 2025 07:32
Copy link
Contributor

@zach2good zach2good left a comment

Choose a reason for hiding this comment

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

Seems fine to me, but I'd want @KnowOne134 and @claywar to cast their wise eyes over it

['Cacaroon'] =
{
onTrigger = function(player, npc)
if quest:getVar(player, 'Temp') == 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.

for var reference in the quest system we are going to start ultizing a pattern again. please use vars Prog, Stage, Option, Wait with these we should be able to handle every situation that may arise and even bit pack them is needed.
so this var should be tagged with the 'Option' label as its an optional CS.

Copy link
Contributor

Choose a reason for hiding this comment

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

also does this cs still pay after the trade of 1000 gil? if not it could just be if var prog == 0 :oncePerZone() instead of even needed the option var set

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Plays after the player pays 1k gil. Swapped over to if quest:getVar(player, 'Prog') >= 1 Will continue to play once per zone from then forward until the quest is complete. Consistent with what was found in captures.

{
onTrigger = function(player, npc)
local progress = quest:getVar(player, 'Prog')
local liquid = npc:getID()
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for the ocal here, its only being called 1x, just move the logic to the check

if npc:getID() == ID.npc.QUEST_LIQUID then

{
onMobDeath = function(mob, player, optParams)
if quest:getVar(player, 'Prog') == 2 then
quest:setVar(player, 'Prog', 3) -- Zoning will NOT reset progress. Progress is gained and kept as long as the NM has died.
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be a ocal var as zoning loses the info and must fight again, adjust the code above as well to reflect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Zoning does not lose the value and does not require you to fight the NM again. This was tested in captures.

@@ -43,7 +44,8 @@ zones[xi.zone.MAMOOK] =
},
npc =
{
LOGGING = GetTableOfIDs('Logging_Point'),
LOGGING = GetTableOfIDs('Logging_Point'),
QUEST_LIQUID = GetTableOfIDs('Viscous_Liquid')[6], -- Used in quest Two Horn the Savage
Copy link
Contributor

Choose a reason for hiding this comment

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

thinking we should just use:

VISCOUS_LIQUID = GetTableOfIDs('Viscous_Liquid')

then reference the 6th id in the quest script, so if code needs others in future edits wont be needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clay suggested I do it this Way. As far as I'm aware, this quest is the only other content that uses Viscous Liquids besides the illusion effect.

Copy link
Contributor

Choose a reason for hiding this comment

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

@claywar thoughts on change or eave as is, coud also remove comment about what it is if change made.
not liking the naming of it as quest_liquid then saying what its for after, just personal prefences really, but i like consistancy.

-- !spawnmob 17043871
-- Viscous Liquid !pos -262.437 5.130 -141.241
-----------------------------------
mixins = { require('scripts/mixins/families/mamool_ja'), require('scripts/mixins/weapon_break') }
Copy link
Contributor

Choose a reason for hiding this comment

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

mixins =
{
    require('scripts/mixins/families/mamool_ja'),
    require('scripts/mixins/weapon_break') 
}

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