More height levels (in trunk since r27010)

[ChillCore]

And one more bug bites the dust.

- Max generated height is not respected when using the original land generator.
This seems to happen if you have selected mountainous or alpinist while switching from terragenesis.
Fixed:
When selecting the original terraingenerator and mountainous or alpinist was selected, revert to hilly.
When original terraingenerator is selected and player selects mountainous or alpinist, revert selection to hilly.
(I would rather have the two options greyed out but I haven't figured out where to do that yet ...)

- Done:
Make may way through the TILE_SIZE occurances in viewport.cpp
so I can declare TILE_SIZE as uint in tile_type.h as it should without the game crashing.

Changed:
-More codingstyle.
-Fixed a few incorect comments.

Logfile attached:
-Updated known bugs.
-Updated todo.
-Extended tuning yourself section.

[Kogut]

Here is an updated build with the updated small map works:

Is it possible to get new build?

[ChillCore]

Here's a small update to current trunk.

Most notable changes:
- Solved a few conflicts. (Bumping to trunk and adapting to changes from r20183 to r20192)
- #include "slope_func.h" in dummy_land.cpp and renamed function for drawing tiles outside the map. (@see r20187)
- Added two more todos in terraform_cmd.cpp (Mark functions for removal when the patch is finished.)
- Minor coding style changes.

All in all not very much changed but trunk has changed a lot lately which prevents the previous patch to be applied to current trunk.

[Kogut]

Thanks! Now, with my new ability (openttd compiling) I finally will play game with that patch!

[Moriarty]

It's good to see you're still working on this Chill (thanks, even though I've never used the patch - lack of compiled for-win version). I don't suppose there's any remote possiblity of it being included in trunk some day? I think more vertical flexibility is important to the game's future.

[ChillCore]

It's good to see you're still working on this Chill (thanks, even though I've never used the patch - lack of compiled for-win version).

As long as I am having fun while doing so I will continue.
The patch is also included in my patch pack. Which I have made to test this patch to the bone.
Check the second post in my thread for a link to the latest compiled win binary. I have just released v8 so you may want to wait a bit to see if someone feels like posting the new version. v7 is pretty stable too but you may want to read the first posts and the last few pages for known bugs. (Or read the whole topic while you are there?)

I don't suppose there's any remote possiblity of it being included in trunk some day?

Yes there is. Read: No dev has said that they do not want it.
Ofcourse for this ever to be considered all issues have to be resolved first. (See the logfile a few post back: Known bugs and Todo sections.)
Once I/we have done that we need to find a way to break it up in smaller pieces (easier for review) and hope for the best.
I have been thinking to try and propose small changes to trunk to ease the process but I did not get to that yet.
Also I still prefer to not know for the moment.

I think more vertical flexibility is important to the game's future.

I agree although I think OpenTTD will survive just fine without this patch also


Thank you for the moral support.

[Moriarty]

Hey, thanks for pointing me at the patch-pack, I've never really bothered with those things in the past, but this one seems to work passably well.

A "bug" for you relating to your heights from it - when selecting the "snow line", there are only 13 levels to choose from. Shouldn't there be 255? There are for tree-line height.

[Kogut]

13 level bug: Known and reported

[ChillCore]

Shouldn't default snowline height be 255?

No because trees are placed from level 0 up to treeline height (only in the patchpack) as opposed to snow which is placed from level 255 down to snowline height.
If I would put it at 255 there would be no snow at all. Also max snowline height is 253.

Until I have figured out where to correct the limitation you can set the desired values in the scenario editor.

If somebody knows what to change to fix it, feel free to share.
I know (read: I think/guess/assume) it is some lettercode in table/settings.h (see line 208 and following) but that is about it for now.

[Alberth]

Shouldn't default snowline height be 255?

No because trees are placed from level 0 up to treeline height (only in the patchpack) as opposed to snow which is placed from level 255 down to snowline height.
If I would put it at 255 there would be no snow at all. Also max snowline height is 253.

FYI: Having just coded snowlines in NML, I happen to know that the NFO specification states that the snowline lies between 0x10 and 0xef, where values should be multiples of 8 to prevent glitches. In other words, between 2*8 and 29*8 (0xe8) in steps of 8.
Anything else is going to break snowlines of NewGRFs.

[ChillCore]

Thank you for the heads up Alberth.
I did not know that although I had noticed that in the configuration file the snowline height is specified in pixels.

ps:
I have tried to compile a grf using NML some time ago but failed ...
I will be trying that again sometime soon as I noticed many changes have been made lately.
It looks very interesting and much easier to understand then nfo coding.

[ChillCore]

Please find attached a small update to trunk.
A few conflicts with trunk are solved but there are no functional changes for the moment.

I have been looking some more at the missing levelling error message and clearing tiles bug. I am getting closer at solving it but I am not there quite yet. (Just to be clear, the attached patch includes no changes in that area for the moment.)

[ic111]

Hello,

I have just downloaded the patch again. Works quite well for me now, I was able to play for a longer time (although I used only road vehicles and trains).

First of all: I might have time for inspecting some bugs in the future. What are remaining bugs that need a closer look?

Second: The only bug from a player perspective I found in the current version is that on all steep land tiles at height zero with direction north, a mouse pointer is displayed. Sounds odd, have a look at the attached screenshot. If I terraform such a tile, it disappears, if the result of terraforming is such a steep tile, then for the moment it is displayed without that mouse pointer, but if I scroll it again shows up. Also, this display error survives a save/load cycle, so my first idea is that some code running periodically might corrupt the data of such tiles - although for the moment I have no clue what code this might be...

I am pretty sure that this bug wasn't there when I played the patch last time about half a year ago. Can you reproduce it? If yes, do you know since which version it is there? Or has someone already started inspecting it?

[ChillCore]

Hello ic111,

I have just downloaded the patch again. Works quite well for me now, I was able to play for a longer time (although I used only road vehicles and trains).

Cool. I have included the patch in my patchpack and there are few additional bugs that have show up recently. (see GRF_bugs at the end of attached bug txt)

First of all: I might have time for inspecting some bugs in the future. What are remaining bugs that need a closer look?

I have attached a short bug list(it contains also a longer version at the bottom of the file) and the latest version updated to trunk.

Second: The only bug from a player perspective I found in the current version is that on all steep land tiles at height zero with direction north, a mouse pointer is displayed. Sounds odd, have a look at the attached screenshot. If I terraform such a tile, it disappears, if the result of terraforming is such a steep tile, then for the moment it is displayed without that mouse pointer, but if I scroll it again shows up. Also, this display error survives a save/load cycle, so my first idea is that some code running periodically might corrupt the data of such tiles - although for the moment I have no clue what code this might be...

I am pretty sure that this bug wasn't there when I played the patch last time about half a year ago. Can you reproduce it? If yes, do you know since which version it is there? Or has someone already started inspecting it?

I can not reproduce it.
I was going to say update to the latest version of OpenGFX but your screenshot shows the original graphics ...
Can I have a look at your savegame please? Maybe it is caused by some grf that is corrupt?

[ic111]

See attached savegame.

Cool, this means that only a quite low number of bugs remain to be fixed, right? So, in the not too far future one might think about splitting the patch up into smaller parts for applying them to trunk.

[ChillCore]

Thank you for the savegame but I still can not reproduce it.
I will try recompiling the version against r20612 to see if the problem shows up in there.

As you can see in the attached screenshot I am still missing some GRFs although I have replacements for all missing ones.
What version of the ottdc_pack are you using so I can find the correct ones?

Also, on a side note,
Narrow gauge should be after TTRS.
Expensive short and slow bridges is not designed to work with pb_viaduct.grf.
DB Set is disabled. (Don't know why as it is not mentioned.)

[quote=""ic111"]
Cool, this means that only a quite low number of bugs remain to be fixed, right? So, in the not too far future one might think about splitting the patch up into smaller parts for applying them to trunk.
[/quote]
Yep, only a few remain for as far as I know.
I could use some help in smallmap_gui.cpp and terraform_cmd.cpp as I tried to fix them a few times already but I failed on each occasion.
For the code in tgp.cpp (heightmapvariation) I have not yet really tried ...

ps:
The logfile a few posts back is somewhat more complete then the bugs.txt I posted in regards of TODOs but not much. Most can be found by searching for them in the patch.

[ic111]

I think the problem are the narrow gauge rails. When I removed them (and let all other grfs as they are), the problem disappears. However, putting them at the very end, after TTRS as you proposed didn't help.

Hence, I don't think this is a problem of the patch, rather one of that grf. So let's ignore it with respect to the patch.

BTW, what is ottdc_pack?

[planetmaker]

BTW, what is ottdc_pack?

http://wiki.openttdcoop.org/GRF

[ChillCore]

My bad that would be ottdc_grfpack. The package that the coop guys use.

I just find it strange that I do not see the same problem overhere ... it does not matter if I activate the dos or the windows version. (I noticed that your palettes are set to dos ...).
The Narrow gauge version I have is v0.93a. The suggestion to put it after TTRS comes from the GRF itself. Open the grf list ingame, select it and see the NewGRF information part of the gui ...

Anyway it is good to see the problem is solved (partially) and does not come from the patch.


Edit: planetmaker is fast ...

[ic111]

I just had a look on the bug that opening the small map for a high mountain leads to the wrong position. The reason is simply that height up to now is completely ignored in this case. As the changes for fixing this are not big, and my code currently is not ready for making a patch, I document them just here. Note that this only solves the case "Open small map, position it to the correct tile". The other case "Click into small map, position main map correctly", where heightlevel information is also ignored, remains to be solved.

Bugfix:
(1) Add function

Code: Select all

int GetRowAtTile(int viewport_y, Point tile);

to viewport_func.h

(2) Remove the static keyword for this function in viewport.cpp
(3) Add an include for viewport_func.h in smallmap_gui.cpp
(4) Replace function smallMapCenterOnCurrentPos in smallmap_gui.cpp as follows:

Code: Select all

	void SmallMapCenterOnCurrentPos()
	{
		// Goal: Given the viewport coordinates of the middle of the map window, find
		// out which tile is displayed there.

		// First find out which tile would be there if we ignore height
		const ViewPort *vp = FindWindowById(WC_MAIN_WINDOW, 0)->viewport;
		Point pt = InverseRemapCoords(vp->virtual_left + vp->virtual_width  / 2, vp->virtual_top  + vp->virtual_height / 2);
		Point ptWithoutHeight = {pt.x / TILE_SIZE, pt.y / TILE_SIZE};

		// Problem: There are mountains.  So the tile actually displayed at the given position
		// might be the high mountain of 30 tiles south.
		// Unfortunately, there is no closed formula for finding such a tile.
		// We call GetRowAtTile originally implemented for the viewport code, which performs
		// a interval search.  For details, see its documentation.
		int rowWithoutHeight = ptWithoutHeight.x + ptWithoutHeight.y;
		int rowWithHeight = GetRowAtTile(vp->virtual_top + vp->virtual_height / 2, ptWithoutHeight);
		int rowOffset = rowWithHeight - rowWithoutHeight;
		Point ptWithHeight = {ptWithoutHeight.x + rowOffset / 2, ptWithoutHeight.y + rowOffset / 2};

		// And finally scroll to that position.
		int sub;
		const NWidgetBase *wid = this->GetWidget<NWidgetBase>(SM_WIDGET_MAP);
		Point sxy = this->ComputeScroll(ptWithHeight.x, ptWithHeight.y,
										max(0, (int)wid->current_x / 2 - 2), wid->current_y / 2, &sub);
		this->SetNewScroll(sxy.x, sxy.y, sub);
		this->SetDirty();
	}