More height levels (in trunk since r27010)

[ic111]

Just a short comment from me: I probably have no time to dive into this soon. Thus, if someone wants to fix what needs to be fixed regarding this, this certainly would be great.

What I know is that I already times ago introduced the concept of a maximum height level, although I don´t remember exactly how it interacts with heightmaps.

[speeder]

Is anyone still working on this and with heightmap?

Also, will it be ever on the mainline download?

I was toying trying to make a heightmap of the state where I live in Brazil, and in-game it looks horrible... Mostly because the location where I live is mostly flat, but with a 2800 meters tall mountain...

On the game the result was a mostly flat map, with a "rough" path where the mountain is supposed to be, unfortunately the "rough" path is just "rough" not a mountain, so you can still build all over it (instead of having to use tunnels and bridges).

One of the interesting features of the place I live is exactly that place, that requries some cool transportation engineering...

Pic for reference: http://1.bp.blogspot.com/-uhnz_CNjWPg/U ... tes_SP.jpg


In game that is not doable without very high height limits...

[kamnet]

It's no impossible to do, but it will like take a lot of work in tweaking a heightmap to generate the terrain you desire.

It is doubtful that this patch ever makes it to trunk, largely because there is a formation of an alternate plan to include higher levels along with other metadata such as the placement and locations of water and coordinates for cities. This isn't in any sort of usable patch yet, but it's perceive to be the better way to go in the future.

[planetmaker]

It's no impossible to do, but it will like take a lot of work in tweaking a heightmap to generate the terrain you desire.

It is doubtful that this patch ever makes it to trunk, largely because there is a formation of an alternate plan to include higher levels along with other metadata such as the placement and locations of water and coordinates for cities. This isn't in any sort of usable patch yet, but it's perceive to be the better way to go in the future.

You're mixing up a lot of things. As does this patch series.

* there is "just" the possibility of more height levels with the necessary fixes to other places in order to allow them (map, aircraft movement, etc). That's the largest and most complicated part.
* there is the normal terrain generation taking the more height levels into account
* there is, also in this patch series, an additional terrain generator
* you are talking of a new scenario format. That's something completely different and independent of the possible max height levels.

[ic111]

EDIT: Planetmaker was a bit faster than I, I nevertheless post my answer:

Just one remark about this: Most of this patch is not about the mere changes to data structures, but about making things like terraforming, rendering, aircraft movement, etc. ready for more heightlevels.

I.e. regardless of how the changes to the basic datastructures look like, if one wants more heightlevels, then those topics need to be addressed, and IMHO the code that solves those topics in this patch is modular enough to use it in any environment of "we have more heightlevel" (regardless of how they are saved exactly). Of course given some adjustments.

For example: The patch concerning rendering introduces a completely new algorithm for rendering, because the old one had tight assumptions about the maximum number of heightlevel, and would not scale properly for much more heightlevels (details can be found in this thread).

About planetmaker:

As does this patch series

Removing e.g. the additional terrain generator should be one of the simplest things one could do about this patch series.

[planetmaker]

Right, I scrolled back a bit and found v28 of the patch series on the previous page. That looks indeed much smaller and leaner than what I remembered and the heightmap generation is indeed much easier separated.

For easier sorting I did only a small rename in the patch files:

Code: Select all

for i in `ls ??_*.patch`; do mv $i 0$i; done

Funnily the openttd base version the series start (10_...) applies to cleanly does not even compile for me But seems to apply with a bit of fuzz to trunk head.

EDIT: I've some questions (I'll add them here as I go):
* why is the black_tiles grf and the related code needed with more height levels? Why is that not a problem currently in trunk with free-style map borders?

[ic111]

EDIT: I've some questions (I'll add them here as I go):
* why is the black_tiles grf and the related code needed with more height levels? Why is that not a problem currently in trunk with free-style map borders?

Quick answer: In the release is a readme.txt. Specifically, you might have a look at the descriptions for patches 20, 30, 50.
(Disclaimer: I just see that that readme is not up to date with respect to all patches, but the basic facts about how they work should still hold)

Unfortunately, that readme doesn´t exactly answer the question what the old code exactly did with the outside area. Probably, the old approach of repainting an area according to the height of the highest possible mountain at some position in some way made explicitely painting the outside area as landscape of tiles superfluous.

Also, probably, some more description about this is buried somewhere in the 48 pages of this thread...

If what I wrote here is not enough information, please ask again, then I´ll try to find out more, it´s just too long ago that I implemented this to have every detail still in memory...

(however, I will probably be offline for some days starting with tomorrow)

[Eddi]

random idea: why not paint the outside area with normal ground sprites, but a completely black recolour palette?

[ic111]

You mean taking the usual tile images, but altering the palette in a way that results in black images?

Sounds reasonable, if temporarily altering the palette in that way is possible (I never heard of that before with respect to OpenTTD).

[Eddi]

applying a recolour palette is done in loads of places, from company colours to transparent houses.

adding a recolour palette that replaces every entry with black should be easy.

[ic111]

I just started with bringing the more heightlevels patch up to date. A problem I run into:

In my adapted terraforming code, I use a std::map to keep track of adapted heightlevels while the algorithm runs, i.e. in terraform_cmd I have a

Code: Select all

std::map<TileIndex, int> tile_to_new_height, 

where for each tile the algorithm touches, the new height according to the current state of the algorithm is stored. I.e. at any state of the algorithm, the height of a tile is

Code: Select all

IF (tile is in map) <Take height from map>, ELSE, <Take height from usual tiles array>

Line 27 of terraform_cmd:cpp includes:

Code: Select all

#include <map>

Now, I get the following error message:

Code: Select all

In file included from /usr/include/c++/4.8/string:52:0,
                 from /usr/include/c++/4.8/stdexcept:39,
                 from /usr/include/c++/4.8/array:38,
                 from /usr/include/c++/4.8/tuple:39,
                 from /usr/include/c++/4.8/bits/stl_map.h:63,
                 from /usr/include/c++/4.8/map:61,
                 from /home/wol/openttd/trunk_mhl/src/terraform_cmd.cpp:27:
/usr/include/c++/4.8/bits/basic_string.h: In function ‘std::string std::to_string(int)’:
/usr/include/c++/4.8/bits/basic_string.h:2865:45: error: ‘SAFEGUARD_DO_NOT_USE_THIS_METHOD’ is not a member of ‘std’
   { return __gnu_cxx::__to_xstring<string>(&std::vsnprintf, 4 * sizeof(int),

(and several similar error messages with different lines in basic_string.h as well).

To me this looks like that the internal implementation of std::map uses some string functions you recently forbid using safeguards.h. In safeguards.h, I read "use #vseprintf instead".

Question: Is disallowing the use of std::map actually intended, or is this just an unwanted side effect of disallowing the programmers using the #vsnprintf function directly? In the former case, do you know of any map-equivalent data structure (maybe already implemented in OpenTTD) I can use at that place?

Thanks for hints!

EDIT: Ok, if I place the #include <map> before the #include "safeguards.h", it at least compiles and links. But can that be a proper solution?

[ic111]

adding a recolour palette that replaces every entry with black should be easy.

I just dived a bit into this:

It starts with a call to DrawGroundSprite given some palette != PAL_NONE in DrawTile_Void in void_cmd.cpp (that´s the place where I so far painted the black image using PAL_NONE).

DrawGroundSprite calls DrawGroundSpriteAt calls AddTileSpriteToDraw (all viewport.cpp, passes the palette argument through and stores it in the TileSpriteToDraw structure).

ViewportDoDraw then takes the tile_sprites_to_draw array where those structs are stored, and paints them using ViewportDrawTileSprites.

ViewportDrawTileSprites calls DrawSpriteViewport in gfx.cpp, which calls GfxMainBlitterViewport with different modes according to the passed palette. (here, finally the palette argument is evaluated, and transformed into a blitter-internal mode). GfxMainBlitterViewport calls GfxBlitter, does some bookkeeping which (with one exception) is mode-independent, and finally calls

Code: Select all

BlitterFactory::GetCurrentBlitter()->Draw(&bp, mode, zoom).

Here, as far as I understand, we finally are in device / resolution dependent code.

Conclusion: If I get it right, then I would have to define a new constant PAL_ALL_BLACK or similar, which has to translate into a corresponding blitter mode later. As I don´t see a blitter mode that does what I want, I suspect that also a new blitter mode would be needed, or do I miss something?

I just had a look into 8bpp_optimized.cpp, and there some remap array occurs. Would I have to use that one to finally exchange the colors?

So, given the exploration I just performed, is that approach indeed feasable (i.e. better than adding black images), and if yes, what can I do to make it work? Up to now, it´s not at all clear to me how I can get a palette that recolors anything with black.

[ic111]

A new release, v29.

Changes:
- general adaptions to current trunk
- Bugfix: Clicks into the smallmap now again center the map on the corresponding position; this feature was broken probably because I missed some earlier change in trunk.

For now, the way the void tiles outside the map are painted has not changed; if senseful I will implement that in a future version.

[Eddi]

you want a recolour map, which is an array of 256 entries. recolour maps can be supplied via base- or NewGRF. the recolour maps from the baseset are referenced in src/table/sprites.h as PALETTE_* constants, you'd probably want to add your PALETTE_ALL_BLACK there, and then add an action5 type for this palette, to add it to media/extra_grf/something.nfo [some details of how to do this omitted].

format of a recolour table: http://newgrf-specs.tt-wiki.net/wiki/RecolorSprites. you probably want all entries to be 01 (black)

i don't think you need a new blitter mode. i haven't looked at what it does, but i suppose it switches from drawing actual recoloured pixels (like company colours) to recolouring the existing pixels (like transparency). you definitely don't want that.

[ic111]

Ok, I do my very best

(given that I never before edited grfs in any way...)

If I get you right, then media/extra_grf is where I need to add an nfo file. In openttd.nfo, I find various #includes, do I want to add one there?
And are those files actual source files, or are they generated / extracted from somewhere else?

Concerning the contents of the newgrf, http://newgrf-specs.tt-wiki.net/wiki/RecolorSprites reads like I have to add something like this

Code: Select all

0 * 257 00 <256 times "01", i.e. 010101...> 

to some palette.nfo. The sprite number at the beginning of the line, is it file-local, or how can I determine it? On the other hand, http://newgrf-specs.tt-wiki.net/wiki/Action5 reads like I need something like

Code: Select all

0 * <length> 05 0A 01 <offset>

(probably followed by the line above?) where the following things are unclear to me:

How can I determine that length / the offset? Why does the "num-sprites" column on that page contain the value 256 for type 0A?

Also, somewhere needs to be the link between the value of the PAL_XX-constant define in src/tables/sprites.h, and the sprites in the nfo-file. Is it the sprite number at the beginning of the line(s) above? If yes, how can I choose it, the constants in the sprites.h files look quite arbitrary at first glance.

Obviously, some example of an action 5 adding a palette would make live quite a lot easier for me, however I didn´t find one, but read somewhere that they are all bundled in media/baseset itself.

(of course, I found http://www.tt-wiki.net/wiki/NFOTutorial, and tried to find/read what´s important for me; also I already found and downloaded grfcodec).

[Eddi]

If I get you right, then media/extra_grf is where I need to add an nfo file. In openttd.nfo, I find various #includes, do I want to add one there?

yes, or reuse one if it's suitible.

And are those files actual source files, or are they generated / extracted from somewhere else?

those are the source files which create openttd.grf. the makefile will automatically create this if it finds grfcodec in your path, otherwise it uses the prebuilt one.

Concerning the contents of the newgrf, http://newgrf-specs.tt-wiki.net/wiki/RecolorSprites reads like I have to add something like this

Code: Select all

0 * 257 00 <256 times "01", i.e. 010101...> 

add spaces between each 01.

to some palette.nfo. The sprite number at the beginning of the line, is it file-local, or how can I determine it?

the sprite number is filled in automatically, you don't have to worry.

How can I determine that length / the offset? Why does the "num-sprites" column on that page contain the value 256 for type 0A?

this is where it gets complicated. action5 types are defined in src/newgrf.cpp:_action5_types. these define how many sprites each one may get. type 0A, for example is recolour tables for two company colours. there are 16 company colours, so there are 16*16=256 sprites needed.

Also, somewhere needs to be the link between the value of the PAL_XX-constant define in src/tables/sprites.h, and the sprites in the nfo-file. Is it the sprite number at the beginning of the line(s) above? If yes, how can I choose it, the constants in the sprites.h files look quite arbitrary at first glance.

the sprite numbers in src/table/sprites.h are counted over all base grf files, not just openttd.grf. so first all files from trg1r.grf, then from all the other trgXr.grf, and then all sprites added by action 5. so basically you can add your new sprite anywhere in the action5 range, as long as you bump up all following sprites. it's probably best idea to add it somewhere at the end (i.e. before SPR_NEWGRFS_BASE)

oh, and before i forget: NFOrenum will probably complain about your new action5 type which it doesn't know yet. this may prevent automatic creation of the GRF. but grfcodec itself shouldn't mind, so you should be able to compile it manually

[ic111]

those are the source files which create openttd.grf. the makefile will automatically create this if it finds grfcodec in your path, otherwise it uses the prebuilt one.

I put grfcodec on the path, and as even with a configure, in objs/extra_grf/Makefile, nothing else than "GRFCODEC =" showed up (i.e. it didn´t compile openttd.grf at all for me), I manually changed that to "GRFCODEC = grfcodec". Now, it compiles openttd.grf with the following output:

Code: Select all

wol@dbis-lap13:~/openttd/trunk_mhl/objs/extra_grf$ make
[BASESET] Assembling openttd.nfo
make: s: Kommando nicht gefunden
make: [/home/wol/openttd/trunk_mhl/bin/baseset/openttd.grf] Fehler 127 (ignoriert)
[BASESET] Compiling openttd.grf
Warning: Sprite 1480 reports 44 bytes, but I found 52.
Warning: Found sprite 0 looking for sprite 4248.
Warning: Found sprite 0 looking for sprite 4249.
/home/wol/openttd/trunk_mhl/objs/extra_grf/openttd.grf:1: Warning: Found 4249 more sprites than sprite 0 reports.
[BASESET] Updating orig_dos.obg
[BASESET] Updating orig_dos_de.obg
[BASESET] Updating orig_win.obg
wol@dbis-lap13:~/openttd/trunk_mhl/objs/extra_grf$

i.e. it compiles openttd.grf, but first tries to call some funny command named "s", where I have no clue why...

After all, I have a newly compiled openttd.grf, when I start OpenTTD it complains about

Code: Select all

wol@dbis-lap13:~/openttd/trunk_mhl/bin$ ./openttd 
dbg: [grf] Palette sprites are missing
wol@dbis-lap13:~/openttd/trunk_mhl/bin$ 

i.e. seems like I missed some step, or did something wrong. After starting a game, additionally the message

Code: Select all

dbg: [sprite] Tried to load normal sprite #723 as a recolour sprite. Probable cause: NewGRF interference

is printed.

What I did can be seen in the attached diff, maybe you see some more or less obvious mistake... BTW, I did not notice any complaints from NFOrenum.

Thank you for your help!

[michael blunck]

Code: Select all

Warning: Sprite 1480 reports 44 bytes, but I found 52.
Warning: Found sprite 0 looking for sprite 4248.
Warning: Found sprite 0 looking for sprite 4249.

From years of experience with grfcodec, I can tell you that these are not just "warnings", but are hinting to errors which make the resulting newGRF non-functional.

So, there must be a bug somewhere.

regards
Michael

[Alberth]

If you do

Code: Select all

VERBOSE=1 make

you will get also the commands it executes.

Maybe your grfcodec is too old? You can check the config.log to see what it decided with grfcodec.

[frosch]

In the action 5 you specify offset 1. The offset should be 0, though with A5BLOCK_FIXED no offsets are allowed anyway.

So

Code: Select all

+0 * 0 05 18 01 01

should be

Code: Select all

+0 * 0 05 18 01