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

Remove exp hint of a few properties #80326

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

MewPurPur
Copy link
Contributor

@MewPurPur MewPurPur commented Aug 6, 2023

image

EditorSpinSlider, used for our properties, extends Range, and it likewise doesn't work if min is 0 or less, yet we have a few dozen properties that use exp and start with 0.

I wanted to add support for some kind of exponential easing without these restraints, but I'm not sure if I'll finish it, so I just wanted to start with removing a few "exp" hints where exponential easing doesn't make much sense.

First is CameraAttributes, where the min value of DOF near/far transition is -1 and so the exp hint didn't work in the editor. I assume -1 is something representing an invalid value.

Second is CSGShape, where the min value of angle simplification is 0, meaning no angle simplifications will be made. I get how you're likely to want smaller angle simplifications rather than big ones towards the max value 180, but I don't think exponential easing makes sense here, as it's something you use when different common values of a property are orders of magnitude apart.

@MewPurPur MewPurPur requested a review from a team as a code owner August 6, 2023 09:26
@Calinou Calinou added bug topic:editor cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Aug 6, 2023
@Calinou Calinou added this to the 4.2 milestone Aug 6, 2023
@YuriSizov YuriSizov added cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release and removed cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Nov 10, 2023
@YuriSizov YuriSizov modified the milestones: 4.2, 4.3 Nov 10, 2023
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

This makes sense. Depth of field transition values are distances, not attenuation falloffs, so they shouldn't be configured as exponential.

Likewise, angles are not falloffs either, so it makes sense to remove the hint for Path Simplify Angle too.

@akien-mga akien-mga merged commit 94edf0f into godotengine:master Dec 4, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@MewPurPur MewPurPur deleted the no-exp-with-zero branch December 5, 2023 02:20
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.2.1.

@YuriSizov YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants