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: Move progressive tool options handling in features #4374

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

Jouramie
Copy link
Contributor

@Jouramie Jouramie commented Dec 14, 2024

What is this fixing or adding?

In prevision of adding the option to start without any tools, I'm moving the handling of the progressive tools to the feature architecture, introduced with 6.x.x.

The next steps will be to actually add the option and review the logic, but in order to make those changes more reviewable, I'm trying to break down the changes in more manageable PRs.

The tool distribution stuff should also be handled by content packs instead of being directly in the feature. This would allow better handling of potential new tools or materials from mods. However, it did not seem necessary right now.

How was this tested?

Added some unit tests to make sure the tool count is correct depending on the options. I also generated games with combination or progressive tools / progressive skills, and the item count was fine.

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 14, 2024
@agilbert1412 agilbert1412 added the is: refactor/cleanup Improvements to code/output readability or organizization. label Dec 14, 2024
@agilbert1412 agilbert1412 self-requested a review December 17, 2024 03:13
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.

Couple of questions, will approve if the answer is that things are okay as is

worlds/stardew_valley/content/__init__.py Outdated Show resolved Hide resolved
worlds/stardew_valley/logic/mine_logic.py Outdated Show resolved Hide resolved
Jouramie added a commit to agilbert1412/Archipelago that referenced this pull request Dec 17, 2024
@Jouramie Jouramie force-pushed the StardewValley/move-tool-handling-in-feature branch from 8bdd6fd to 5d31729 Compare January 9, 2025 04:57
Copy link
Collaborator

@beauxq beauxq left a comment

Choose a reason for hiding this comment

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

I don't see any malware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: refactor/cleanup Improvements to code/output readability or organizization. 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