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: Force deactivation of Mr. Qi's special orders when ginger island is deactivated #4348

Conversation

Jouramie
Copy link
Contributor

@Jouramie Jouramie commented Dec 9, 2024

What is this fixing or adding?

This force the deactivation of Mr. Qi's special orders when ginger island is deactivated. Not force disabling this option was not a bug, because every time we check if Qi's special orders were enabled, we also check if ginger island is enabled (now this is unnecessary). The goal is more to handle incompatible option combination at the same place and log a warning to the person generating knows that some locations won't be included. This will also prevent future bug if we eventually forgot to check ginger island every time we also check if Qi's special orders are enabled.

image

How was this tested?

  • Wrote some unit tests to validate specifically the function forcing options activation
  • Generate games with qi's special orders settings and ginger island excluded/included. With those changes, is deactivated when ginger island is deactivated.

If this makes graphical changes, please attach screenshots.

N/A

@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Dec 9, 2024
@agilbert1412 agilbert1412 added the is: enhancement Issues requesting new features or pull requests implementing new features. label Dec 9, 2024
Copy link
Collaborator

@agilbert1412 agilbert1412 left a comment

Choose a reason for hiding this comment

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

Small QoL suggestion that is not very important.
Otherwise, good change.
Should we also remove the instances of filtering that this replaces?

@Berserker66 Berserker66 merged commit aa22b62 into ArchipelagoMW:main Dec 9, 2024
16 checks passed
@Jouramie
Copy link
Contributor Author

Jouramie commented Dec 9, 2024

Should we also remove the instances of filtering that this replaces?

Woops I missed that question. I intend to remove the filtering when the special orders will be reworked to be in the content pack.

@Jouramie Jouramie deleted the StardewValley/force-deactivation-of-qis-special-orders branch December 10, 2024 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants