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

Portals - Improve UI and add shortcuts #51152

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

lawnjelly
Copy link
Member

@lawnjelly lawnjelly commented Aug 1, 2021

This PR makes the 'convert rooms' button permanently on the toolbar and accessible whichever node is selected, so you can convert rooms without having to select the RoomManager first.

It also adds a togglable item 'view portal culling' to the 'View' menu which is a simple way of setting the RoomManager 'active' setting without the RoomManager being the selected node.

Both of these have keyboard shortcuts, which should make it much faster to reconvert rooms and edit.

In addition there the string in the 'Perspective' Listbox is modified to show [portals active] when portal culling is operational, for visual feedback. This is updated when you change modes, and when the rooms are invalidated.

The rooms convert button is to the right of the camera button. It disappears when there is no RoomManager in the scene.. it could alternatively be greyed out.
portals_new_ui

Notes

  • I've tried various places for these two buttons but this seems to make the most sense so far. I'd welcome other ideas though.
  • The buttons need to be accessible continuously in the editor for the shortcuts to work. I did also try creating the 'convert rooms' button in the room_manager_editor_plugin, and simply not hiding it, but it didn't seem like it was easy to control the location on the toolbar unless I put it in the spatial_editor_plugin.
  • One thing I'm not quite sure about, is that now the convert rooms button is permanently on the toolbar, it may not be so obvious to first time users because it doesn't have a big text label. Perhaps I could make a label appear when the RoomManager is selected? EDIT : Have done this.
  • In most cases the [portals active] wording does react instantly when you change things, but there are a couple of instances where there is a delay until you click on that window. This doesn't seem serious but I'll see if I can track them down.
  • When you change the view_portal_culling toggle, the rooms_active toggle in the RoomManager is also changed and the tick updated. The names aren't quite the same. Not sure how to improve the naming situation.
  • I have no idea what the best shortcuts should be. So far I'd been using ALT+P to show and hide portal culling and ALT+C to convert the rooms. But I think there may be a conflict as when I press ALT+C in some Godot windows, it tries to select things.

@steffendietz
Copy link

Regarding

One thing I'm not quite sure about, is that now the convert rooms button is permanently on the toolbar, it may not be so obvious to first time users because it doesn't have a big text label.

Maybe show the new buttons and the "View portal culling" submenu entry only if a RoomManager is present in the current scene.

@lawnjelly
Copy link
Member Author

Maybe show the new buttons and the "View portal culling" submenu entry only if a RoomManager is present in the current scene.

Yup it does this already. The convert_rooms button is hidden and the the view_portal_culling is greyed out. But they can both be hidden or both greyed out depending on what we like best.

This PR makes the 'convert rooms' button permanently on the toolbar and accessible whichever node is selected, so you can convert rooms without having to select the RoomManager first.

It also adds a togglable item 'view portal culling' to the 'View' menu which is a simple way of setting the RoomManager 'active' setting without the RoomManager being the selected node.

Both of these have keyboard shortcuts, which should make it much faster to reconvert rooms and edit.

In addition there the string in the 'Perspective' Listbox is modified to show [portals active] when portal culling is operational, for visual feedback. This is updated when you change modes, and when the rooms are invalidated.
@lawnjelly lawnjelly marked this pull request as ready for review August 2, 2021 08:16
@lawnjelly lawnjelly requested review from a team as code owners August 2, 2021 08:16
@lawnjelly
Copy link
Member Author

Marking as ready for review to get some eyes on. I suspect we may need different shortcuts, I know next to nothing about choosing them (they are ALT+C and ALT+P at the moment). I don't even know if these are available on Mac.

@akien-mga
Copy link
Member

I have no idea what the best shortcuts should be. So far I'd been using ALT+P to show and hide portal culling and ALT+C to convert the rooms. But I think there may be a conflict as when I press ALT+C in some Godot windows, it tries to select things.

From a quick look it should be fine:

$ rg "KEY_MASK_ALT .* KEY_P"
editor/editor_node.cpp
6199:   ED_SHORTCUT("editor/filter_files", TTR("Filter Files..."), KEY_MASK_CMD + KEY_MASK_ALT + KEY_P);
$ rg "KEY_MASK_ALT .* KEY_C"

@akien-mga akien-mga merged commit 73c6ab0 into godotengine:3.x Aug 2, 2021
@akien-mga
Copy link
Member

Thanks!

@lawnjelly lawnjelly deleted the portals_improve_ui branch August 2, 2021 13:58
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