Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't rely on (ZOOM_LVL_MIN == 0) and allow more zoom levels #3094

Closed
DorpsGek opened this issue Aug 5, 2009 · 8 comments
Closed

don't rely on (ZOOM_LVL_MIN == 0) and allow more zoom levels #3094

DorpsGek opened this issue Aug 5, 2009 · 8 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay

Comments

@DorpsGek
Copy link
Member

DorpsGek commented Aug 5, 2009

fonsinchen opened the ticket and wrote:

The blitters only allow a rather narrow set of zoom levels and it is hard to change that. Those zoom levels are defined in the ZoomLevel enum. In order to allow other windows than the main viewport to use more zoom levels than the blitter can handle ZoomLevel has to be extended and new minimum and maximum zoom levels have to be added to determine which ones the blitter can use.
Unfortunately in several places ZOOM_LVL_MIN is implicitly assumed to be 0. The attached patch changes that and adds ZOOM_LVL_BLITTER_MIN and ZOOM_LVL_BLITTER_MAX. For now ZOOM_LVL_BLITTER_MIN == ZOOM_LVL_MIN and ZOOM_LVL_BLITTER_MAX == ZOOM_LVL_MAX, but this can be (and is, in other patches) changed in order to take advantage of the new possibilities.

Attachments

Reported version: trunk
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/3094
@DorpsGek
Copy link
Member Author

peter1138 wrote:

Why? The ZoomLevel enum is clearly meant for ViewPorts, not for random things that could be zoomed.


This comment was imported from FlySpray: https://bugs.openttd.org/task/3094#comment6499

@DorpsGek
Copy link
Member Author

fonsinchen wrote:

So what solution would you propose for zooming in on the smallmap? I could make my own smallmap zoom enum, but that would be rather silly, wouldn't it? I see that you don't like the idea of zooming in on the smallmap at all, but we have discussed extensively on IRC and as a result of that I have put considerable work into splitting up the original smallmap zooming patch into this one and the ones in #54. I have done that on the premise that you would accept smallmap zoom out and an extension of the zoom levels if it's done properly.

About zooming in on the smallmap: I know it doesn't make much sense for the game as it is now. However, the smallmap and especially its new link statistics view is the central user interface for cargo distribution. I propose you try it and see how hard it would be to visualize the cargo distribution without zooming in. So, if you aren't principally opposed to eventually accepting cargo distribution, you have to admit that zooming in is definitely needed. That doesn't mean you should merge this patch now, but please consider my argument before rejecting it.


This comment was imported from FlySpray: https://bugs.openttd.org/task/3094#comment6501

@DorpsGek
Copy link
Member Author

peter1138 wrote:

Woah there leslie!

"I see that you don't like the idea of zooming in on the smallmap at all"

What brings you to that conclusion?

If I remember from back when I_MYSELF_CONTRIBUTED to #54, there was no need for any enum for smallmap zoom. If there is now, then it should be a separate enum, not fucking around with blitter code for no good reason.

Don't be a jerk. I never opposed smallmap zoom, I oppose this stupid patch.


This comment was imported from FlySpray: https://bugs.openttd.org/task/3094#comment6504

@DorpsGek
Copy link
Member Author

fonsinchen wrote:

As stated in #54 the changes zoomlevels are not needed for zooming out on the smallmap, only for zooming in. This is the exact reason why I created 3 patches, not 2. The smallmap-zoom-out patch can easily be applied without this one. And I have to admit that in our IRC discussion you (petern) didn't actually comment on smallmap-zoom-in (mind that I wasn't talking about smallmap-zoom-out), but other devs did and not in a positive way. That "you" in my previous comment here was meant as plural. I'm sorry if I have offended you.

So, to conclude: With a separate enum for smallmap zooming, would the patches in #54 be accepted? Creating a separate enum is one of the easier solutions and if this time it's not in vain I'll do it.


This comment was imported from FlySpray: https://bugs.openttd.org/task/3094#comment6505

@DorpsGek
Copy link
Member Author

peter1138 wrote:

I fail to see why zoom in and out need to be separate.


This comment was imported from FlySpray: https://bugs.openttd.org/task/3094#comment6506

@DorpsGek
Copy link
Member Author

fonsinchen wrote:

look at our discussion: http://irclogs.qmsk.net/channels/openttd/date/2009-07-22?page=5
For example: 15:19:51 <@smatz> patch using zoom-in for minimap has very low chance of getting includeed in trunk


This comment was imported from FlySpray: https://bugs.openttd.org/task/3094#comment6507

@DorpsGek
Copy link
Member Author

fonsinchen wrote:

I withdraw this report as I have implemented zoom-in differently based on the new zoom-out functionality in trunk.


This comment was imported from FlySpray: https://bugs.openttd.org/task/3094#comment7607

@DorpsGek
Copy link
Member Author

Alberth closed the ticket.

Reason for closing: Out of date

Closed on request of the reporter


This comment was imported from FlySpray: https://bugs.openttd.org/task/3094

@DorpsGek DorpsGek added Core flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay labels Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay
Projects
None yet
Development

No branches or pull requests

1 participant