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

CommonClient: Improve commands #2761

Closed
wants to merge 2 commits into from

Conversation

EmilyV99
Copy link
Contributor

@EmilyV99 EmilyV99 commented Jan 24, 2024

What is this fixing or adding?

Modified some basic CommonClient commands to be more useful.
'/missing' now properly can take multi-word filter text (instead of giving an error for invalid param count), and is case-insensitive.
'/received', '/items', and '/locations' now also take optional filter text.
Feedback from all changed commands also slightly improved (ex. now repeats the filter text used, indicating the results only include those that match that filter; where it previously showed a number of results, that now only counts those that match the filter, instead of all)

MultiServer's '!missing' and '!checked' have also received similar improvements.

How was this tested?

Ran all of the changed commands, checking for expected output. Some examples shown below.
Commands were tested both with and without filter text.

If this makes graphical changes, please attach screenshots.

image
(/received: Finding the days of the traveling cart in Stardew Valley that have been received was the main driving factor in this addition, and works flawlessly)
image
(!missing - the MultiServer command)
image
(!checked - the MultiServer command)
image
(/missing - the CommonClient command)
image
(/items)
image
(/locations)

@Berserker66
Copy link
Member

First one is a duplicate of #2675

@ScipioWright ScipioWright added is: enhancement Issues requesting new features or pull requests implementing new features. affects: core Issues/PRs that touch core and may need additional validation. labels Jan 26, 2024
@ScipioWright ScipioWright added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Feb 14, 2024
@EmilyV99 EmilyV99 force-pushed the CommonClient branch 5 times, most recently from b207af8 to efc4e82 Compare February 21, 2024 11:15
@beauxq
Copy link
Collaborator

beauxq commented Feb 21, 2024

EmilyV99#2

@beauxq
Copy link
Collaborator

beauxq commented Feb 21, 2024

Did you test to make sure they still work without filter text?

@EmilyV99
Copy link
Contributor Author

Did you test to make sure they still work without filter text?

aye, they all work without filter text, and give a different message output (ex. Found x/y missing checks vs Found x/y matching missing checks) based on being filtered or not.

CommonClient.py Show resolved Hide resolved
CommonClient.py Outdated Show resolved Hide resolved
CommonClient.py Show resolved Hide resolved
@EmilyV99 EmilyV99 requested a review from beauxq March 7, 2024 19:59
CommonClient.py Outdated Show resolved Hide resolved
@Exempt-Medic Exempt-Medic added the waiting-on: author Issue/PR is waiting for feedback or changes from its author. label Apr 26, 2024
@Exempt-Medic
Copy link
Member

This has conflicts now just so you know

@EmilyV99 EmilyV99 closed this Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features. waiting-on: author Issue/PR is waiting for feedback or changes from its author. 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.

5 participants