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: Add basic text client with UT tab #3596

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

Conversation

Jouramie
Copy link
Contributor

@Jouramie Jouramie commented Jun 28, 2024

What is this fixing or adding?

This adds a text client specific to Stardew Valley, with integration to Universal Tracker (when the apworld is installed).

This will later be used for future features, like being able to explain why a location or a game item is in logic or not. The idea came to me seeing Figment wondering what was the rule for the Magnet Bait, to be able to fill their Master Baiter Bundle. Exploiting the Stardew rule framework will allow the client to give user-friendly explanation to why an item is in logic or not based on their seed progress.

Note that Tracker Page still lies on a lot of location and is not yet integrated with ER, bundles randomization and season randomization. This will come in a later PR, possibly only after 6.x.x is merged. Until it is ready, the link in the AP Launcher is removed. It will not be displayed until UT is properly integrated with Stardew, to avoid confusion.

How was this tested?

Launched the client with and without Universal Tracker. Connected to a multiworld to make sure allegedly available locations are displayed in the tracker tab. This might seem light, but this PR rely entirely on UT for all the tracking.

If this makes graphical changes, please attach screenshots.

image

image

image

@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Jun 28, 2024
@Jouramie Jouramie force-pushed the StardewValley/stardew-text-client branch from 55add71 to 14f3c61 Compare June 28, 2024 05:00
@ScipioWright
Copy link
Collaborator

This should absolutely wait until support has been put in for ER to work properly with UT, or you are going to greatly increase the number of false reports of UT bugs that the UT devs get.

@ScipioWright ScipioWright added the is: enhancement Issues requesting new features or pull requests implementing new features. label Jun 28, 2024
@agilbert1412 agilbert1412 self-requested a review June 28, 2024 13:10
@Jouramie
Copy link
Contributor Author

@ScipioWright Based on discussion in UT thread, I removed the link from the launcher at least until UT works with

  • ER
  • Bundles
  • Starting season

This PR is still completely valid. The launcher entry is hidden behind a feature flag. I plan to complete the integration in future PR for the sake of doing small incremental changes.

@Jouramie Jouramie requested a review from beauxq July 3, 2024 01:33
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.

Sounds like a pretty good first step

data/stardew.png Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if the SMAPI icon would make more sense in this case
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure I don't mind changing it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if we should. I was just asking.
Our game is Stardew, so Stardew icon makes sense, but it's also a lot more chaotic than SMAPI has ever been, so the messy mashup sprite also fits.

I really don't know which is the better choice tbh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the client is hidden from the launcher, that gives us plenty of time to choose the right icon until it's really released

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.

None yet

4 participants