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

Submarine Editor Transform Tool #15469

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

Conversation

Jade-Harleyy
Copy link

@Jade-Harleyy Jade-Harleyy commented Jan 4, 2025

This PR adds a rotation & scaling tool to the submarine editor.
Its functions can be toggled with the tickboxes to the right of the "Generate Waypoints" button.

With "Edit Rotation" active, users can drag the widget around to rotate the selected entities about the center of the selection.
With "Edit Scaling" active, users can drag the widget around to scale the selected entities about the center of the selection.

The current rotation and/or scaling offset is visible in a tooltip next to the widget while it is being dragged.

Added content is in the mod provided with the PR.
The tickboxes also require their own art to be done, hence the duplicate styles I have added.

image

@Regalis11 Regalis11 self-requested a review January 7, 2025 12:11
@Regalis11 Regalis11 added Code Programming task Design Design-related task labels Jan 7, 2025
Copy link
Collaborator

@Regalis11 Regalis11 left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! This seems like a really handy addition. The code changes look mostly good, but I spotted a couple of things that could have some negative performance implications and would most likely be easy to optimize.

Copy link
Collaborator

@Regalis11 Regalis11 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 there's still an issue with how the behavior of the Hovered callback was modified.

@@ -112,9 +112,9 @@ public virtual void Update(float deltaTime)
{
PreUpdate?.Invoke(deltaTime);
if (!enabled) { return; }
if (IsMouseOver || (!RequireMouseOn && SelectedWidgets.Contains(this) && PlayerInput.PrimaryMouseButtonHeld()))
if (IsMouseOver) { Hovered?.Invoke(); }
if (PlayerInput.PrimaryMouseButtonHeld() && (IsMouseOver || !RequireMouseOn && SelectedWidgets.Contains(this)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This still changes the behavior of the method. Previously Hovered was called if the cursor is on the widget, or if it's being dragged and doesn't require the mouse to be on it to be draggable.

If you want the new widgets to behave in some other way, I think that should be handled in the callbacks or by disabling RequireMouseOn.

@Regalis11 Regalis11 added the Waiting Cannot be worked on/reviewed/tested at the moment for some reason label Jan 21, 2025
@SomeRandomNoobKekeke
Copy link

SomeRandomNoobKekeke commented Jan 21, 2025

  1. If you press esc while the widget is grabbed you won't be able to release it, and if you quit to main menu and enter sub editor again the widget will become ungrabable. Because it's still in Widget.SelectedWidgets? God knows how baro gui works
    So i think you should somehow clear SelectedWidgets on quiting

  2. I think it should react to moving camera with wasd / mouse scroll while grabbed. Barotrauma widgets do, so i think it's possible

  3. Widget should appear at the angle of rotation of the item and initial scale should be set to the scale of the item. I understand that you work with selections, but make an exception if only one item is selected, this is deceiving, it shows you that item always have 0 rotation and 1 scale whenever you grab it

  4. Can numbers in the widget have fixed level of precision?

  5. When placing walls by dragging why do you snap them to grid? Placing and rotating them manually doesn't have this limitation

  6. When you finish placing a wall by dragging, the wall jumps dramatically ruining your precise positioning

Revert changes to Widget.Update in favor of custom implementation.
Keep widget at a fixed distance when only rotation is enabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Programming task Design Design-related task Waiting Cannot be worked on/reviewed/tested at the moment for some reason
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants