FakeFishGames / Barotrauma

A 2D online multiplayer game taking place in a submarine travelling through the icy depths of Jupiter's moon Europa.
http://www.barotraumagame.com/
1.67k stars 394 forks source link

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

Open Jade-Harleyy opened 2 months ago

Jade-Harleyy commented 2 months ago

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 commented 2 months ago

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

Jade-Harleyy commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

So basically int minResizableItemSize = 2;? Sounds fair.