Yes, I was precisely thinking the same thing in the bus one hour ago. is_in_map_limits sounds so much better than ist_in_kartengrenzen that didn't noticed it until later. the name I chosed was a loo literal translation Iguess, it's what google translator suggested. In fact I was quite sure in the bus too a Norwegian guy whould quote me and point me precisely this in the forum.

I'll change to is_within_map_limits and is_within_grid_limits. before someone starts using the new names.

Given a x,y tiles long map, the map_limits is a x*y sized array, coords go from ( 0 .. x-1 ,0 .. y-1 ) each position represents a TILE, has a planquadrat_t iirc.

grid_limits sizes grid_hgts, and each point represents the height of a CORNER (shared by a maximum of 4 tiles), a vertex, so it has one more position in each dimension.

(0..x,0..y)

For example, a 8x8 map contains 8x8 TILES, and is represented by a 9x9 grid of heights.

If you translate is, I propose rather is_valid_koord() (or even is valid pos, as the posiition is the be queried) and is_valid_pos_grid(). The latter, because the grid is not used so often, and the connection is more clear this way, imho.

mmm... I'll get my hands into this then. Don't worry all will be documented.

I also liked within_limits because a coord outside limits it's not invalid per-se, depending of the meaning of the coord. If we allowed to use the inspect tool outside the map, whould be valid and outside limits.

With the water_level project with the raise/lower tools I have to use fictional tile coords, for example a tile (64,64) position, in a 64x64 map, that's right now not a valid tile coordinate, to be able to raise the SE corner of the map (right now that tool only raises NW corners of "valid" tiles ). You can also consider that coordinate is a grid coord, or a outside limits tile coordinate, but that doesn't make it invalid. Depends on the point of view.

is_within_ is also ok. Just is_in_map_limit() is an ugly translation. Get_groesse_x stems from pre-koord era. Thus returning a koord there is fine with me. It is also no used in any critical prce, I think.

The get_groesse_x() and get_groesse_y() are best replaced by a single get_size() which returns a koord.

But again, I am not to keen to translate too much. I would not translate local variables. This could just introduce new errors for exactly no much gain. Same with mark and unmark, I think one routine did at least used this together with markiere. (May be wrong, before renaming check it.)

get_grundwasser -> get_waterlevel() (which will require a lot of search an replace in Kierons patch I think)

simwerkz.cc: In member function ‘virtual const char* wkz_raise_t::work(karte_t*, spieler_t*, koord3d)’:simwerkz.cc:904:11: erreur: ‘class karte_t’ has no member named ‘is_within_map_limits’simwerkz.cc: In member function ‘virtual const char* wkz_lower_t::work(karte_t*, spieler_t*, koord3d)’:simwerkz.cc:970:11: erreur: ‘class karte_t’ has no member named ‘is_within_map_limits’simwerkz.cc: In static member function ‘static const char* wkz_setslope_t::wkz_set_slope_work(karte_t*, spieler_t*, koord3d, int)’:simwerkz.cc:1056:15: erreur: ‘class karte_t’ has no member named ‘is_within_map_limits’simwerkz.cc:1056:73: erreur: ‘class karte_t’ has no member named ‘is_within_map_limits’simwerkz.cc: In member function ‘virtual const char* wkz_marker_t::work(karte_t*, spieler_t*, koord3d)’:simwerkz.cc:1306:11: erreur: ‘class karte_t’ has no member named ‘is_within_map_limits’simwerkz.cc: In member function ‘const char* wkz_station_t::wkz_station_dock_aux(karte_t*, spieler_t*, koord3d, const haus_besch_t*)’:simwerkz.cc:3146:14: erreur: ‘class karte_t’ has no member named ‘is_within_map_limits’simwerkz.cc: In member function ‘const char* wkz_depot_t::wkz_depot_aux(karte_t*, spieler_t*, koord3d, const haus_besch_t*, waytype_t, sint64)’:simwerkz.cc:4199:11: erreur: ‘class karte_t’ has no member named ‘is_within_map_limits’simwerkz.cc: In member function ‘virtual const char* wkz_headquarter_t::work(karte_t*, spieler_t*, koord3d)’:simwerkz.cc:4795:11: erreur: ‘class karte_t’ has no member named ‘is_within_map_limits’make: *** [build/default/simwerkz.o] Erreur 1

karte_t::get_size_grid() -> has to be renamed to get_grid_size, it sounds weird like this. get_size() is too generic maybe, and there are classes that have that method name already, better to choose different names I think.

karte_t::get_size_grid() -> has to be renamed to get_grid_size, it sounds weird like this. get_size() is too generic maybe, and there are classes that have that method name already, better to choose different names I think.

BTW I noticed scenarios code is failing now, to check if it was my changes I downloaded nightly 6298 and 6296, they still fail.sim-wingdi_2013-01-17_v112.1_r6284.zip <- Failssim-wingdi_2013-01-16_v112.1_r6279.zip <- Works

None of the changes between 6279 and 6284 should have broken this part of the code. Is the file script/scenario_base.nut broken/missing/not up-to-date?

get_size should replace get_groesse_*(). But why the first lines of your patch repalce get_groesse by get_grid_size? That is certainly not what is intended. Because when you toate the map, stuff will be one position off.

Same for gameinfo, baum ... at many positions get_groesse is replaced by get_grid_size instead of get_size. Somehow this does not look like a clean translation patch when I see this.

On a 64x64 map, the first one will return (63,63), and the second one will return (64x64) This comes from the fact that some parts of the code care for the grid, that's a 65x65 (from 0 to 64) and other iterate over planquadrat_t items, that are stored on a 64x64 (from 0 to 63) array. Those 2 kind of algorithms just used get_groesse_? functions, and many of them looked like:

This functions had to do that minus 1 operation, that was (I know, performance difference will be near zero) sightly slower. And I assume they just executed once because the function is const, but if any operation inside the loop was not const, it whould have triggered the operation each iteration.

Now you can see easily if the loops are iterating over the grid or the map easily, just looking wich of those 2 functions it's using. So it comes with a performance gain and a readibility improvement in my oppinion.

I could have created get_size alone and leave all those -1 around the code, but this feels more clean to me.

Anyway if you want me to revert that and just use get_size() I can do it.

I've translated almost all function names. there is still some german in simworld.h where I coudn't find a translation clear enough, (remember I don't know german, I gust used google translator and tried to guess reading code).

I'm quite sure I didn't break anything, I just restrained myself to simworld.h, and just modified other files when there was no choice.

Pending of your comments, and from prissi to confirm me what to do on get_size(), won't commit yet.

Simutrans is in most parts not written for speed but for clarity, at least this is the intention. Thus if you operate on the map, you should use get_size() and if you operate on the grid get_grid_size().

I know you had good intentions, but this built in -1 will confuse more (as it already did to me) than a german function name. Thus get_size must return 64,64, as the size is 64 and get_grid_size() 65,65.

Also a for loop over the map will run from i=0 to i<64 if i[64]; The -1 is only for special situations for instance when an object is to be placed, and could run as well only to get_size()-get_size_of_object().

I admid that the -1 should have been documented in such cases. Actually, since a map can be rotated, such searches should run from 1 to get_size()-1 correctly.

-- spieler[]: maybe add a note: 1 = public player, 0 = standard human player-- current season: 0..3 (0 .. spring ??)-- msg: holds all the text messages in the messagebox (chat, new vehicle etc)-- average speed (per way type) do determine speedbonus iirc-- tile_counter: iirc this is used to distribute the workload when changing seasons to several steps-- get_size: maybe better return 'koord const &' instead of 'koord' to avoid unnecessary copying ? - it seems not to be used in any time critical routine.

It would be great next time, when you change anway thing contain "wrong" simutrans formatting "if (bla==1 && b) {" to change this also to "if( bla == 1 && b ) {" and so on. THis would get use closer to consistent formatted code too.