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

Clean up UI of ReflectionProbe #99920

Merged

Conversation

lander-vr
Copy link
Contributor

This PR cleans up some of the UI of ReflectionProbes.
Proposal: godotengine/godot-proposals#11269

  • Removed corner to center-offset lines
  • Only show center-offset representation cross when center-offset actually has an offset
    • I would prefer to use an icon that shows up when the probe is selected, instead of the cross, but it doesn't look like add_unscaled_billboard supports offsets.
  • Removed handles for the center-offset
  • Significantly reduced opacity on solid box

Still missing from that proposal:
Improved bounding box handles feedback and depth, however that should be an improvement for all objects using handles, for which a separate PR makes more sense. (#60474)

Master this PR
image image
image image

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.

Tested locally, it works as expected.

image

We should also decrease VoxelGI's gizmo opacity, as it's now much more opaque than ReflectionProbe's. Can you look into this as well?

image

@lander-vr
Copy link
Contributor Author

Tested locally, it works as expected.

Is there anything else I should do to get this approved? :)

@Calinou
Copy link
Member

Calinou commented Dec 3, 2024

Is there anything else I should do to get this approved? :)

Could you also reduce the VoxelGI gizmo's opacity? It makes sense to do this in the same PR since they're closely related.

@lander-vr
Copy link
Contributor Author

lander-vr commented Dec 3, 2024

Could you also reduce the VoxelGI gizmo's opacity? It makes sense to do this in the same PR since they're closely related.

Already did that in a separate PR, thought it would make more sense since this PR includes a bit more than just adjusting the opacity of the materials.
#99969

@clayjohn clayjohn modified the milestones: 4.x, 4.4 Dec 3, 2024
@Repiteo Repiteo merged commit 370e5f3 into godotengine:master Dec 5, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 5, 2024

Thanks!

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.

5 participants