jsettlers / settlers-remake

A Remake of "The Settlers III" for Windows, Linux, Mac and Android
http://www.settlers-android-clone.com
MIT License
355 stars 101 forks source link

Fix minimap screen size #706

Closed Simulant87 closed 6 years ago

Simulant87 commented 6 years ago

The displayed screen size on the mini map was too big. So I fixed it to represent the actual part of the currently displayed map.

This is the mini map on the current master branch (too big) onmaster

This is the mini map on my branch. onbranch

I had to work with an offset = 50; of the calculation of size for the Y-coordinate. I think there is an underlying issue maybe in the MapCoordinateConverter#getMapForScreen on the calculation for the Y-coordinate that I just work around for the mini map calculation and that could be fixed in general. This would als help to calculate the correct center of the screen of the current view port. I also addapted this part but the incomming Y still seems to be wrong. The point is still a bit to high. So I am open for discussions and improvements.

andreas-eberle commented 6 years ago

@michaelzangl: Do you need any more changes? If not, please change your review to accepted.

michaelzangl commented 6 years ago

No, thanks, that's it ;-)

andreas-eberle commented 6 years ago

I found a small bug in the way the map viewed map area is drawn on the minimap (see image, marked in red). This happened after I zoomed out and then back in. The white line on the left side did not shrink as expected.

grafik

@Simulant87 @michaelzangl: Do you have an idea where this comes from / can fix this?

Simulant87 commented 6 years ago

@andreas-eberle at first I could reproduce this bug. But after switching to master (could not reproduce there) switching bach to the branch and doing a clean build of jsettlers.graphics I could no longer reproduce it. Can you reproduce it after a clean build?

andreas-eberle commented 6 years ago

I was able to reproduce it the following way:

grafik

grafik

grafik

Simulant87 commented 6 years ago

ahh. I can only reproduce it in full screen mode. Then I also don*t have to zoom out and in before. I will have a look.

andreas-eberle commented 6 years ago

Thanks!

Simulant87 commented 6 years ago

@andreas-eberle fixed, extracted, and simplified the calculation.

andreas-eberle commented 6 years ago

@Simulant87: Thanks! @michaelzangl: Can you have a look at the new changes? It seems to work fine.

andreas-eberle commented 6 years ago

@Simulant87: Thanks for this fix.