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

Stardew Valley - Prize Ticket and Mystery Box grinding requires the abilty to redeem them #3728

Conversation

agilbert1412
Copy link
Collaborator

What is this fixing or adding?

Owning a Mystery Box or a Prize Ticket isn't enough to get rewards from it. Mystery boxes must be opened at the blacksmith, and Prize tickets must be redeemed at the prize machine in Lewis's house.

With Entrance Randomization, these two areas are not always available. But they were missing from the condition when grinding these items. Now I added it

Note: You do not require these areas to own a mystery box or prize ticket, for checks like "Shipsanity: X". You only need access to claim these items to use the rewards from them, which at the moment includes very few things. The most notable one is the book "Friendship 101", which was the reason the bug was discovered.

How was this tested?

Unit tests and local generation with spheres

@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Aug 2, 2024
@agilbert1412 agilbert1412 added the is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. label Aug 2, 2024
Comment on lines +46 to +47
return self.logic.and_(opening_rule, mystery_box_rule,
book_of_mysteries_rule, time_rule,)
Copy link
Member

Choose a reason for hiding this comment

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

Any reason this isn't just one line?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not particularly, any reason it should be?

Copy link
Member

Choose a reason for hiding this comment

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

Just seems odd to me, since it's a different formatting from before and also it has the final , which none of the other lines in this file do

Copy link
Contributor

Choose a reason for hiding this comment

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

The previous formatting was one argument per line to minimize potential git conflicts. We should stick to it or change it to the standard "all arguments on the same line". Two arguments is quite unconventional...

Copy link
Member

@Exempt-Medic Exempt-Medic left a comment

Choose a reason for hiding this comment

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

The changes LGTM and match my understanding of the game. It generates just fine still

Copy link
Contributor

@Jouramie Jouramie left a comment

Choose a reason for hiding this comment

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

Weird formatting, but otherwise lgtm

@Exempt-Medic Exempt-Medic added waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer. and removed waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Aug 29, 2024
@NewSoupVi NewSoupVi merged commit 8a809be into ArchipelagoMW:main Aug 31, 2024
17 checks passed
@Jouramie Jouramie deleted the StardewValley/PrizeTicketUsageRequiresLewisHouse branch September 3, 2024 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: bug/fix Issues that are reporting bugs or pull requests that are fixing bugs. waiting-on: core-review Issue/PR has been peer-reviewed and is ready to be merged or needs input from a core maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants