Color Picker For Color Controllers #140
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
FEAT: Color picker rendering progress. The selected color, HSL, and RGB gradients all render FEAT: Added new method for rendering a rainbow in the AbstractWidget class FEAT: Added a new method for rendering a sideways gradient in the AbstractWidget class TEST: Added mouse-only detection for enabling/disabling the color picker. It needs some work to allow for controller support(e.g. a button instead of the current mouseX/mouseY detection) TEST: Started work on the RGB slider detection. Needs a lot of work, and probably needs to be moved as well TEST: Added an extra color option in the "aaaaaaaaaaaa..." category BUG: The color picker has some "z-fighting" issues with options behind it BUG: The color picker needs to be in its own widget, not the way it is right now Everything is still heavy WIP, I just needed a checkpoint/backup for my work thus far
REFACTOR: Moved almost all of the color picker code/rendering outside of the ColorControllerElement and into a new ColorPickerElement REFACTOR: Renamed a few variables
FEAT: Added a method which determines the slider's x pos based upon the pending value's hue BUG: If the color is completely black or white, the slider automatically goes to the beginning, as if the color was completely red BUG: The color picker "z-fighting" with the option behind it is still an issue
FIX: Fixed majority of the z-fighting with the color picker BUG: The option behind the color picker still gets selected. For example, the string controller behind the color picker can be typed in after clicking the color picker
FEAT: Made the down button sound only play when clicking the color preview, which enables the color picker FIX: Fixed a bug where the down button sound would play upon clicking the color option(doesn't occur with other string related controllers)
REFACTOR: I seemed to have refactored a few things. I did this a month ago, and don't remember anything. It probably doesn't matter, because I have plans for a huge refactor anyways in the near futre
FEAT: Began work on the hue slider's hue choosing mechanisms. It took slightly longer than I feel like it should've to figure this out, but I'm happy with its current implementation, and the ideas I have to improve it. REFACTOR: Huge refactor on the color picker and how it is rendered. Firstly, the values are now almost not-hardcoded at all, being dynamic to the screen's gui size. REFACTOR: Moved the whole color picker to its own class REFACTOR: Removed the test color variable in the "aaaaaaaaa..." category BUG: Color picker's x/xLimit seems to be flipped incorrectly BUG: HUGE lag spike while choosing the value's hue(I'm unsure how I'm going to fix this) TODO: Possible better implementation of the color picker as a whole to be clicked on not using the temporary workaround This is more of a checkpoint for me than anything. I've spent multiple hours each night working on this, and losing this progress would be terrible.
FIX: Apparently fixed the huge lag spike when changing the hue... sometimes. It seems to have something to do with hotswapping, which shouldn't affect anybody outside of dev. env.(will do more testing later) FIX: Fixed the hue slider gradient being incorrectly displayed REFACTOR: Updated the param. usage in gradient related methods. For some reason, I had them sorta pretty janky when being used. While the code for the actual methods seem more confusing, using said methods is now less confusing. Will probably come back to this later REFACTOR: Deleted small bits of commented out code
WIP: Some code for the color picker clicking stuff. Hopefully to be improved soon, but this prototype actually fully works(just implemented in a way I'm not currently happy with)
REFACTOR: Cleaned up some of the prototype mouse click code BUG: Dragging the color picker doesn't work I'm saving this incase I want to revert later. I'm very confident there is a better way than this, however. Which is why I'm going to continue testing new ideas until I find something better.
FEAT: Made it so that the color picker element from a color controller gets added to the YACL Screen's children, which handles rendering and mouse click events FIX: Finally, at last, fixed the color picker mouse clicking z-fighting BUG: Changing the color from the color picker, then typing in the color controller crashes the game This took many hours. I spent a long time trying to fix the color picker being removed crashing the game. I thought there were 5 different reasons for as to why it was crashing before finally finding out the true reason. A huge cleanup and possibly making the ColorPickerElement extend the AbstractWidget class instead of the StringElement class might be better.
REFACTOR: Removed commented out code which previously handled the mouse clicking for the color picker BUG: Attempting to scroll through the YACL screen while hovering over the color picker ceases all scrolling
REFACTOR: Cleaned up the ColorPickerElement class, making it only have methods and variables it needs/will need REFACTOR: Optmizied imports for ColorController class and updated some FIXMEs REFACTOR: Removed old, unused test method in YACLScreen class
FEAT: Made it so that clicking on the hue slider of the color picker will now result with the controller's hue changing REFACTOR: Made it so that the hue slider thumb now properly moves only when it should REFACTOR: Clicking on the saturation/light gradient no longer changes the hue(will have proper support for sat/light gradient soon)
REFACTOR: Updated the background for the color picker, now looking like an inventory container background TODO: I'm most likely going to touch up on most of the main rendering code, as it is a mess
# Conflicts: # common/src/main/resources/yacl.accesswidener
REFACTOR: Completely redesigned the color picker's looks, having an inventory container like background now. REFACTOR: I think I increased the hue slider thumb's height by 2 pixels to adjust for the new redesign to stay satisfying FIX: Fixed the colorPickerDim.y and colorPickerDim.yLimit being flipped(that was easier and harder than I thought it would be both at the same time) FIX: Cleaned up a bunch of messy rendering code. Turns out, it worked, but wasn't as easy to change as I thought. Should be somewhat better now, I might come back and clean it up some more though
FEAT: Made it so that you can now choose the saturation and light values of a color! Holding down the mouse button allows you to leave the dedicated box for easier color picking. Doing the same with the hue slider doesn't affect the saturation/light picking either REFACTOR: If a color is very bright, the mini color preview outline will change to a light grey upon hovering instead of plain white. This is to indicate to new users who may have a very bright color as an option that it can still be clicked on. FIX: Fixed a bug where choosing a color that was too dark would result in the saturation being reset FIX: Fixed a bug where having too dark of a color would mess up the hue slider TEST: Tried making the background of the color picker a texture. I was unsuccessful this time, but I'm going to try again soon.
FIX: Fixed a bug where typing in the color controller while the color picker is visible would result in the color picker desyncing TEMPFIX: Added a temporary workaround to the color picker floating when the color controller wasn't visible anymore. A better fix will be added in the future. The current workaround probably isn't great for performance.
# Conflicts: # common/src/main/java/dev/isxander/yacl3/gui/YACLScreen.java
FEAT: Color picker background texture FEAT: Transparent square texture REFACTOR: Made the background of the color picker use a texture instead of manual rendering REFACTOR: Removed code for manual rendering of the color picker background, as it has been replaced REFACTOR: Removed unused code from the YACLScreen which I didn't mean to commit in the first place CHORE: Optimized imports for ColorController
FEAT: Added some comments a couple weeks ago. I'm going to look into the popupscreen now.
FEAT: Started work on testing the Color Picker as a popup screen Oh, oh my! I've managed to solve more problems in 3 hours than I was able to in 3 weeks with the popup screen-like function!
FIX: Fixed the color picker not always appearing on first click after closing FIX: Fixed the color picker scrolling when it shouldn't be FIX: Fixed the color picker's color controller's color preview's outline(goodness) not highlighting while the color picker was visible
…color picker if there isn't room above the controller, and another scrolling bug fix TEST: Added a new Color Picker test category FEAT: Made it so that if there isn't enough room for a color picker to be easily usable above a color controller, the color picker will appear beneath the color controller FEAT: Started work on alpha-related stuff for the color picker FIX: Fixed a bug where there were about 2 pixels worth of area where scrolling would result in the color picker "desyncing" from the color controller
FEAT: Finished the alpha slider if the color picker's color controller has it enabled FEAT: Added some extra color options to the test config to showcase other features of the color picker
FIX: Fixed a bug where the "fillSidewaysGradient" method was just completely broken and made zero sense. REFACTOR: Improved(hopefully) the "drawRainbowGradient" method's code REFACTOR: Cleaned up some unused methods in the ColorPickerElement REFACTOR: Changed the transparent texture's sizing to 7x7, so that is doesn't clip vertically (still clips a little bit horizontally) REFACTOR: Removed some commented out code in some other classes from previous testing
REFACTOR: Did some additional cleanup in the ColorController.java class to remove really old code that was added before hte ColorPickerElement class was moved outside the ColorController class
|
I've made a lot of the requested changes, including remove unneeded variables, renaming methods to fit your naming scheme, fixing the keyboard inputs on the color picker, and removing the unused access wideners. I've also tried to make all the popup color picker related stuff to a popup controller widget. I ask that you look at this because I'm not feeling super confident about it, but I can't figure out what I'm not feeling confident on. I tried sleeping on it but that didn't help either. I also ask that you consider this feedback and see if I need to change anything.
|
|
How about you flash the outline from black to white and back again slowly when hovering, as if to say something would happen if you click it. |
|
I am not very good with Java generics, so I don't know if this actually poses a problem or if there's an easy solution. Maybe @isXander can weigh in: The new Can this be made work by templatifying |
I believe you can do |
|
Ah yes, |
Crendgrim
reviewed
Jan 14, 2024
common/src/main/java/dev/isxander/yacl3/gui/controllers/ColorPickerElement.java
Outdated
Show resolved
Hide resolved
Crendgrim
reviewed
Jan 14, 2024
common/src/main/java/dev/isxander/yacl3/gui/controllers/PopupControllerScreen.java
Outdated
Show resolved
Hide resolved
common/src/main/java/dev/isxander/yacl3/gui/controllers/PopupControllerScreen.java
Outdated
Show resolved
Hide resolved
common/src/main/java/dev/isxander/yacl3/gui/controllers/PopupControllerScreen.java
Outdated
Show resolved
Hide resolved
FEAT: Added color preview outline fading to the color controller. The color preview outline will slowly flash from white to black indicating to a user to press it to enable the color picker. It has a boolean ready to be changed to a config option for when that gets added.
FIX: Made changes based on feedback to ensure that all popup controller widget related items can easily be used for the dropdown controller, and any future controllers that may need it.
REFACTOR: Made it so that the color picker y is now set by the color controller REFACTOR: The color picker now gets removed if the color controller is partially offscreen REFACTOR: Removed all code related to the manual scrolling of the color picker/popup widget
FIX: Removed the option widget list parameter for the PopupControllerScreen, because it isn't used anymore
Crendgrim
reviewed
Jan 22, 2024
common/src/main/java/dev/isxander/yacl3/gui/controllers/ColorPickerElement.java
Outdated
Show resolved
Hide resolved
Crendgrim
reviewed
Jan 22, 2024
common/src/main/java/dev/isxander/yacl3/gui/controllers/ColorPickerElement.java
Outdated
Show resolved
Hide resolved
REFACTOR: Renamed ColorPickerElement to ColorPickerWidget FIX: Small changes when YaclScreen#clearPopupControllerWidget is called.
FIX: (Hopefully) fixed a bug where the YaclScreen#clearPopupController method would get called twice, instead of just once.
|
Is this ready for review? |
I believe the only thing I'm waiting on now is for you to add the config for the color preview outline to know if it should be flashing or not. I was going to ping you about this on the weekend but you beat me to it :) /lh |
FEAT: Added a boolean to the YACLConfig which determines if a color controller's color preview's outline(color picker indicator) should flash or not. This boolean is on by default, and automatically disables itself upon the color picker's first opening.
…n + config fix REFACTOR: Improved the color picker's implementation detecting if it should be beneath the controller or not(moved order of operations) FIX: Fixed the config option for flashing the color picker indicator not being a serial entry
|
I've resolved the final comments(took me a little bit because I wanted to give them a good look before resolving). If any other changes are needed on my end, please let me know! |
|
I found out there is a request re-review button :) |
… into 1.20.x/dev # Conflicts: # common/src/main/java/dev/isxander/yacl3/platform/YACLConfig.java # src/main/java/dev/isxander/yacl3/gui/YACLScreen.java # src/main/java/dev/isxander/yacl3/gui/controllers/ColorPickerWidget.java # src/main/java/dev/isxander/yacl3/gui/controllers/ControllerPopupWidget.java # src/main/java/dev/isxander/yacl3/gui/controllers/PopupControllerScreen.java
isXander
approved these changes
Apr 11, 2024
|
Finally! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Streamline the color config option by more easily allowing users to choose colors. With support for color controllers that have alpha enabled as well!
Other details:
The color picker works by detecting if a user has their mouse down in the color preview of the color controller. The color picker widget is a custom screen that will render the YACLScreen in its
renderBackgroundmethod. Upon the window being resized, or the color picker screen being closed, the minecraft client screen will be set back to the YACLScreen without reinitializing it. ThePopupColorPickerScreendoesn't extend thePoupScreenas I figured it would be better to have it this way(also I was messing something up somewhere because it didn't let me extend it)I ask that you look over everything just to make sure that it is okay. I'd also like to have you specifically look at method I added in the
ElementListWidgetExtclass. I added a method to get the color picker's y with smooth scroll, as I couldn't determine if there was a better way of doing so.There are also a LOT of commits in this PR. You only need to worry about the last 7-ish, as code in others were either replaced/redone, or were checkpoints for mechanics that were replaced/redone. I don't mind copy-pasting my work into a new fork to have just one commit if you care about commit history.
Beyond that, I also ask for any feedback you may have, as I've never done a PR this big before.