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

Move UV editor to bottom dock, rename to Polygon editor #99439

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

aXu-AP
Copy link
Contributor

@aXu-AP aXu-AP commented Nov 19, 2024

Implements godotengine/godot-proposals#8209 (with the exception of refactoring input function).
kuva

  • Move polygon editor to the bottom dock similiarly how tilemaps, animations etc. are handled.
  • Reorder the modes to reflect usual workflow (move UV to third place)
  • Refactor enums for modes, actions and menu items.
  • Rename bunch of stuff (mainly remove "uv" prefixes) to more understandable, and reorder some members in header file.
  • Some smaller cleanups

The pr is currently in multiple commits in case it's easier to review (for example, the functional change is just a few lines compared to refactoring). Or if some changes are deemed unwelcome, or better split to separate prs, we can cherry pick more easily. I will squash them eventually.

Move UV editor to bottom panel to allow realtime preview of results. Also allow UV editor to be used with textureless polygons.
Remove "uv" prefixes and clarify some names. Reorder/regroup some members in header file.
@aXu-AP
Copy link
Contributor Author

aXu-AP commented Nov 19, 2024

Marked as draft because of that one failing check. Any help to solve it would be appreciated...

@@ -50,50 +50,60 @@ class VScrollBar;
class Polygon2DEditor : public AbstractPolygon2DEditor {
GDCLASS(Polygon2DEditor, AbstractPolygon2DEditor);

enum Mode {
MODE_EDIT_UV = MODE_CONT,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This enum is some kind of continuation from AbstractPolygon2DEditor, but the usage is totally unrelated (especially after removing the dialog opening "uv" button), so I changed the code to reflect that.

Comment on lines 85 to 95
VBoxContainer *polygon_edit = nullptr;
Mode current_mode;
Button *mode_buttons[MODE_MAX];
Ref<ButtonGroup> mode_button_group;
Action selected_action;
Button *action_buttons[ACTION_MAX];
Button *b_snap_enable = nullptr;
Button *b_snap_grid = nullptr;
Panel *uv_edit_background = nullptr;
MenuButton *edit_menu = nullptr;

Control *canvas = nullptr;
Panel *canvas_background = nullptr;
Copy link
Contributor Author

@aXu-AP aXu-AP Nov 19, 2024

Choose a reason for hiding this comment

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

There's no new properties here, but some are renamed (eg. uv_edit_draw -> canvas) and rearranged for better grouping (eg. edit_menu grouped with other toolbar objects instead of scrollbars).

}
get_tree()->connect("process_frame", callable_mp(this, &Polygon2DEditor::_center_view), CONNECT_ONE_SHOT);
// Whenever polygon gets redrawn, there's possible changes for the editor as well.
node->connect("draw", callable_mp(draw, &CanvasItem::queue_redraw));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of adding redraw into undoredo actions, we should redraw whenever the target is redrawn. This solves the issue where uv editor doesn't update if the polygon is modified by other actors (like inspector).

bone_paint_radius_label->show();
_update_bone_list();
bone_paint_pos = Vector2(-100000, -100000); //send brush away when switching
void Polygon2DEditor::_select_mode(int p_mode) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made this function more concise by hiding all toolbar elements and then reshowing them depending on mode. Old method was error prone, whenever new controls would be added to some mode there was the risk of forgetting to hide it in other modes. Now every mode simply enables the tools it needs.

@fire
Copy link
Member

fire commented Nov 20, 2024

I have no idea, so I ran the failed job again. Will investigate.

Free the editor upon exiting.
Robystify mode changing logic and use it to initialize the toolbar upon entering the editor.
@aXu-AP aXu-AP marked this pull request as ready for review November 21, 2024 12:26
@aXu-AP aXu-AP requested a review from a team as a code owner November 21, 2024 12:26
@aXu-AP
Copy link
Contributor Author

aXu-AP commented Nov 21, 2024

Managed to fix the problem. Since the editor is outside the tree when it's not in the dock, it won't free itself upon closing. Added cleanup in the NOTIFICATION_PREDELETE (I hope that's the correct place for it).

Now the pr is ready for review :)

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.

3 participants