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

Allow using non-HDR color pickers on gradient options #290

Merged
merged 8 commits into from
Jan 7, 2025
Merged

Allow using non-HDR color pickers on gradient options #290

merged 8 commits into from
Jan 7, 2025

Conversation

2394425147
Copy link
Contributor

Description

Summary

Added an option to use non-HDR color fields on gradation color 1 and 2

Motivation

In some versions of Unity, HDR color pickers doesn't have a HEX color field. This makes pasting colors somewhat difficult if a game uses an external design software.

Additionally, HDR color pickers may not be in the same color space as the non-HDR color fields, making the color in the field preview inaccurate.

Dependencies

None added

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Update documentations
  • Others (refactoring, style changes, etc.)

Test environment

  • Platform: Editor(Windows)
  • Unity version: 2020.3.48f1
  • Build options: Mono, .Net Standard 2.0, BIRP

Checklist

  • This pull request is for merging into the develop branch
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have checked my code and corrected any misspellings

image

@2394425147 2394425147 requested a review from mob-sakai as a code owner January 5, 2025 16:50
@2394425147
Copy link
Contributor Author

Expected functionality:
By default, color fields used by horizontal, vertical, radial fast, radial detail, diagonal to right bottom, diagonal to left bottom and angle display an HDR color field.
{42394D8F-F5F2-476F-97D8-5682EAB980C2}

When a user navigates to the UI Effects settings page, they should see a checkbox HDR Gradient that reflects the current color field type.
{AFE41CDC-C5F6-430A-9AB9-340CB9D0A3C5}

When the user toggles this checkbox, the UIEFFECTS_GRADIENT_NO_HDR flag is added to, or removed from the current build target's scripting define symbols.
{A97AA27E-2268-4A89-B792-EFB495A95F78}

This triggers the compiler macro for this flag to either use an HDR color picker, or the standard color picker.
image
{AD2FC37C-0BB9-4D07-B1E9-C444FADA0C8A}

@mob-sakai
Copy link
Owner

Thank you for your PR!
It includes detailed explanations and documentation updates, which is fantastic! 👍

  • To unify user experience, it would be even better if the ability to enable/disable HDR mode were extended not just to Gradient Color 1 and 2 but to all configurable colors:
    • Color Filter > Color
    • Transition Filter > Transition Color
    • Shadow Mode > Shadow Color
  • Please apply the code style (indentation and spacing) defined in the .editorconfig file.
  • From an SSOT (Single Source of Truth) perspective, HDR Gradient should be saved in UIEffectProjectSettings. Additionally, the InitializeOnLoad attribute should be used to set the ScriptingDefineSymbol as needed.
    • This ensures the same settings can be restored even when switching platforms or if a commit was missed.

@mob-sakai mob-sakai self-assigned this Jan 6, 2025
@2394425147
style: format
31ad9e2
@2394425147
feat: use m_UseHdrColorPicker field inside UIEffectProjectSettings.…
7b39d00 
…cs with custom color picker fields instead of using a scripting define
@2394425147
Copy link
Contributor Author

2394425147 commented Jan 7, 2025 •
edited
Loading

Thanks for reviewing!
I decided to replace the scripting define with a field inside UIEffectProjectSettings. The UIEffectEditor then reads the exposed static property to override the color picker drawers through IMGUI.

image
image

This removes the need to recompile the code whenever that option is toggled, and follows the SSOT principle.
I've also reformatted the UIEffect script according to the project format.

@2394425147
Copy link
Contributor Author

Fields inside Color Filter > Color, Transition Filter > Transition Color, and Shadow Mode > Shadow Color are now also affected by this option.

{13F047CA-8CA8-40FA-9D69-3437C52EBCC2}
{8502B064-5EF1-42D2-B502-E8D626838F7D}
{38496BB0-D832-4B94-BD49-B4364758178E}

@mob-sakai
Copy link
Owner

LGTM, Thank you 👍

@mob-sakai mob-sakai merged commit f504975 into mob-sakai:develop Jan 7, 2025
21 checks passed
@mob-sakai
Copy link
Owner

mob-sakai commented Jan 7, 2025 •
edited
Loading

I will make some adjustments to your commit, and it is scheduled to be released as version 5.2.0.

@2394425147
Copy link
Contributor Author

Sounds great, thank you so much!

mob-sakai pushed a commit that referenced this pull request Jan 7, 2025
@2394425147 @mob-sakai
feat: add Use HDR Color Picker option for project settings (editor)
1be0cd4 
close #290
github-actions bot pushed a commit that referenced this pull request Jan 7, 2025
@semantic-release-bot
chore(release): 5.2.0 [skip ci]
811f576 
# [5.2.0](5.1.0...5.2.0) (2025-01-07)

### Bug Fixes

* `Gradation` properties do not update when a preset is loaded ([edf6943](edf6943)), closes [#291](#291)
* an error occurs when saving a preset ([894c561](894c561)), closes [#293](#293)
* fix gradation gradient edge (angle gradient) ([0ef6027](0ef6027))

### Features

* `Angle Gradient` gradation supports fixed mode gradient ([7cb8325](7cb8325))
* `Angle Gradient` gradation supports RectTransform pivot ([2f63adf](2f63adf))
* add `Use HDR Color Picker` option for project settings (editor) ([1be0cd4](1be0cd4)), closes [#290](#290)
* the `Allow To Modify Mesh Shape` property should remain unchanged when loading a preset ([f7175c0](f7175c0)), closes [#294](#294)
* the `Sampling Scale` property should remain unchanged when loading a preset ([238a17d](238a17d)), closes [#292](#292)
@mob-sakai
Copy link
Owner

🎉 This PR is included in version 5.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@2394425147 2394425147 deleted the develop branch January 8, 2025 12:21
@2394425147 2394425147 restored the develop branch January 8, 2025 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mob-sakai mob-sakai Awaiting requested review from mob-sakai mob-sakai is a code owner

Assignees

@mob-sakai mob-sakai

Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@2394425147 @mob-sakai