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

Allow resizing structures down to 2 cm instead of 16 cm. #13921

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

Conversation

Jade-Harleyy
Copy link

@Jade-Harleyy Jade-Harleyy commented May 3, 2024

I've ran into this problem a couple times, where I needed to have a resizable structure smaller than 16 cm, but I wasn't allowed to make it any smaller. This PR fixes that.

I chose 2 cm instead of 1 cm since the handles would overlap at 1 cm.
image

@mygamingaccount
Copy link

mygamingaccount commented May 6, 2024

Fitting to grid size seems like an arbitrary choice, but then again so does 2. Why not 1?

@Jade-Harleyy
Copy link
Author

Fitting to grid size seems like an arbitrary choice, but then again so does 2. Why not 1?

As I stated in the PR, I chose 2 as the minimum since, even when fully zoomed in, the resizing handles would overlap, making objects harder to work with.

@lewri
Copy link

lewri commented May 7, 2024

While I'm not a person accustomed to the Barotrauma code I do question whether Barotrauma has a policy to deter magic numbers. You've replaced a standardised variable (Submarine.GridSize.X / Submarine.GridSize.Y) with a magic number (2) without context here given to it. It may be beneficial to give the number a single referenced point that then equals 2.
(of course if magic numbers aren't an issue to the devs I'll stay quiet and run away again 😅)

@mygamingaccount
Copy link

So basically int minResizableItemSize = 2;? Sounds fair.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants