Loobinex / keeperfx-unofficial

KeeperFX - Unofficial release
60 stars 7 forks source link

Update engine_camera.c - remove unnecessary lines #172

Closed eddebaby closed 4 years ago

eddebaby commented 4 years ago

These adjustments that are suggested to be removed seem to have been put in place before the menu could scale, to adjust for higher resolutions - either way they seem to be wrong. See comment below for images and more details...

The side menu is scaled now, removing these adjustments give much better results at higher resolutions (slightly more zoomed out - in line with 640x400).

There are no more graphical glitches than when using 640x400 mode. May still be slightly wrong for "weird" menu widths (e.g. 1280x720, 1280x1024, or any "high res" that is not 16:10). Will check add new solution if needed as suspected.

Loobinex commented 4 years ago

Great, thank you. I'll look into it.

Loobinex commented 4 years ago

Could you explain what the difference would be between the current code and your code, if I set the zoom level to your suggestion?

Because how I understand it now is:

I have a feeling I'm misunderstanding something. Also, feel free to contact me on the discord.

eddebaby commented 4 years ago

I'll look at the discord tomorrow.

I don't really suggest you implement second bullet point, it was in-case the pull request is seen to degrade the experience, if one was enjoying hiding the menu to get a better view in 1080p, one might not like the glitchy bottom row(s) suddenly coming back.

Personally I always use the menu, so I prefer the further zoom (at higher resolutions) with this pull request.

Either way, with the Loobinex:master code:

If the second bullet point is implemented, then even 640x400 mode will be glitch free in the bottom row (when hiding the menu). This also means that the view will be more zoomed in (at 640x400 and all other resolutions), and will not fully imitate the original.

With the pull request: All resolutions will be handled equally, and will have the correct zoom distance in comparison to 640x400 mode (i.e. resolutions of the same aspect ratio, but different number of pixels will be able to see exactly the same number of tiles). This is not the case in the base code.

[Optional] With the change to CAMERA_ZOOM_MIN: Workaround legacy bug - restrict maximum zoom-out of camera, so that the bottom rows of map view (isometric mode) are not glitched in fullscreen mode (when the menu is hidden) [not enough resources to render that many tiles].

The following tables shows off some approximate values I measured from screenshots, of the current master build (with the menu in view).

X Y Horizontal Tiles Vertical Tiles Total View Size
320 200 10.6 11.4 120.84
640 400 10.6 11.4 120.84
640 480 9.5 12.4 117.8
1280 600 11.8 9.2 108.56
1280 720 10.5 10.2 107.1
1024 768 8.7 11.5 100.05
1280 800 9.6 10.8 103.68
1440 900 9.7 10.5 101.85
1600 900 10.3 9.8 100.94
1280 960 9.5 12.4 117.8
1280 1024 8.5 11.9 101.15
1680 1050 10.5 9.5 99.75
1920 1080 10.2 10 102

Try lots of aspect ratios and resolution sizes in game to see the variability of the current build (and try again with patch). I can provide more screenshots and tables too.

Sorry for the long post, I wanted to cover all bases, and I do hope I am making some sense! Obviously, would love to be able to just render more stuff (larger array), and then we could increase the zoom out range. (keeperfx-unofficial-zoomContd's commits helped me find this). But this is the limits of what is available now.

Loobinex commented 4 years ago

I've been made aware that in 4k resolutions, the zoom in is extreme. With your pull request I already see on 1920x1080 that I can indeed zoom out a bit more. So I'm happy with your suggestion.

Now, without the menu I indeed see the glitching bottom, which I do not have in the master code, so I was leaning towards setting CAMERA_ZOOM_MIN to 4400. But if I understand your analysis correctly, that means that even people playing with the menu on, will have their game zoomed in further than in the current master, and in the original game. This would not be a proper trade-off.

This would make me lean towards merging this commit as is. But, do you think it would be possible to only limit zoom when the menu is off? That sounds like the best of both worlds: 1) Consistent zoom level and being able to zoom out further for high resolutions 2) No glitches for people who play with the menu off.

eddebaby commented 4 years ago

That would be ideal, and something I was going to write earlier. It should hopefully be possible, I'll investigate.

If you set CAMERA_ZOOM_MIN to 4400, most (all) resolutions will be about as zoomed in as 1080p is in the current master. I have not thoroughly tested this zoom level with all aspect ratios though.