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

Only draw enabled shapes not based on if selected within the Editor. #18423

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

yaakuro
Copy link
Contributor

@yaakuro yaakuro commented Oct 28, 2024

What does this PR do?

This pull request modifies how shapes are displayed in the viewport. As shown below, the 'Visible' property determines whether the shape remains visible in the editor viewport at all times. Currently it also seem to depend if the entity where this component resides is selected or not. Meaning even if I do set the 'Visible' property to false (not show) the shape will be shown when the entity with the component is selected.

image

Deselecting the entity will then hide it, if 'Visible' property is disabled.
I do however expect the shape not being drawn no matter the entity is selected or not, if this property is set to false.
So I like to propose to change this behavior to honor the setting 'Visible', as the tooltip of this property says:

'Always display this shape in the editor viewport.'

How was this PR tested?

Used only a Box Shape to render in the level. Set the property 'Visible' to false (un-checked).

@yaakuro yaakuro requested review from a team as code owners October 28, 2024 19:37
@AMZN-daimini
Copy link
Contributor

Hi!
First of all, looping in @o3de/sig-ui-ux

I have an issue with this change - shapes can be used by all sorts of components, like ones related to landscape or collision.
In those cases, the collider needs to be invisible during simulation/gameplay, but being able to see it when the entity is selected in the editor allows users to clearly edit its size and proportions to fit their needs, especially through component mode. So I think this should not be changed.

What I can agree on is that the labeling / description could be improved if it caused confusion, or we could add an additional option to always hide if that fits some other workflow that is not accounted for. Feel free to elaborate the need further so we can discuss action :)

@AMZN-daimini AMZN-daimini requested a review from a team October 29, 2024 09:36
@byrcolin byrcolin added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Oct 29, 2024
Copy link
Contributor

@gadams3 gadams3 left a comment

Choose a reason for hiding this comment

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

I think the edit context name and description probably need to be updated, something like “always visible in editor". I think the intention is to allow the shape to be visible in the editor even if it is not selected.

@yaakuro
Copy link
Contributor Author

yaakuro commented Oct 29, 2024

Agree, the Edit behavior needs to be changed, like when in Edit Mode it should show the shape, but here an example what I do not like. I tell the component to do this:

image

I de-selected the 'Visible' property but it is still visible, yes because it is selected. But my choice is ignored. Most of the time I do end up changing then the 'Filled' property. So the current behavior forces me to see this shape if I want it or not. Or, yes renaming it would be better but still, disabling completely would be nice too.
In this case I am having a 'Diffuse Probe Grid' and want to play with the parameters from this distance, but the shape is visible because I need to select the component.
Yeah I know, might be a personal preference thing :D.

@gadams3
Copy link
Contributor

gadams3 commented Nov 7, 2024

Quote reply

No matter what, I recommend updating the display text to say visible in editor or editor visibility. After that you can add another flag to toggle the behavior you're describing to completely hide the shape in all cases. Even better, rename the variable and the display text to editor visibility and make it an enum with visibility modes for never, always, and when selected. Have it default to when selected. You might also need to add a serializer or something to handle the version change to preserve backward compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants