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

TGUI Color Picker #26326

Merged
merged 34 commits into from
Nov 7, 2024
Merged

TGUI Color Picker #26326

merged 34 commits into from
Nov 7, 2024

Conversation

Spaghetti-bit
Copy link
Contributor

What Does This PR Do

Ports BeeStation/BeeStation-Hornet#9417 from Beestation.

TGUI Color wheel, converts most (if not all) color picker matrixes into TGUI Color pickers.

Why It's Good For The Game

It's nice on the eyes and is another step forward towards implementing TGUI fully.

Images of changes

image

Testing

Lots of different UI tests, character preferences, appearance changer / mirror, DNA powers like morph, machine monitor.

Changelog

🆑
tweak: Color Picker now uses TGUI
/:cl:

@Burzah Burzah marked this pull request as draft July 29, 2024 01:26
@ParadiseSS13-Bot ParadiseSS13-Bot added -Status: Awaiting type assignment This PR is waiting for its type to be assigned internally TGUI This PR modifies TGUI, will conflict labels Jul 29, 2024
@Burzah Burzah added the FEATURE FROZEN This PR is feature frozen label Jul 29, 2024
@S34NW S34NW added the Untested This didnt even pass CI. label Jul 29, 2024
@S34NW
Copy link
Member

S34NW commented Jul 29, 2024

Hi, as explained in #26085, on discord here and on the top of the repo home page here. We are currently in a period of "quality releases" where only refactor or fix PRs are permitted unless you have prior approval for another PR type. This is commonly referred to as a feature freeze.

As such, this PR is going to be closed. You are welcome to revisit the idea once the freeze has been lifted, but we encourage all contributors to get stuck in with fixes for items on the issue list!

@S34NW S34NW closed this Jul 29, 2024
@Spaghetti-bit Spaghetti-bit reopened this Aug 8, 2024
@S34NW S34NW removed the FEATURE FROZEN This PR is feature frozen label Aug 8, 2024
@Spaghetti-bit Spaghetti-bit marked this pull request as ready for review August 8, 2024 19:49
@Burzah Burzah requested review from Sirryan2002 and Burzah August 8, 2024 20:37
Copy link
Contributor

@AyIong AyIong left a comment

Choose a reason for hiding this comment

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

I get the impression that this has not been properly tested
Here's a small review of what should be done first of all, add a if(isnull(value)) return check after input where it doesn't exist, secondly replace if(value) ... and if(!value) return with the aforementioned isnull().

Since tgui inputs return null if the player decides to close the window or click Cancel
I hope you don't repeat my fate with a bunch of follow up PRs on fix checks :feelsgood:

code/modules/tgui/tgui_input/color_input.dm Outdated Show resolved Hide resolved
code/modules/tgui/tgui_input/color_input.dm Outdated Show resolved Hide resolved
code/modules/tgui/tgui_input/color_input.dm Outdated Show resolved Hide resolved
code/modules/tgui/tgui_input/color_input.dm Outdated Show resolved Hide resolved
code/modules/tgui/tgui_input/color_input.dm Outdated Show resolved Hide resolved
code/modules/tgui/tgui_input/color_input.dm Outdated Show resolved Hide resolved
code/modules/tgui/tgui_input/color_input.dm Outdated Show resolved Hide resolved
code/game/dna/mutations/mutation_powers.dm Outdated Show resolved Hide resolved
code/game/machinery/dye_generator.dm Outdated Show resolved Hide resolved
code/game/turfs/simulated/floor/light_floor.dm Outdated Show resolved Hide resolved
@Burzah Burzah removed the Untested This didnt even pass CI. label Aug 8, 2024
@ParadiseSS13-Bot ParadiseSS13-Bot added -Status: Awaiting review This PR is awaiting review from the review team and removed -Status: Awaiting type assignment This PR is waiting for its type to be assigned internally labels Aug 10, 2024
@github-actions github-actions bot added the Merge Conflict This PR is merge conflicted label Aug 11, 2024
@github-actions github-actions bot removed the Merge Conflict This PR is merge conflicted label Nov 5, 2024
Co-authored-by: Contrabang <[email protected]>
Signed-off-by: Spaghetti-bit <[email protected]>
@github-actions github-actions bot added the Merge Conflict This PR is merge conflicted label Nov 6, 2024
@Burzah Burzah force-pushed the TGUIColorInput branch 2 times, most recently from 623939d to b14682d Compare November 7, 2024 08:07
Copy link
Contributor

@Contrabang Contrabang left a comment

Choose a reason for hiding this comment

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

Needs a TM

@ParadiseSS13-Bot ParadiseSS13-Bot added -Status: Awaiting merge This PR is ready for merge and removed -Status: Awaiting review This PR is awaiting review from the review team labels Nov 7, 2024
@AyIong
Copy link
Contributor

AyIong commented Nov 7, 2024

Needs a TM

old-man

@ParadiseSS13-Bot ParadiseSS13-Bot added the Testmerge Active This PR is currently testmerged on production label Nov 7, 2024
@github-actions github-actions bot removed the Merge Conflict This PR is merge conflicted label Nov 7, 2024
Copy link
Member

@Burzah Burzah 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. Test merge has seen no issues at this time. All outstanding conversations have been resolved.

@Burzah Burzah added this pull request to the merge queue Nov 7, 2024
Merged via the queue into ParadiseSS13:master with commit afd52f0 Nov 7, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-Status: Awaiting merge This PR is ready for merge Port This PR is from another server, or heavily inspired/adapted. Testmerge Active This PR is currently testmerged on production Testmerge Requested This PR has a pending testmerge request TGUI This PR modifies TGUI, will conflict
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants