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.

Sign up for GitHub

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

Jump to bottom

ChemMaster+ #2778

Closed
wants to merge 2 commits into from
Closed

ChemMaster+ #2778

wants to merge 2 commits into from

Conversation

sleepyyapril
Copy link
Contributor

About the PR

Changes how the ChemMaster works:

  1. Removes the amount buttons and instead uses a textbox that resets whenever a value is entered or focus ends.
  2. Shrinks the ChemMaster again.
  3. Adds sorting options, including for quantity, last added, and a sort option for the (default) alphabetical order.
  4. Sorting options save via the ChemMaster itself, not per-user.

Why / Balance

Sorting the ChemMaster by default is bad, let people have the right to choose how it sorts.
Also lets you set your own quantity to transfer/discard.

Technical details

  1. Adds TransferringAmount and SortMethod to ChemMasterComponent, to save separately for each ChemMaster.
  2. Edits ChemMasterUiState to support the above.
  3. Shrinks the ChemMaster's UI because we no longer need that much width.

Mostly UI changes.

Media

Video showcase:
https://discord.com/channels/968983104247185448/1010560161888350340/1330470986205761597

Requirements

  • I have tested all added content and changes.
  • I have added media to this PR or it does not require an ingame showcase.

Breaking changes

Changelog

🆑

  • add: Added ChemMaster sorting and a custom amount input.

@sleepyyapril @VMSolidus
ChemMaster+ (DeltaV-Station#1585)
7b5b033 
<!--
This is a semi-strict format, you can add/remove sections as needed but
the order/format should be kept the same
Remove these comments before submitting
-->

<!--
Explain this PR in as much detail as applicable

Some example prompts to consider:
How might this affect the game? The codebase?
What might be some alternatives to this?
How/Who does this benefit/hurt [the game/codebase]?
-->

Changes how the ChemMaster works:
1. Removes the amount buttons and instead uses a textbox that resets
whenever a value is entered or focus ends.
2. Shrinks the ChemMaster again.
3. Adds sorting options, including for quantity, last added, and a sort
option for the (default) alphabetical order.
4. Sorting options save via the ChemMaster itself, not per-user.

Video showcase:
https://discord.com/channels/1218698320155906090/1218698321053356060/1330129166384894046

---

<!--
You can add an author after the `:cl:` to change the name that appears
in the changelog (ex: `:cl: Death`)
Leaving it blank will default to your GitHub display name
This includes all available types for the changelog
-->

:cl:
- add: Added sorting options to the ChemMaster.
- add: Added the ability to input custom amounts into the ChemMaster via
a textbox that resets on change.
- tweak: The width of the ChemMaster UI has been lowered.
- remove: Removed quantity buttons from the ChemMaster.

---------

Co-authored-by: VMSolidus <[email protected]>
@sleepyyapril sleepyyapril requested a review from a team as a code owner January 20, 2025 00:48
@github-actions github-actions bot added S: Needs Review size/L 256-1023 lines Changes: UI Changes: C# Changes any cs files Changes: Localization Changes any ftl files labels Jan 20, 2025
@sleepyyapril
Copy link
Contributor Author

I could theoretically upstream.

@ThataKat
Copy link
Contributor

Direction is reviewing, get back to you in about 24 hours!

@rosieposieeee
Copy link
Contributor

might be a PITA, but this exclusively touches upstream files, so i would suggest opening a parallel PR and seeing if that gets accepted

@sleepyyapril
Copy link
Contributor Author

sleepyyapril commented Jan 21, 2025 •
edited
Loading

might be a PITA, but this exclusively touches upstream files, so i would suggest opening a parallel PR and seeing if that gets accepted

Not able to until one of the ChemMaster PR snipe changes gets merged.

@sleepyyapril
Copy link
Contributor Author

sleepyyapril commented Jan 21, 2025 •
edited
Loading

I'm going over this, and I'm likely going to redo the entire ChemMaster systems as a whole. You can merge this as-is, though.

@dvir001
Copy link
Contributor

dvir001 commented Jan 21, 2025

I'm going over this, and I'm likely going to redo the entire ChemMaster systems as a whole. You can merge this as-is, though.

No comments, no merge.

@ThataKat
Copy link
Contributor

Seen closed. Direction recommends trying this type of change upstream first anyhow.

@sleepyyapril
Copy link
Contributor Author

Seen closed. Direction recommends trying this type of change upstream first anyhow.

Again, as stated, I can't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Changes: C# Changes any cs files Changes: Localization Changes any ftl files Changes: UI S: Needs Review size/L 256-1023 lines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sleepyyapril @ThataKat @rosieposieeee @dvir001