X-Hax / sa2-mod-loader

Sonic Adventure 2 PC Mod Loader
49 stars 15 forks source link

Window fix #50

Closed ItsEasyActually closed 1 week ago

ItsEasyActually commented 1 month ago

Due to some unfortunate tomfoolery and other general we made some mistakes and here's hoping we can rectify them.

Proposal: Window Config for the internal maintainAspectRatio option will be enabled by default, and will be disabled if the corresponding settings settings.KeepAspectOnResize is set to true. This will effectively allow a "stretch to window" behavior. This should be close to 1:1 with the original behavior before the merge into the SAMM with only the bool handling being inversed now.

Video showcasing changes in action (Please be aware that the Keep Aspect On Resize has not been renamed in this video, see the Notes section for more info):

https://github.com/X-Hax/sa2-mod-loader/assets/8052134/4d483193-3f46-433b-b390-a63d064edda2

Notes:

kellsnc commented 1 month ago

After testing it is indeed the original behavior but with the setting inversed.

Regarding the setting we can either keep it inversed on the loader side (which may impact end users), or only have it inversed on the manager side (which may be confusing.)

Once we settle on functionality I can rework the SADX version.

Thanks!

ItsEasyActually commented 1 month ago

Following this up with discussion kellsnc and I had after making this PR. We agreed it should be okay to make the changes to the settings configuration on the manager's side and then update the loader to follow those settings as necessary.

My reason for this decision boils down to two points:

  1. The Keep Aspect Ratio on Resize setting, as it was originally made, being completely useless going forward. It's easier to just change it internally in the config on the Manager's side. It will be made into a Stretch to Window Config option.
  2. It's much better for us to make changes where we keep option names consistent instead of using names that don't make sense for their intended setting, even internally, going forward.

Will update this PR once the Manager side has been updated with the necessary changes, and we can proceed with the updates.

ItsEasyActually commented 1 month ago

Manager side as been updated with the new settings, PR open here. That PR will remain open and pending until the changes on this side have been finalized and ready to released.

kellsnc commented 1 month ago

Following changes have been made:

ItsEasyActually commented 1 week ago

Went testing as we're gearing up to do releases across the board for both loaders and the manager and have found that there's a bug. If you enable the window resize option on the Custom Window, the inner window does not keep its rendering size.

no-resize yes-resize

In the above images, the settings Custom Window, Render Res at 640x480 and Window Size at 1280x720. The first image does not have Resizable Window toggled, but the second image does. The inner window's resolution in the second image is bumped to 720 (and scales when resizing the window) when the intended behavior should be keeping the inner window locked to 640x480.

ItsEasyActually commented 1 week ago

And we are good to go as of fa818ec