adventuregamestudio / ags

AGS editor and engine source code
Other
707 stars 159 forks source link

Winsetup: rearrange into a multi-paged dialog #2527

Closed ivan-mogilko closed 2 months ago

ivan-mogilko commented 2 months ago

This is a proposal to rearrange WinSetup dialog into a multi-paged dialog, which is more convenient when you need to add extra options.

Currently provides following pages:

I moved "custom paths" to a separate page, because it seems more convenient to make them stand out, and also simply because they were making "advanced" tab too high compared with "basic".

This is achieved with almost no changes to the code that handles setup controls, that was mostly just moved around into separate classes. Majority of changes is code restructuring and writing wrapper around WinAPI's tab control.

Example screenshot:

winsetup--tabbed

ericoporto commented 2 months ago

Looks great. The old had a problem that if you clicked advance you are now advance forever, so this is a lot better.

image

This blank space here, I think I want to try to add a feature to throw a banner there with the game logo.

Ah, that option of 85 Hz for CRT, is the most curious option I have ever seen, every time I look at it I stop and contemplate.

Anyway, the changes looks alright for merging too, the code in separate files is a lot saner.

ivan-mogilko commented 2 months ago

that option of 85 Hz for CRT, is the most curious option I have ever seen

I don't know what this setting is for, and never seen this sort of standalone option in any other engines or games. I suppose this refresh rate should be a part of the driver's fullscreen mode selection instead, if at all.

But looking through the code in the master branch, I see that this setting is not used at all. It is only ever mentioned in Direct3D renderer (maybe after SDL port). and even there it's disabled: c57d04cad95368dbcc9991654eec849e1e7e023e

I think this had to be removed long time ago.

ericoporto commented 2 months ago

Well, I haven't seen a crt monitor in a long time I am alright with it being removed

ivan-mogilko commented 2 months ago

Ok, I removed 85hz checkbox, starting from release-3.6.1 branch.