Pick the largest suitable default scale. Fix #468 #472

2024-11-18T18:19:13Z

NQNStudios commented

2024-11-18 18:19:13 +00:00

(Migrated from github.com)

No description provided.

CelticMinstrel (Migrated from github.com) reviewed 2024-11-19 02:58:03 +00:00

src/pcedit/pc.graphics.cpp

						
				@@ -29,7 +29,7 @@ extern fs::path file_in_mem;

				extern short store_flags[3];

CelticMinstrel (Migrated from github.com) commented

2024-11-19 02:58:02 +00:00

What do you mean, they're switched? The rectangle constructor is top, left, bottom, right.

CelticMinstrel (Migrated from github.com) reviewed 2024-11-19 03:00:30 +00:00

src/tools/winutil.hpp

						
				@@ -62,6 +66,16 @@ inline int os_specific_y_offset() {

				#endif

CelticMinstrel (Migrated from github.com) commented

2024-11-19 03:00:29 +00:00

I don't like variable names beginning with an underscore. Wouldn't just scale do just fine?
This is a pretty big function, does it really need to be defined inline in the header?

1. I don't like variable names beginning with an underscore. Wouldn't just `scale` do just fine? 2. This is a pretty big function, does it really need to be defined inline in the header?

NQNStudios (Migrated from github.com) reviewed 2024-11-19 16:28:17 +00:00

src/pcedit/pc.graphics.cpp

						
				@@ -29,7 +29,7 @@ extern fs::path file_in_mem;

				extern short store_flags[3];

NQNStudios (Migrated from github.com) commented

2024-11-19 16:28:17 +00:00

I'm really used to the order being {x, y, width, height} in other frameworks. My bad.

NQNStudios (Migrated from github.com) reviewed 2024-11-19 16:36:52 +00:00

src/tools/winutil.hpp

						
				@@ -62,6 +66,16 @@ inline int os_specific_y_offset() {

				#endif

NQNStudios (Migrated from github.com) commented

2024-11-19 16:36:51 +00:00

Yes
Probably not. I just need to find a different header/source file that is included in the Common sources where it would make sense to be forward-declared/defined. Because if I declare it in winutil.hpp, the definition would have to be repeated in the 3 different winutil.*.cpp/mm files. Or add a winutil.all.cpp file? There have been other moments where I wished there was a winutil.all.cpp file, so maybe that's worth doing.

1. Yes 2. Probably not. I just need to find a different header/source file that is included in the Common sources where it would make sense to be forward-declared/defined. Because if I declare it in winutil.hpp, the definition would have to be repeated in the 3 different winutil.*.cpp/mm files. Or add a winutil.all.cpp file? There have been other moments where I wished there was a winutil.all.cpp file, so maybe that's worth doing.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: openboe/oboe#472

				`@@ -29,7 +29,7 @@ extern fs::path file_in_mem;`
				`extern short store_flags[3];`

				`@@ -62,6 +66,16 @@ inline int os_specific_y_offset() {`
				`#endif`