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

Chem master UI #33328

Merged

Conversation

Intoxicating-Innocence
Copy link
Contributor

@Intoxicating-Innocence Intoxicating-Innocence commented Nov 15, 2024

About the PR

Added striped rows, as well as a small reagent colored bar for each reagent to the Chem Master buffer and input buffer UI.

Left side of Chem Master Buffer Buttons have been made into plain rectangles, in order to look better with the new colored bars.

Pill and bottle dosages in the output tab will now default to the largest valid value, rather than zero.

Why / Balance

This should not have an appreciable effect on game balance, but it should make the eyes of myself and other chemists very happy, and should make using Chem Masters to actually make pills slightly more tolerable.

Technical details

Not counting comments, and lines that seem to have been counted as additions because they were moved slightly, this PR contains probably less than 10 lines of novel code.

Edit: Changing the input buffer was more involved, and much was restructured, still not a large change, and limited to the one xaml.cs

Edit2: The vast majority of the file has now been desecrated in one manner or another.

Media!

Visuals as of latest commit:
ChemMasterInuputUICommit

Requirements

Breaking changes

It is my fondest hope that this breaks absolutely nothing.

Changelog

@github-actions github-actions bot added size/M Denotes a PR that changes 30-99 lines. S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. Changes: UI Changes: Might require knowledge of UI design or code. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Nov 15, 2024
Copy link
Contributor

@SaphireLattice SaphireLattice left a comment

Choose a reason for hiding this comment

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

Hooray, some UI love and care without any balancing concerns. More of this sort of thing please.

Probably want to make a Control for striped lists at some point, also.

Content.Client/Chemistry/UI/ChemMasterWindow.xaml.cs Outdated Show resolved Hide resolved
@SaphireLattice SaphireLattice added P3: Standard Priority: Default priority for repository items. D2: Medium Difficulty: A good amount of codebase knowledge required. S: Needs Review Status: Requires additional reviews before being fully accepted S: Awaiting Changes Status: Changes are required before another review can happen T: UI / UX Improvement Type: UI and player facing interactive graphical interfaces A: Medical Area: Medical department, including Chemistry T: Cleanup Type: Code clean-up, without being a full refactor or feature P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. and removed P3: Standard Priority: Default priority for repository items. labels Nov 15, 2024
@Intoxicating-Innocence Intoxicating-Innocence marked this pull request as draft November 15, 2024 13:04
@github-actions github-actions bot removed the S: Awaiting Changes Status: Changes are required before another review can happen label Nov 15, 2024
Copy link
Contributor

@SaphireLattice SaphireLattice left a comment

Choose a reason for hiding this comment

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

Looks good to me. Note for the future: stripey item lists as own control element

@SaphireLattice SaphireLattice added the S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. label Nov 15, 2024
@Intoxicating-Innocence Intoxicating-Innocence marked this pull request as ready for review November 15, 2024 17:38
@SaphireLattice SaphireLattice removed the T: Cleanup Type: Code clean-up, without being a full refactor or feature label Nov 15, 2024
@chromiumboy
Copy link
Contributor

I like the new appearance. Is there a reason why you didn't do the input container as well?

@SaphireLattice
Copy link
Contributor

Oops, didn't notice the input container didn't get the glow-up too.

Gonna request that changed too for full approval, oops.

@chromiumboy chromiumboy added the S: Awaiting Changes Status: Changes are required before another review can happen label Nov 17, 2024
@github-actions github-actions bot added size/L Denotes a PR that changes 100-1000 lines. and removed size/M Denotes a PR that changes 30-99 lines. labels Nov 18, 2024
@Intoxicating-Innocence
Copy link
Contributor Author

Oops, didn't notice the input container didn't get the glow-up too.

Gonna request that changed too for full approval, oops.

resolved, see EDIT.

@Intoxicating-Innocence
Copy link
Contributor Author

Intoxicating-Innocence commented Nov 18, 2024

I think it would look better if i could shape the colored elements to mirror the rightmost button and/or resemble the left side of the old 1u button, but I've no idea how to accomplish this in a manner that isn't perverse. If anyone who knows more about Style sheets than me can offer some wisdom that would be much appreciated.

Edit: I do not feel strongly enough about this to put off merging if maintainers find the PR acceptable/desireable as is.

…nheight specification from verticalstretch elements
@github-actions github-actions bot removed the S: Awaiting Changes Status: Changes are required before another review can happen label Nov 19, 2024
@Intoxicating-Innocence Intoxicating-Innocence marked this pull request as draft November 20, 2024 07:18
@Intoxicating-Innocence Intoxicating-Innocence marked this pull request as ready for review November 21, 2024 12:17
@Intoxicating-Innocence
Copy link
Contributor Author

just found a bug that lets it make more pills than fit in the canister, this ejects the pills, will fix this later today, maybe put off review until next push.

@metalgearsloth metalgearsloth added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted labels Nov 25, 2024
@github-actions github-actions bot added S: Needs Review Status: Requires additional reviews before being fully accepted and removed S: Awaiting Changes Status: Changes are required before another review can happen labels Nov 25, 2024
@FloatingFeeling
Copy link

HOLY PEAK PLEASE MERGE THIS I BEG

@SlamBamActionman SlamBamActionman merged commit 2ccd471 into space-wizards:master Dec 16, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: Medical Area: Medical department, including Chemistry Changes: UI Changes: Might require knowledge of UI design or code. D2: Medium Difficulty: A good amount of codebase knowledge required. P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. S: Approved Status: Reviewed and approved by at least one maintainer; a PR may require another approval. S: Needs Review Status: Requires additional reviews before being fully accepted size/L Denotes a PR that changes 100-1000 lines. T: UI / UX Improvement Type: UI and player facing interactive graphical interfaces
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants