The International Simutrans Forum

 

Author Topic: Minimap improvements  (Read 634 times)

0 Members and 1 Guest are viewing this topic.

Offline Freahk

  • Devotee
  • *
  • Posts: 1473
  • Languages: DE, EN
Minimap improvements
« on: April 30, 2021, 09:39:12 AM »
There's a thread in the extended section of the forums but I guess most of these minimap improvements are actually not specific to standard but might also do a good jon in standard.

In the past, Ranran already implemented some improvements to the minimap.
Those changes I remember are:
 - Merging the Mail/Passenger layer into to coverage layer. The usual line filters can be used to filter the display by Pax/Mail/Freight including specific freight types.
- The "show contour" checkbox. This will hide height informations on the minimap, which sometimes is quite useful to get a clearer look on the map.
I guess there had been a few more changes, I might have a detailled look later.

If the standard team agrees that those changes might be a good hing in standard, I will backport this from extended.

Further, In preparation to some changes of minimaps "network" overlay, I have reorganised the drawing code of those ordinal-cardinal lines.
It's just a code cleanup in preperation to further work, but I guess it's worth to backport that cleanup.
This can be found in here:
https://github.com/irgend42/simutrans-extended/commit/b77b8022edc09fcd03b3e2a6a84c31dbd8fb240f

Further changes currently WIP:
« Last Edit: April 30, 2021, 02:26:37 PM by Freahk »

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 10573
  • Languages: De,EN,JP
Re: Minimap improvements
« Reply #1 on: April 30, 2021, 12:16:16 PM »
The contour thing was in standard and I think Ranran took it to experimental.

The changes in the constructor minimap.h in the diff I personally do not like. This is much harder to follow (and also involves more conditionals) than the old code.

The rest of cleanup is not clear to me (apart from deleting the obsolete constructor), because I cannot compile it after patching:
Code: [Select]
Error C2664 'void swap(planquadrat_t &,planquadrat_t &)': cannot convert argument 1 from 'scr_coord' to 'planquadrat_t &' Simutrans GDI C:\msys64\home\Markus\simutrans\trunk\gui\minimap.cc 495
same in line 496.

Offline Freahk

  • Devotee
  • *
  • Posts: 1473
  • Languages: DE, EN
Re: Minimap improvements
« Reply #2 on: April 30, 2021, 12:29:34 PM »
This is an extended commit!
You cannot simply apply the patch to standard.
It's just there to show what has changed in this commit.
To apply these changes to standard I'll first have to backport this properly.

About the constructor, usually initialisation should be prefered over (re-)assignment.
If you don't like the conditions in the initialisation list, then this might be moved to the constructors body, so these can be swapped when the condition is not met.

About the swap: It does not refer to swap(planquadrat_t &,planquadrat_t &), but to void swap( T& a, T& b ) from the C++ standard. https://en.cppreference.com/w/cpp/algorithm/swap
Anyways, It's not explicitly importated but "simply there" due to imports from other files. I'll clean this up.
Which C++ standard does simutrans comply to? swap seems to be a C++ 11 feature.
Anyways, this will not prevent a backport to standard. Manually swapping the values is not a big thing.
« Last Edit: April 30, 2021, 12:53:30 PM by Freahk »

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 10573
  • Languages: De,EN,JP
Re: Minimap improvements
« Reply #3 on: April 30, 2021, 12:52:56 PM »
The compiler should be clever enough that both give the same code. Maybe the single it is even better one, since there is only on if evaluation instead of four.

I must confess that putting everything into the constructor is harder to follow for me, which is the most important thing on not performance critical code for me ...

Anyway, if this is not a workable patch, I cannot really figure out what you want to achieve from this, so I cannot comment at all.

Offline Freahk

  • Devotee
  • *
  • Posts: 1473
  • Languages: DE, EN
Re: Minimap improvements
« Reply #4 on: April 30, 2021, 12:55:49 PM »
It IS a workable commit in extended.
The question here is: Is standard interessted in a backport of this commit to standard or not?
It seems quite pointless to backport this just to throw the backport away.
« Last Edit: April 30, 2021, 01:43:45 PM by Freahk »

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4863
  • Languages: EN, DE, AT
Re: Minimap improvements
« Reply #5 on: April 30, 2021, 03:16:40 PM »
Seems like an unproblematic change. Why not implement that big constructor in the cc file?
I would like to see this
Code: [Select]
if((bool)start_offset!=(bool)end_offset) {
..
}
changed to
Code: [Select]
if( (start_offset==0) != (end_offset==0)) {
..
}
Also please add space after start of comment //. :)

Offline Freahk

  • Devotee
  • *
  • Posts: 1473
  • Languages: DE, EN
Re: Minimap improvements
« Reply #6 on: April 30, 2021, 03:33:40 PM »
Seems like an unproblematic change. Why not implement that big constructor in the cc file?
I have no idea.
Sombody moved it to the .h file so I expected there was a reason for this.

That change makes sense, changed in the extended branch.

I'll go on and prepare this for standard now.

What about mentioned change of Passenger/Mail layer into the "stop coverage" layer?

Offline Freahk

  • Devotee
  • *
  • Posts: 1473
  • Languages: DE, EN

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 10573
  • Languages: De,EN,JP
Re: Minimap improvements
« Reply #8 on: May 01, 2021, 12:41:43 PM »
The code is not identical, it is much more than a leanup. Your code generates more wiggles. See the red marks.

Offline Freahk

  • Devotee
  • *
  • Posts: 1473
  • Languages: DE, EN
Re: Minimap improvements
« Reply #9 on: May 01, 2021, 08:01:37 PM »
Indeed I screwed the "reduce overlap" fix up.
Fixed now.

Call it a code reoganisation if you prefer this term.
I actually didn't cherry pick all of the cleanup things as it was rejected in the first place.
« Last Edit: May 02, 2021, 10:44:03 AM by Freahk »

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 10573
  • Languages: De,EN,JP
Re: Minimap improvements
« Reply #10 on: May 02, 2021, 10:25:57 AM »
Sorry, I did not want to offence you., especially since you mainly work for experimental. Just back then I spent some effort to reduce bends;so the change was obvious.

However, everything looks like before, so in it goes in r 9750, thank you.

Offline Freahk

  • Devotee
  • *
  • Posts: 1473
  • Languages: DE, EN
Re: Minimap improvements
« Reply #11 on: May 02, 2021, 10:37:55 AM »
I am totally fine with this. Sorry, if it didn't seem like that.
Pointing out issues is part of a proper review and the code was actually wrong in the diagonal fixup part.

Offline ampersand

  • Devotee
  • *
  • Posts: 151
  • Languages: FR, PL
Re: Minimap improvements
« Reply #12 on: May 02, 2021, 10:55:31 AM »
so in it goes in r 9750, thank you.
Does it mean it will be introduced to the standard Simutrans?

Offline Freahk

  • Devotee
  • *
  • Posts: 1473
  • Languages: DE, EN
Re: Minimap improvements
« Reply #13 on: May 02, 2021, 01:17:26 PM »
Yes, I think so.

However, so far it's just a small internal reorganisation of how those ordinal-cardinal lines are drawn.
Once implemented in extended, I will list the minimap improvements and backport these if standard considers these worth.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 10573
  • Languages: De,EN,JP
Re: Minimap improvements
« Reply #14 on: May 03, 2021, 10:23:41 AM »
The contour drawing is in as well, since I followed the thread of experimental. (Or rather I did it my way, since at that time it was not finished.)