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

Tool and game object selection bugfixes #160

Merged

Conversation

Dgzt
Copy link
Collaborator

@Dgzt Dgzt commented Apr 27, 2023

1) Clearing selection

Currently if select a game object and press ESC key then the selection is clearing but the game object is selected in the Outline and see it's properties on Inspector. And if change a property in Inspector then it doesn't change on game object.

Before fix:
clear_selection_before

After fix:
clear_selection_after

2) Selecting a brush tool

If selected a brush tool on terrain then the previous tool still selected on the toolbar, but not this tool the active. It's misleading. After fix, the previous selected tool (any tool from toolbar, like translate tool, rotate tool, etc) will be deselected.

Before fix:
terrain_brush_tool_selection_before

After fix:
terrain_brush_tool_selection_after

3) Going back from brush tool with ESC key

Currently If selected a terrain brush tool and pressing ESC key then brush tool will be deactivated but the previous tool on toolbar still visible as active but it doesn't active, the wireframe doesn't visible. After fix after pressing ESC key then the translate tool will be active and visible as active. And if press again ESC, then clear selection, see at 1).

Before fix:
back_from_brush_tool_with_esc_key_before

After fix:
back_from_brush_tool_with_esc_key_after

4) Selecting tool from toolbar while in brush tool

Currently if selecting a tool from toolbar while we are using a brush tool then the tool on toolbar will be visible as active but it doesn't active, the wireframe doesn't visible.

Before fix:
select_tool_from_toolbar_while_in_brush_tool_before

After fix:
select_tool_from_toolbar_while_in_brush_tool_after

5) Selecting asset while a tool was selected from toolbar

Currently if selecting an asset from assert dockbar while a tool was selected on a game object then the Outline still shows the previously selected game object and the wireframe still visible but the Inspector shows the asset''s data.

Before fix:
select_asset_while_toolbar_tool_active_before

After fix:
select_asset_while_toolbar_tool_active_after

6) Selecting asset while a terrain brush tool was selected

Currently if selecting an asset while a brush tool was selected then the brush tool still active.

Before fix:
select_asset_while_brush_tool_selected_before

After fix:
select_asset_while_brush_tool_selected_after

7) Editing position/rotation/scale while a terrain brush tool was selected

Currently if editing position or rotation or scale while a terrain brush tool was selected then nothing happens. After fix the whole transformation widget will be disabled if a brush tool was selected.

Before fix:
edititng_transformation_while_in_brush_tool_before

After fix:
edititng_transformation_while_in_brush_tool_after

8) Changing tab on terrain component widget after selected a brush tool

Currently if selected brush tool on a tab on terrain component widget and changing tab the selected tool still active and you don't see what tool active at the moment. After fix if changing tab on terrain component widget then the active tool will be the default translate tool.

Before fix:
changing_tab_after_selected_brush_tool_before

After fix:
changing_tab_after_selected_brush_tool_after

9) Editing name/visibility/tag while a terrain brush tool was selected

Similar to 7). Currently if editing name, visibility or tag in identifier widget while a terrain brush tool was selected then nothing happens. After fix the whole identifier widget will be disabled if a brush tool was selected.

Before fix:
editing_field_in_inspector_while_in_brush_tool_before

After fix:
editing_field_in_inspector_while_in_brush_tool_after

10) Missing highlight for selected texture on paint tab.

Currently if want to paint on a terrain the currently selected texture is not highlighted. After fix at paint tab open the first texture will be selected and highlighted and if select a texture it will be highlighted.

Before fix:
missing_highlight_for_selected_texture_on_paint_tab_before

After fix:
missing_highlight_for_selected_texture_on_paint_tab_after

@Dgzt Dgzt marked this pull request as ready for review April 27, 2023 14:54
@Dgzt
Copy link
Collaborator Author

Dgzt commented Apr 27, 2023

@JamesTKhan @antzGames (and anyone else) What do you think these changes? Do you know any other scenario with tools?

@Dgzt Dgzt added the bug Something isn't working label Apr 27, 2023
@antzGames
Copy link
Collaborator

@Dgzt

  1. I like these changes a lot.
  2. You covered the annoying ones for sure. I cannot think of any others right now.

I can test your branch maybe this weekend.

@Dgzt
Copy link
Collaborator Author

Dgzt commented Apr 28, 2023

I've found that if editing position or rotation or scale while a terrain brush tool was selected then nothing happens. Added as 7). The fix is missing yet.

@Dgzt Dgzt marked this pull request as draft April 28, 2023 19:56
@antzGames
Copy link
Collaborator

Did some testing. Here are my results and comments:

  1. Clearing selection - works great. OK
  2. Selecting a brush tool - I am confused what is the difference between the 2 videos (before/after)
  3. Going back from brush tool with ESC key - works great. Love the double ESC (links to 1) brings you back to nothing selected.
  4. Selecting tool from toolbar while in brush tool - OK
  5. Selecting asset while a tool was selected from toolbar - OK - Yes this needed to be fixed.
  6. Selecting asset while a terrain brush tool was selected - OK - Yes this needed to be fixed.
  7. waiting for fix

I think Up/Down > Flatten > Smooth tab TOOLS should all be in one TAB (something like below pic).

The reason is that you will absolutely know which tool (up/down or flatten or smooth) is selected because its highlighted. Right now you need look at all the tabs to see what is selected. I know this is more work. Just wanted to let you know my opinion.

What is the benefit of having these 3 tools on separate tabs?

image

@Dgzt
Copy link
Collaborator Author

Dgzt commented Apr 29, 2023

@antzGames Thank you that tested changes and wrote comment.

2. Selecting a brush tool - I am confused what is the difference between the 2 videos (before/after)

After selecting a brush tool the previous selected tool on toolbar (like translate tool, scale tool, etc) still visible as active. After fix the previous tool will be deselected. I've updated the description.

I think Up/Down > Flatten > Smooth tab TOOLS should all be in one TAB (something like below pic).

The reason is that you will absolutely know which tool (up/down or flatten or smooth) is selected because its highlighted. Right now you need look at all the tabs to see what is selected. I know this is more work. Just wanted to let you know my opinion.

What is the benefit of having these 3 tools on separate tabs?

image

Thank you that found this issue, I will ad to the list. I think it is better if all brush tools are on separate tab, because each brush tool has different tip and later can be more options. What do you think if change tab on Terrain Component widget then applications clears brush tool selection and selects translate tool on toolbar. Like if you press ESC button, in 3). Like this:

Peek 2023-04-29 22-23

It is very easy to implement.

@Dgzt
Copy link
Collaborator Author

Dgzt commented Apr 29, 2023

... I will ad to the list. ...

I've added as 8).

@antzGames
Copy link
Collaborator

Thank you that found this issue, I will ad to the list. I think it is better if all brush tools are on separate tab, because each brush tool has different tip and later can be more options. **What do you think if change tab on Terrain Component widget then applications clears brush tool selection and selects translate tool on toolbar. Like if you press ESC button, in 3).**

Yes this will work too.

@Dgzt
Copy link
Collaborator Author

Dgzt commented Apr 30, 2023

I've found that if editing any field in identifier widget while a terrain brush tool is selected then nothing happens. Added as 9).

@Dgzt
Copy link
Collaborator Author

Dgzt commented Apr 30, 2023

I've found the highlight for selected texture is missing on paint tab. Added as 10).

@Dgzt Dgzt marked this pull request as ready for review April 30, 2023 19:59
@Dgzt
Copy link
Collaborator Author

Dgzt commented Apr 30, 2023

I've fixed all issues what I've found. If you test this PR and find an issue related this please let write a comment.

@antzGames
Copy link
Collaborator

antzGames commented May 1, 2023

  1. So the selected texture on paint is the green highlight. Can we just have a green box around it? or a green check mark? You cannot see the selected texture's color. At first I thought it was a bug. But once I clicked on another texture to paint I saw what you were doing. Still testing.

image

@antzGames
Copy link
Collaborator

  1. Editing position/rotation/scale while a terrain brush tool was selected: OK
  2. Changing tab on terrain component widget after selected a brush tool: OK
  3. Editing name/visibility/tag while a terrain brush tool was selected: OK
  4. Missing highlight for selected texture on paint tab.: OK but see my comment in previous post.

@Dgzt
Copy link
Collaborator Author

Dgzt commented May 1, 2023

  1. So the selected texture on paint is the green highlight. Can we just have a green box around it? or a green check mark? You cannot see the selected texture's color. At first I thought it was a bug. But once I clicked on another texture to paint I saw what you were doing.

I've changed to this:
image
Yes, you are right. It's better.

I've updated video.

@JamesTKhan
Copy link
Owner

JamesTKhan commented May 3, 2023

This looks awesome! Thanks for tracking down all these annoying little bugs. I have encountered them but never sat down and tried to list them all out.

I noticed one interesting thing, I didn't even realize SceneGraph had a selected variable for GameObject. That seems weird since EditorScene already tracks that. I see that you are updating the value

projectManager.current().currScene.currentSelection = selectedGameObject
projectManager.current().currScene.sceneGraph.selected = selectedGameObject

As far as I can tell looking through the code sceneGraph.selected is not used anywhere (except being set, but never is read/used) at all. Might be some old left over code. I think we can remove private GameObject selected; and its getters and setters from Scenegraph entirely.

If you can remove it, go ahead, otherwise remove your code that is setting sceneGraph.selected and I will remove the variable later.

@Dgzt
Copy link
Collaborator Author

Dgzt commented May 3, 2023

This looks awesome! Thanks for tracking down all these annoying little bugs. I have encountered them but never sat down and tried to list them all out.

I noticed one interesting thing, I didn't even realize SceneGraph had a selected variable for GameObject. That seems weird since EditorScene already tracks that. I see that you are updating the value

projectManager.current().currScene.currentSelection = selectedGameObject
projectManager.current().currScene.sceneGraph.selected = selectedGameObject

As far as I can tell looking through the code sceneGraph.selected is not used anywhere (except being set, but never is read/used) at all. Might be some old left over code. I think we can remove private GameObject selected; and its getters and setters from Scenegraph entirely.

If you can remove it, go ahead, otherwise remove your code that is setting sceneGraph.selected and I will remove the variable later.

Good catch. I removed this field.

@JamesTKhan JamesTKhan merged commit 2fda251 into JamesTKhan:master May 6, 2023
@Dgzt Dgzt deleted the tool-and-gameobject-selection-bugfixes branch May 9, 2023 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants