[patch] toggle terrain on minimap

Forum for technical discussions regarding development. If you have a general suggestion, problem or comment, please use one of the other forums.

Moderator: OpenTTD Developers

User avatar
3D_Grabber
Engineer
Engineer
Posts: 6
Joined: 06 Jun 2006 21:58
Location: Zürich, Switzerland

[patch] toggle terrain on minimap

Post by 3D_Grabber »

hi all,

one thing that annoyed me always, was that it wasn't possible to see
the industries and the terrain at the same time on the minimap.

finally I set out to fix the issue once and for all (hopefully) :D

the patch changes the terrain ("contours") button behaviour from
radiobutton to checkbox. you can turn it on/off (toggle) independently from the
other selections.
this makes it possible to see the terrain togheter with the industries, vehicles, routes, owner etc.
minimap with industries and terrain
minimap with industries and terrain
toggle_terrain.PNG (11.22 KiB) Viewed 9983 times
i tried to keep my modifications "minimally invasive".
only a few dozend lines in one single source file are changed (smallmap_gui.cpp).

this is my 1st patch.

what do you think?
cheers
Attachments
minimap_toggle_contours.patch
(9.97 KiB) Downloaded 282 times
User avatar
Zuu
OpenTTD Developer
OpenTTD Developer
Posts: 4553
Joined: 09 Jun 2003 18:21
Location: /home/sweden

Re: [patch] toggle terrain on minimap

Post by Zuu »

Nice work,

Regarding the coding style:
* You have some extra tabs at on two lines I can find. (Highlight tabs and you will find them on line 268 and 271 in your patch file)
* You add an extra white line at line 218 and 296.
* Line 272 can be discussed if it should be added or not. But the code block above (line 266-270) should be indented one more step as you can see other case blocks are.

Otherwise I have not came to a conclusion if your way to implement the feature is the best possible or if there exist other ways. But it seams quite clean still. It seams a bit strange that the big table with land legend colors gets removed but guess there is a reason why it is so.
My OpenTTD contributions (AIs, Game Scripts, patches, OpenTTD Auto Updater, and some sprites)
Junctioneer (a traffic intersection simulator)
User avatar
3D_Grabber
Engineer
Engineer
Posts: 6
Joined: 06 Jun 2006 21:58
Location: Zürich, Switzerland

Re: [patch] toggle terrain on minimap

Post by 3D_Grabber »

thx,
Regarding the coding style...
>> I'll fix it
big table with land legend colors gets removed
I am aware of this.
It's a trade-off. the other possibility would be to display the
height-level legend together with whatever other legend that is selected.
this would probably clutter the ui a lot.
since the height level legend just adds some virtual scale without real
playing value (IMO), I decided not to display it.
User avatar
Dimme
Transport Coordinator
Transport Coordinator
Posts: 277
Joined: 30 Jul 2008 12:42
Location: Trondheim, Norway

Re: [patch] toggle terrain on minimap

Post by Dimme »

Great! This has annoyed me too :) The height-level legend is not necessary, imo.
Try my modular airports minigame!

Image
User avatar
FooBar
Tycoon
Tycoon
Posts: 6553
Joined: 21 May 2007 11:47
Location: The Netherlands
Contact:

Re: [patch] toggle terrain on minimap

Post by FooBar »

Nice! I'd love to see this feature in trunk some time.

About the land height legend:
Maybe you can include a check to only display that legend if none of the other options are selected. So basically if you have the land height level together with some other option enabled, the legend of that other option takes precedence. If you only have the land height level option enabled, it displays the land level legend.
User avatar
Bilbo
Tycoon
Tycoon
Posts: 1710
Joined: 06 Jun 2007 21:07
Location: Czech Republic

Re: [patch] toggle terrain on minimap

Post by Bilbo »

The patch no longer apply to current trunk.

Also, the original "landscape" mode had different colors for roads (black) and rails (grey). The railroads+roads mode (button right next to the town button) in the original minimap had this feature too, however in patched version when I activate this mode, both roads and railroads are black on the minimap. I found the cause (GetSmallMapRoutesPixels used _smallmap_vehicles_andor instead of _smallmap_contours_andor like it was in original minimap) and fixed that too.

I am attaching updated and fixed patch.
Attachments
minimap_toggle_contours-16069.patch
Fixed version of patch, updated to 16069
(9.92 KiB) Downloaded 234 times
If you need something, do it yourself or it will be never done.

My patches: Extra large maps (1048576 high, 1048576 wide) (FS#1059), Vehicle + Town + Industry console commands (FS#1060), few minor patches (FS#2820, FS#1521, FS#2837, FS#2843), AI debugging facility

Other: Very large ships NewGRF, Bilbo's multiplayer patch pack v5 (for OpenTTD 0.7.3)
chrissicom
Route Supervisor
Route Supervisor
Posts: 415
Joined: 07 Oct 2004 10:05

Re: [patch] toggle terrain on minimap

Post by chrissicom »

It would be nice if I could still select just landscape with this patch :) With the patch I can only view something else or something else and landscape but not landscape alone.
User avatar
Ammler
President
President
Posts: 953
Joined: 18 Jun 2006 18:18
Location: Switzerland
Contact:

Re: [patch] toggle terrain on minimap

Post by Ammler »

chrissicom wrote:It would be nice if I could still select just landscape with this patch :) With the patch I can only view something else or something else and landscape but not landscape alone.
Maybe like the "lock" feature on the transparent gui? (I like the ctrl-magic. ;-)

Greets
Ammler
User avatar
Bilbo
Tycoon
Tycoon
Posts: 1710
Joined: 06 Jun 2007 21:07
Location: Czech Republic

Re: [patch] toggle terrain on minimap

Post by Bilbo »

I personally don't like the CTRL-magic, that way half of the functions are "hidden" and you have to remember what exactly the CTRL-click on the widget does, unless you want to experient and see parhaps later if it is not something destructive...
If you need something, do it yourself or it will be never done.

My patches: Extra large maps (1048576 high, 1048576 wide) (FS#1059), Vehicle + Town + Industry console commands (FS#1060), few minor patches (FS#2820, FS#1521, FS#2837, FS#2843), AI debugging facility

Other: Very large ships NewGRF, Bilbo's multiplayer patch pack v5 (for OpenTTD 0.7.3)
Eddi
Tycoon
Tycoon
Posts: 8258
Joined: 17 Jan 2007 00:14

Re: [patch] toggle terrain on minimap

Post by Eddi »

Bilbo wrote:I personally don't like the CTRL-magic, that way half of the functions are "hidden" and you have to remember what exactly the CTRL-click on the widget does, unless you want to experient and see parhaps later if it is not something destructive...
or you could just right click, and read the description of what CTRL does on this button.
User avatar
Bilbo
Tycoon
Tycoon
Posts: 1710
Joined: 06 Jun 2007 21:07
Location: Czech Republic

Re: [patch] toggle terrain on minimap

Post by Bilbo »

While that is true on some occasions, there are still places that lack these descriptions. Like the list of stations, right click won't tell you that ctrl-click will oven a new viewport with that station....
If you need something, do it yourself or it will be never done.

My patches: Extra large maps (1048576 high, 1048576 wide) (FS#1059), Vehicle + Town + Industry console commands (FS#1060), few minor patches (FS#2820, FS#1521, FS#2837, FS#2843), AI debugging facility

Other: Very large ships NewGRF, Bilbo's multiplayer patch pack v5 (for OpenTTD 0.7.3)
Eddi
Tycoon
Tycoon
Posts: 8258
Joined: 17 Jan 2007 00:14

Re: [patch] toggle terrain on minimap

Post by Eddi »

so that is a problem of the documentation, not of ctrl
User avatar
Ammler
President
President
Posts: 953
Joined: 18 Jun 2006 18:18
Location: Switzerland
Contact:

Re: [patch] toggle terrain on minimap

Post by Ammler »

Still better to use ctrl than to change old behaviour which some would miss...

This feature with ctrl is less "hidden" then no inclusion.

Greets
Ammler
2007Alain2007
Chief Executive
Chief Executive
Posts: 658
Joined: 11 Nov 2007 12:06
Contact:

Re: [patch] toggle terrain on minimap

Post by 2007Alain2007 »

Could we use this not only for the terrain but for all of them becuse some times you want to see trains and Industry and other things would that be hard to do
For Community Integrated Version http://code.google.com/p/civopenttd/
Roujin
Tycoon
Tycoon
Posts: 1884
Joined: 08 Apr 2007 04:07

Re: [patch] toggle terrain on minimap

Post by Roujin »

2007Alain2007 wrote:Could we use this not only for the terrain but for all of them becuse some times you want to see trains and Industry and other things would that be hard to do
The problem with this idea is that some of the modes are not compatible with others. For example you cannot display the owner of some tracks and the type of tracks (road? rail?) at the same time.
* @Belugas wonders what is worst... a mom or a wife...
<Lakie> Well, they do the same thing but the code is different.

______________
My patches
check my wiki page (sticky button) for a complete list

ImageImage
ImageImageImageImageImageImageImage
2007Alain2007
Chief Executive
Chief Executive
Posts: 658
Joined: 11 Nov 2007 12:06
Contact:

Re: [patch] toggle terrain on minimap

Post by 2007Alain2007 »

ok i see were your coming from there cheers any ways
For Community Integrated Version http://code.google.com/p/civopenttd/
Terkhen
OpenTTD Developer
OpenTTD Developer
Posts: 1034
Joined: 11 Sep 2008 07:32
Location: Spain

Re: [patch] toggle terrain on minimap

Post by Terkhen »

Bilbo's corrected version of the patch updated to current trunk.
Attachments
minimap_toggle_contours-16507.diff
(12.15 KiB) Downloaded 233 times
User avatar
adf88
Chief Executive
Chief Executive
Posts: 644
Joined: 14 Jan 2008 15:51
Location: PL

Re: [patch] toggle terrain on minimap

Post by adf88 »

I rewrote the patch from the beginning. Now it works properly - turned off industries are not shown on the map.
Original "Contours" view can be used by unclicking selected view button. As in previous patch "land" button works independently from view buttons and is only a switch between land height/green background.
Also there is no height legend. I think it's useless. However I can change the patch to show this legend. What's your opinion ? Where show it ? At every view if "land" is pressed ?
//EDIT: Legend is present now, at every view but only if the height is shown.

//EDIT2: Updated to 0.7.2 and r17288

//EDIT3: Fixed coding rules, updated to r17654
Attachments
minimap_toggle_contours_2.1-ottd_0.7.2.diff
Minimap Toggle Contours 2.1 for OTTD 0.7.2
(14.84 KiB) Downloaded 186 times
minimap_toggle_contours_2.1-ottd_r17654.diff
Minimap Toggle Contours 2.1 for OTTD r17654
(11.38 KiB) Downloaded 209 times
Last edited by adf88 on 27 Sep 2009 14:51, edited 14 times in total.
:] don't worry, be happy and checkout my patches
Terkhen
OpenTTD Developer
OpenTTD Developer
Posts: 1034
Joined: 11 Sep 2008 07:32
Location: Spain

Re: [patch] toggle terrain on minimap

Post by Terkhen »

I prefer the new behaviour for the contour button you implemented, adf88. I have two comments about it, though. The first is a problem that you share with the first implementation of the patch: I don't think that you should change the names of anything in the trunk code (for example the change from SM_WIDGET_CONTOUR to SM_WIDGET_TOGGLELANDHEIGHT). The second thing is that you should not remove the height legend, because then this patch makes two changes instead of a single one, and patches should be as small as posible.
User avatar
adf88
Chief Executive
Chief Executive
Posts: 644
Joined: 14 Jan 2008 15:51
Location: PL

Re: [patch] toggle terrain on minimap

Post by adf88 »

Yes, you right. I made suggested changes. Height legend is now only at "Contour" view, maybe add it to every view ?
:] don't worry, be happy and checkout my patches
Post Reply

Return to “OpenTTD Development”

Who is online

Users browsing this forum: No registered users and 9 guests