Enhanced viewport: zoom out, overlays & tooltips (r53_27127)

[MJP]

And now I know where train stations are
(other station types are not affected by the black mask)

[MJP]

r29 brings the route steps feature, I hope you'll like it
Each sign shows the order index and the type of destination (ST for station, WP for waypoint, IM for implicit and DE for depot).

For the time being, the refit window is not handled ; I don't know yet if it's really a problem.

While this feature is already somewhat useful, my dream is to draw the real path of the focused vehicle, not just the steps. I have no idea if this is feasible.

[Michi_cc]

r29 brings the route steps feature, I hope you'll like it

I'd strongly advise you to never mix two basically unrelated features in the same patch, unless you already know that this will never, ever go into trunk.

-- Michael Lutz

[MJP]

r29 brings the route steps feature, I hope you'll like it

I'd strongly advise you to never mix two basically unrelated features in the same patch, unless you already know that this will never, ever go into trunk.

Thanks for the advice.

In the zip archive I provide in the first post, I agree there is an all-in-one diff. But all main features are also provided as a patch sequence composed of 8 independent parts (well, almost: partX+1 won't be happy without partX previously applied).

And speaking of the new feature of r29, I find it quite related with the zoom out one: it helps to see quickly the route of a vehicle, it's an extension that fits nicely to the purpose of a map IMHO

[Michi_cc]

And speaking of the new feature of r29, I find it quite related with the zoom out one: it helps to see quickly the route of a vehicle, it's an extension that fits nicely to the purpose of a map IMHO

Personally I think this is something that makes sense regardless of zoom level. If I'd want to see the route of an inner-city bus line, the stations might be much too close to still see in minimap-style view, so it should also work in the normal view.

-- Michael Lutz

[Eddi]

the CargoDist-style overlay may be interesting in that aspect. the efforts should probably be combined there.

[Creat]

the CargoDist-style overlay may be interesting in that aspect. the efforts should probably be combined there.

Yes, I was about to mention this as well. Cargodist is already capable of overlaying quite a bit of stuff on the main view. Even though it is mostly related to cargo flow (usage statistics and the like), I'm sure that overlay capability can be useful to you as well. It also supports a delayed drawing mode, where the display isn't necessarily kept up to date all the time, but only actually drawn if the view area stays constant for a while. Since he keeps his changes neatly separated in branches you can likely just take a look at the corresponding branch (viewport-overlay I'd guess).

If you decide to check it out just to see what it can do, you can enable it in the menu of the map-button (link graph legend). Then both "All" buttons (you need to have an existing network for anything meaningful to show up though).

I also tried to merge your patch with CargoDist, but since it also modifies the main view as well as the minimap quite a bit it became a bit crashy... Could you possibly take a look and maybe provide a patch for Cargodist as well? Feel free to decline (especially if you don't play with CargoDist yourself), since I know this would most likely not be totally trivial and a probably be a significant amount of work...
I would really love to play with this, but just can't play anymore without CargoDist

[Leanden]

Could you possibly take a look and maybe provide a patch for Cargodist as well?

Don't take this the wrong way, but I dont think there is much incentive to rewrite the patch to support another patch which isn't in trunk, unless someone (and by someone i mean Chill) plans to include this into a patchpack which also include Cargodist, then I can't see somebody attempting to maintain 2 patches at once.

You may say, well why not just maintain a patch to work with Cargodist, but then you'd be effectively cutting your legs off by preventing your patch ever hitting trunk by making it dependant on another patch.

[MJP]

Personally I think this is something that makes sense regardless of zoom level. If I'd want to see the route of an inner-city bus line, the stations might be much too close to still see in minimap-style view, so it should also work in the normal view.

Indeed. I quickly tested the route steps feature works fine with all zoom levels (with only 2 lines modified). I also confirm this feature is totally independent of all the others.

For now, I keep my patches as a pack (hg MQ): it's easier for me to maintain. But if something in particular is interesting enough to go into trunk, I guess someone will ask for it and I'll do a clean standalone patch at that moment.

Eddi & Creat> I took a quick look at the cargodist patch but the only overlay I saw was on the smallmap, not on viewport. I'll check again but I don't see yet what could be useful (since I have a system already well integrated into the game).

Could you possibly take a look and maybe provide a patch for Cargodist as well?

The combination might be interesting. I'll see what can be done, I mean what I can do to simplify the merge (as Leanden said, having another patch between ottd and my own patches is not very attractive from my point of view).

[Creat]

Eddi & Creat> I took a quick look at the cargodist patch but the only overlay I saw was on the smallmap, not on viewport. I'll check again but I don't see yet what could be useful (since I have a system already well integrated into the game).

It is definitely there. What version did you use? I'm not sure if the CargoDist in Chills patch pack has that for example, as I remember someone saying it's a bit old (I don't know if that was in regards to the trunk version it's based on or the used cargodist version). A recent (official) build from here should work in any case though.

In case you missed how to get to the overlay in my last post, here it is again: Click and hold the map symbol to get the menu, select "link graph legend". In the window that pops up click both "all"-buttons once. For this to display anything you need to have some vehicles running. Just setup a bus route though some city with a couple of stops and have a bus or two run around, that's enough. It's the same information (afaik) as the overlay in the minimap. And yes, it isn't exactly intuitive, if I wasn't told where this could be found I doubt I'd ever have even stumbled upon it.

In the end I hope we will be able to get completely rid of the smallmap. Being able to display statistical information in the main view is certainly helpful for that.
Just as a side note: you can probably use

Could you possibly take a look and maybe provide a patch for Cargodist as well?

The combination might be interesting. I'll see what can be done, I mean what I can do to simplify the merge (as Leanden said, having another patch between ottd and my own patches is not very attractive from my point of view).

Yea, making the patches easier to merge (not conflict as much) would be great. I don't mind merging them when needed. Or possibly even maintaining that merge for others as well.

I also tried to figure out what the problem is on my own, but it crashes (or fails assertions) at seemingly unrelated positions, for example in spritecache.cpp (assert @ line 425) or 8bpp_optimized.cpp (access violation @ line 90 with "m == 1"). The second one happens pretty much all the time (first one I only saw once a while ago). I'm not deep enough into the code and the inner workings of OpenTTD to figure out what causes problems like that, it also doesn't seem like the blitters were modified (much) by either patch (with the exception of adding the SetLine function). I only know that it happens upon changing the zoom level from/to smallmap mode (meaning x16 to x8 or vice versa) and it doesn't happen always.
Edit: I just had two crashes when scrolling the main view in map mode, not right at the moment of changing zoom level to/from map mode as previously...
Edit2: It always seems to be during the drawing of town names. Just in case you're interested enough to take a look I've attached the crash files as a zip.

[MJP]

In case you missed how to get to the overlay in my last post

Thanks for the tip Now the overlay is enabled, I see what we are talking about.
I'll look deeper into cargodist as soon as I'll have more time.

Your crash is related to DrawString() which needs a special dpi. From what I imagine you may have obtained by merging cargodist and zoom64, I wrote what may be (or not) a fix to your problem.
So you just have to replace the 2 two functions I joined to this post in viewport.cpp (you may have to adapt a little, I don't even know if it compiles properly). Good luck!

[MJP]

r30 introduces basic path rendering:

route_stopover_path.png (92.09 KiB) Viewed 5877 times

There is still some work to do on the handling of _focused_window.

[Creat]

First of all thank you for the corrected functions and sorry it took me so long to get back to you (had an important exam). It now seems to work nicely without crashing (yay). I did pull out the last couple of lines from the "if ... else" statement in ViewportDoDraw, as those were identical in both sections.

The only other thing I've noticed and am unsure about is in MainWindow::OnTick (main_gui.cpp). The following is the result of just slapping the contents of that function from cargodist and your patch together:

Code: Select all

virtual void OnTick()
{
	if (--refresh == 0) {
		this->viewport->overlay->RebuildCache();
		this->GetWidget<NWidgetBase>(WID_M_VIEWPORT)->SetDirty(this);
		this->refresh = LINKGRAPH_REFRESH_PERIOD;
	}
	this->viewport->refresh = 1;
	this->viewport->map_type = (ViewportMapType) _settings_client.gui.default_viewport_map_mode;

	if (_settings_client.gui.viewport_map_in_realtime) return;
	if (this->viewport->zoom < ZOOM_LVL_DRAW_MAP) return;
	if (--this->viewport->refresh != 0) return;
	this->viewport->refresh = FORCE_REFRESH_PERIOD;
	this->SetDirty();
}

The first half is as it is defined by CargoDist, the second half by your patch (as you return early after checks your part had to come in the second half obviously).

Since I don't know how much this->viewport->refresh and this->refresh actually have to do with each other, I don't know how much they interact, but it still seems terribly inefficient as the viewport appears to be refreshed on every tick (?). Maybe I should ask fonso about this, as it's his code (basically) requesting a refresh of the main viewport?

[MJP]

It now seems to work nicely without crashing (yay).

yay too

Code: Select all

	this->viewport->refresh = 1;
	this->viewport->map_type = (ViewportMapType) _settings_client.gui.default_viewport_map_mode;

These 2 lines should be in the constructor. Very strange (and wrong) to find them in ::OnTick().
Setting refresh to 1 here results in a full redraw at each tick. This is very cpu intensive (hence a bad idea). And resetting the map type to the default value before drawing it... Well, I see no good reason to do that

Since I don't know how much this->viewport->refresh and this->refresh actually have to do with each other

They have not much in common, both trigger a full redraw of the viewport, that's all. The one from cargodist prevents excessive processing (usually the role of a cache). The other one attached to the viewport is only useful when realtime rendering is disabled (you can find a similar thing in the smallmap_gui code, which has no realtime support).

[Creat]

Yea I actually have to retract that part. I apparently wasn't really paying enough attention when merging. Clearly those two lines belong in the constructor and the resulting OnTick() seems much more efficient (and should work just fine). In my defense it was the first time I used p4merge, and it screwed up the order of the two respective changes at that point. I just didn't see it for some reason. So clearly it's not fonso writing such nonsense but p4merge (or me, by extension).
Just to clear that up again

[MinchinWeb]

This is a very neat idea and I'd like to give it a try. Has anyone created a Windows binary that I can download to get started quickly? If one doesn't exist, is there somewhere I could request such a binary? Thanks!

[MJP]

This is a very neat idea and I'd like to give it a try.

Has anyone created a Windows binary that I can download to get started quickly? If one doesn't exist, is there somewhere I could request such a binary? Thanks!

The following binaries should work with a nightly release.

EDIT: unproperly packaged files removed

[Lord Aro]

you are not distributing binaries correctly - you must bundle all the files of the in the same archive, or you are in breach of GPL

(i don't know about MSVS, but in mingw/unix the command you want is 'make bundle')

[Creat]

Method for Windows is described here. Basically "make -f Makefile.msvc bundle PLATFORM=WIN32" or "PLATFORM=x64".

[scififan]

Bugreport:

If a vehicle has an invalid order, the game crashes with "Assertion failed at line 2309 of ..\src\viewport.cpp: to_tile != INVALID_TILE"

Reproduction:
Open attached savegame
Click "Fast forward the game"
Game crashes because station "St. Michael North" is removed after a few seconds, giving Train 1 an invalid order.

Tested on Win7 x64 with build r23862 and all-in-1.diff applied