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

Add The Path of Glouphrie #1289

Merged
merged 70 commits into from
Oct 7, 2023

Conversation

pajlada
Copy link
Contributor

@pajlada pajlada commented Sep 25, 2023

The quest guide should be complete, I'd like to finish it on my second character too, to make sure I haven't missed anything.
After the Java 11 & test PRs have been merged in, I'll do my last force push of this branch.

There are a few changes that should be removed before this gets merged in, like the varbit exporter & the optimal quest order

@pajlada pajlada force-pushed the quest/the-path-of-glouphrie branch 2 times, most recently from 6730256 to 18653b3 Compare September 27, 2023 09:46
@pajlada
Copy link
Contributor Author

pajlada commented Sep 27, 2023

Latest progress, puzzle solving works when you have the right pieces.
next step is to make it work for when you just have garbage

pog-puzzle.mp4

@pajlada pajlada force-pushed the quest/the-path-of-glouphrie branch from 21bf8a8 to 334f6c4 Compare September 28, 2023 10:05
@pajlada pajlada force-pushed the quest/the-path-of-glouphrie branch from 0a72b3d to 81e10c6 Compare September 30, 2023 16:12
@pajlada pajlada changed the title [WIP] Add The Path of Glouphrie Add The Path of Glouphrie Sep 30, 2023
@pajlada pajlada marked this pull request as ready for review September 30, 2023 16:13
@pajlada
Copy link
Contributor Author

pajlada commented Sep 30, 2023

I ran through the quest with my main now, fixed some phrasing

There's some more wording that can be cleaned up, and the puzzle could help more if you don't have enough discs

video of my main attempt https://www.youtube.com/watch?v=eVZYh87yea4

@pajlada
Copy link
Contributor Author

pajlada commented Oct 1, 2023

Currently looking into making the exchanger easier to use

@pajlada pajlada force-pushed the quest/the-path-of-glouphrie branch from bef6c94 to 2fd8685 Compare October 1, 2023 11:52
@pajlada
Copy link
Contributor Author

pajlada commented Oct 1, 2023

You're now hinted at what should be put in the exchanger (if you have a single-disc exchangable item that would solve things)
image
(in the current version it would only highlight one item instead of all possible solutions)

When you've exchanged to find a solution, it'll highlight like this:
image

@pajlada
Copy link
Contributor Author

pajlada commented Oct 1, 2023

During the monolith puzzle step, I suggest the player to get discs once from every chest, however some users will not be able to get discs from the last chest because I think the game chests the value of all the discs you have in your inventory, rather than how many discs you have. The check I have now is just for how many discs, so the text is a bit vague saying "if you can't get discs, just drop your discs & then click the chest" - I think this is clear enough since it'd be a bit tricky to implement

Copy link
Owner

@Zoinkwiz Zoinkwiz left a comment

Choose a reason for hiding this comment

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

Went through the quest once with the following points, I have another account I can do this on too so post any changes I can run through it again!

var talkToGianneJnrStep = new ConditionalStep(this, climbUpToGianneJnr);
talkToGianneJnrStep.addStep(inGnomeStrongholdFloor1, talkToGianneJnr);

var talkToLongrambleStep = new ConditionalStep(this, talkToLongramble);
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a benefit to wrapping up individual steps in a ConditionalStep, or is this done as a form of consistency/extendability for the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was mostly a "previous steps used this, so I copy pasted it" - this has been resolved in my refactor.
Each section has its own class so loadSteps looks a bit cleaner https://github.com/Zoinkwiz/quest-helper/pull/1289/files#diff-05bf0febe845e8fc2e1c42b80fd499249b45f2b76ac8e215eb65795ad312be41R121-R156

{
/// Starting off
// Talk to King Bolren
var teleToBolren = new TeleportItemRequirement("Spirit tree to Tree Gnome Village [1]", -1, -1);
Copy link
Owner

Choose a reason for hiding this comment

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

This can be shifted to the setupRequirements section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/// Starting off
// Talk to King Bolren
var teleToBolren = new TeleportItemRequirement("Spirit tree to Tree Gnome Village [1]", -1, -1);
talkToKingBolren = new NpcStep(this, NpcID.KING_BOLREN, new WorldPoint(2542, 3169, 0), "Talk to King Bolren in the Tree Gnome Village to start the quest");
Copy link
Owner

Choose a reason for hiding this comment

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

Generally for steps we'd have the steps have full stops at the end as punctuation, so it'd be good to do the same for all the steps in this helper too.

Copy link
Contributor Author

@pajlada pajlada Oct 1, 2023

Choose a reason for hiding this comment

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

Should be resolved in 07539a9 (#1289) and f3bfdd1 (#1289) (and f3bfdd1 )

regionPoint(39, 25)
));

pushSouthernMonolithUp = new NpcStep(getQuestHelper(), NpcID.BIG_MONOLITH, regionPoint(37, 21), "Push the monolith north once");
Copy link
Owner

Choose a reason for hiding this comment

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

It'd be good to mark the tile to push from with a tile marker for these steps, WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm open to it, but when I was zooming through it, it didn't feel like it was necessary https://youtu.be/eVZYh87yea4?t=44

pushSmallMonolithSouth = new NpcStep(getQuestHelper(), NpcID.SMALL_MONOLITH, regionPoint(39, 33), "Push the small monolith south once");
pushSmallMonolithSouth.setMaxRoamRange(2);

pushNWMonolithWest = new NpcStep(getQuestHelper(), NpcID.BIG_MONOLITH, regionPoint(36, 33), "Push the north-west monolith west once");
Copy link
Owner

Choose a reason for hiding this comment

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

This is an absolute edge-case, but there's two cases of 1: the monolith being pushed all the way to the east (we could highlight it for both tile options), and 2: the SW monolith being pushed up (requires a push down or reset depending on if pushed up once/twice).

This one's more of a nice to have, probably fine to skip it.

Screenshot 2023-10-01 203218 Screenshot 2023-10-01 203447

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a mention of this in the code for future adventurers in 8ffcd66 (#1289) since it would be best to make those sort of changes while in the room

enterSewer.addRequirement(combatGear, prayerPotions, food, crystalChime);

sewer1Ladder = new ObjectStep(this, ObjectID.LADDER_49700, "Climb up the ladder");
sewer2Ladder = new ObjectStep(this, ObjectID.LADDER_49701, new WorldPoint(1529, 4236, 1),
Copy link
Owner

Choose a reason for hiding this comment

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

Would be good to clarify directionally where this is given it's out of sight, for anyone not using the map arrows. 'Climb down the ladder to the far east.' or something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in df7b1f4 (#1289) - I added directions to the three steps that have the user walk far

sewer1Ladder = new ObjectStep(this, ObjectID.LADDER_49700, "Climb up the ladder");
sewer2Ladder = new ObjectStep(this, ObjectID.LADDER_49701, new WorldPoint(1529, 4236, 1),
"Climb down the ladder.");
sewer2Ladder.addRecommended(protectMissiles);
Copy link
Owner

Choose a reason for hiding this comment

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

If the earmuffs reduce damage worth us highlighting to wear them for this section too as recommended.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 24f1930 (#1289)

sewer3Ladder = new ObjectStep(this, ObjectID.LADDER_49700, new WorldPoint(1529, 4253, 0),
"Climb up the ladder.");
sewer3Ladder.addRecommended(protectMissiles);
sewer4Ladder = new ObjectStep(this, ObjectID.LADDER_49701, new WorldPoint(1486, 4283, 1),
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
sewer4Ladder = new ObjectStep(this, ObjectID.LADDER_49701, new WorldPoint(1486, 4283, 1),
sewer4Ladder = new ObjectStep(this, ObjectID.LADDER_49701, new WorldPoint(1486, 4282, 1),

Was a tile too far north of the ladder.

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 catch, fixed in 4770490 (#1289)

"Kill the Terrorbirds. You can use the pillars around the room to only fight one at a time. They fight with both Melee and Ranged.");
bossStep.setAllowMultipleHighlights(true);

peekHeavyDoor = new ObjectStep(this, NullObjectID.NULL_49909, WorldPoint.fromRegion(5955, 49, 31, 1), "Peek the heavy door");
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
peekHeavyDoor = new ObjectStep(this, NullObjectID.NULL_49909, WorldPoint.fromRegion(5955, 49, 31, 1), "Peek the heavy door");
peekHeavyDoor = new ObjectStep(this, NullObjectID.NULL_49909, WorldPoint.fromRegion(5955, 49, 31, 1), "Peek through the heavy door.");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 45929d6 (#1289)

peekHeavyDoor = new ObjectStep(this, NullObjectID.NULL_49909, WorldPoint.fromRegion(5955, 49, 31, 1), "Peek the heavy door");
watchFinalCutscene = new DetailedQuestStep(this, "Watch the final cutscene");

talkToHazelmere = new NpcStep(this, NpcID.HAZELMERE, new WorldPoint(2678, 3086, 1),
Copy link
Owner

Choose a reason for hiding this comment

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

Not certain how, but after the final step the 'Yes.' text on the lamps was highlighting. Not something to be fixed in this, but it seemed there was some weirdness with a step not being properly discarded.

@pajlada pajlada force-pushed the quest/the-path-of-glouphrie branch from 723012c to 47f11a8 Compare October 1, 2023 20:28
@pajlada
Copy link
Contributor Author

pajlada commented Oct 3, 2023

Currently working on cleaning up the exchange process

@pajlada
Copy link
Contributor Author

pajlada commented Oct 4, 2023

This quest is now in a testable state again

What's changed outside of the reviews?

For Yewnock's puzzle:

  • Exchanges now clearly highlight where to insert things, and what buttons to press.
  • When prompted to switch from the exchanger to the machine, or vice versa, the close button will be highlighted
  • Refactored the updateSteps function & added a bunch of comments, hopefully making it a bit clearer what's going on
  • The exchanger now correctly highlights (I had the wrong coordinates chosen)
  • We try exchanging asap, even if we don't even have a solution for puzzle 1, and only prompt the player to fetch discs if they really really have to. (or if they would have had to exchange multiple discs at the same time - we don't support that)

For the sewer steps:

  • Earmuffs to-be-equipped or to-be-had are added as recommended as soon as you're prompted to enter the sewer

For the final step:

  • I incorrectly stated that the lamp(s) that don't fit in your inventory would fall to the ground - this is incorrect (at least for UIM). Instead, say that you need to just make sure you get 4 lamps and if not, talk to Hazelmere again

This is done by:
1) If no solution to the first puzzle is found, try to find valid single-disc exchanges. If this doesn't work, just ask
   them to get more discs (which was the old solution)
2) If there's a solution to the first puzzle, but not even a partial solution to the second solution, try to find an
   exchange that would let us get part of the second puzzle complete. Once that's done, the next solution solve will try
   to use the previous "find exchange for the last part of the second puzzle" method.
If the solution was a double solution, we would previously only check the first item
…the first slot)

The third input slot was ignored previously, potentially causing even more confusion to this confusing part.
Copy link
Owner

@Zoinkwiz Zoinkwiz left a comment

Choose a reason for hiding this comment

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

Looks good for a release! Thanks for the work on this, excited to have this available to everyone :D

@Zoinkwiz Zoinkwiz merged commit 30a7174 into Zoinkwiz:master Oct 7, 2023
1 check passed
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.

2 participants