Missing icons from the toolbar menu

[CarrotCroissant7]

Hi.

I've been playing Simutrans 124.5 version recently and have encountered a problem when loading one of my saved games.

When I loaded my game some of the icons were missing from the toolbar menu such as railway, tram, airport and monorail.

Also in road tools I couldn't even place bus stops.

The icons are missing when I play as human player or public service.

I think it's possible that my saved game forgot that I started from year 0 because I was playing in year 9 at the time.

In year 0 you have all the icons such as roads, railway, monorail, airport and shipping tools all from any year and in the future.

Also in that year you can even build depots and use vehicles from any year as well.

Anyway this is also a problem I encountered in the previous Simutrans 124.4 version as well.

Except I was able to find a way to fix it.

That I'll try to explain in another message.

I've attached a screenshot showing the problem I've encountered and I've drawn an arrow that points to the toolbar.

I should also mention that I've been playing Pak128 and I've added files from Pak128 German, Pak128 Britain, Pak128 Japan and Pak128 Czech. All added to Pak128. I didn't add all the files from the paks. I only added the most needed such as roads, railways, tramlines, monorails, airports, shipping, urban buildings, curiosity buildings, factories, trees, vehicles, bus stops, railway stations, monorail stations, tram stations, docks and airports. Maybe even many more.

Anyway I need to know if there is any way of fixing the save game problem?

[prissi]

Actually, Simutrans should not allow to start that early. The earliest date should be set by the earliest townhsll. Allowing year zero is the actual bug.

Unless, timeline is off when all things should be available.

[makie]

Mixing paks lead to diverse problems of all kinds.
Please don't do it.

As Prissi said:
For pak128.german it is not possible to start bevor 1800.
Pak128.britain may be start year 1750
For all other paks start year 1930 is recommended.
Only pak128.german and pak128.britain are designt and tested for start year bevor 1930.

If you start bevor then toolbar menu are not display because there is nothing to build.

Actually, Simutrans should not allow to start that early. The earliest date should be set by the earliest townhsll. Allowing year zero is the actual bug.

One of the buildings in the mixed lot will have a starting year of 0. That's where this comes from.

In pak128.german we have this problem too. Until we changed all start years consequent to at least 1800.

[Isaac Eiland-Hall]

Nobody has addressed the possibility of saving the save, so in case it might help, what I would try:

1. Of course make a copy in case this doesn't work, so at least you're not worse off
2. Go into public player and skip ahead years until you get so something reasonable and see if things return

[makie]

2. Go into public player and skip ahead years until you get so something reasonable and see if things return

I think that will help, but it means about 1,700 clicks on the "Step time-line by a year" icon.

[Isaac Eiland-Hall]

Rookie numbers for anyone who has ever played Cookie Clicker or the like, though

[victor_18993]

prissi, you wrote that the actual bug here is that year zero is allowed at all, and that the earliest
date should come from the earliest town hall unless the timeline is off. I had a look and that is
what CarrotCroissant7 ran into: an empty toolbar is what a game started before anything exists looks
like. Patch attached against r12130, for review.

Why nothing stops it today

The new world dialog does compute this bound already - it walks the town halls and the roads - but it
only limits what the number field shows. gui_numberinput_t::set_value() clamps its own value and
never writes it back into the settings, and draw() does not sync it either. karte_t::init() then
takes the starting year straight from the settings and never checks it.

So the bound lives in one dialog and every other way in walks past it: starting_year from
simuconf.tab (clamped 0..0x7FFF), the in-game settings dialog (0..2999, and that one does write
back), and -startyear on the command line (1..2999). The dialog can show a corrected year while the
game starts somewhere else entirely.

The patch

hausbauer_t gets a get_earliest_townhall_year(): the smallest introduction date among the town halls,
rounded up to the next year with the same (date+11)/12 the dialog already uses - a game starts in
January, so a town hall introduced in July is of no use until the year after.

karte_t::init() raises the starting year to that when the timeline is on. That is the one place all
three ways of creating a world go through - the dialog, the command line and the heightfield loader -
and the corrected value goes into the world's own settings, so everything downstream that reads the
starting year agrees with the clock. env_t::default_settings is left alone, so the player's own
configured preference is not rewritten behind their back.

Timeline off is untouched, whatever year is asked for, which is the exception you named.

The numbers come from the pakset, and they match what makie said

makie wrote that pak128.german needs 1800 or later. The rule derives exactly that from the
descriptors, without knowing anything about that pakset:

Code Select Expand

pakset          town halls  earliest year
pak64          -            1400
pak128          11          1720
pak128.german  28          1800

With the timeline on, asking for year 0 gives 1400, 1720 and 1800 respectively; with it off, year 0
stays year 0 everywhere.

What it does not touch

• Only new games. Loading a savegame does not go through karte_t::init(), so no saved game has its

date changed and the format is the same.

• No protocol change, no pakset change, no script API change - scripts cannot create a world at all.
• A pakset with no town hall keeps its current behaviour: there is nothing to derive a bound from,

so the helper returns zero and nothing is raised.

• The dialog is not touched, so its own bound stays as it is.

One thing I would rather you decide

The bound comes from the descriptors, and a town hall with no intro_year in its .dat gets
DEFAULT_INTRO_YEAR, which is 1900 - a version 1 building descriptor gets the same when it is read.
For a pakset like that, timeline games could no longer start before 1900. That is the rule working as
you described it, but it is a visible change for old paksets, so I did not want to assume it.

Testing

Full suite 237/237 before and after, the [n/m] sequence identical line by line, and the same three
warnings in both builds.

There is no automated regression test, and I want to be straight about why: the tests run as a
scenario inside a world that is loaded, and a script cannot create a new game - the script API only
accepts general tools and the new map tool is a simple one. Nothing in the suite ever reaches
karte_t::init(), so a test added there would pass before and after the fix and prove nothing. I
verified it with a temporary harness calling the same init() the new map tool calls, over the six
relevant cases and the three paksets above. The harness was removed before the final build and the
final test run.

Follow-up candidates

Three things I ran into and left alone on purpose:

• karte_t::init() ignores get_starting_month(). It sets last_month to zero always. The new map
  tool applies the month afterwards with step_month(), but the command line and the heightfield loader
  do not, so the same setting means different things depending on how the world was made. Looks like a
  bug, low to medium priority. Out of this patch because it is about the month, not the bound, and
  fixing it means deciding which of the three paths is right.
  
• A pakset with no town hall gives an inverted range in the dialog. game_start stays at 4999
  while game_ends becomes 0, so the year field gets limits(4999, 0). Bug, low priority - it needs a
  pakset with no town hall at all to show. Out of this patch because touching it changes dialog
  behaviour, and this patch changes none.
  
• The bound is now computed in two places. The dialog has its own loop, the engine has the
  helper. Technical debt, medium priority, and the one I would do next - but only after the point above
  is settled, because that 4999 is what makes the empty case behave the way it does today.
  

[prissi]

A pakset without townhalls will crash simutrans when generating a town. That is not allowed. But you forgot the patch file to attach. However, I think you found the right answer anyway, go submit. I think the last month for start not in January needs correction indeed.

And 1900 as lowest starting year is ok for townhalls with no dates (which must be ancient paks).

[makie]

And 1900 as lowest starting year is ok for townhalls with no dates (which must be ancient paks

Any object that is missing a intro_year= will be assigned to year 1900; this is correct and avoids errors.

[victor_18993]

Thanks, Prissi and Makie.
That settles the two points I wanted to confirm.
If a pakset without town halls is not a valid case, then there is no need for this patch to define special behaviour for it. And if 1900 is the intended fallback for town halls without an explicit

Code Select Expand

intro_year, then the current implementation already has the desired behaviour for old paksets as well.
I also agree that the starting-month issue should be corrected, but I will keep that separate from this patch so the two behaviours can be reviewed independently.
And yes, I managed to forget the actual patch in the previous post.
Attached now is the r12130 diff described above.

[victor_18993]

The configured starting month was not actually ignored - I had that wrong when I first mentioned it,
and measuring it corrected me. Here is what is really going on, and a two line patch against r12132.

The date was already right; the map was not

karte_t::init() starts the world in January, and tool_work_world_t moves the clock forward afterwards
with step_month(), one new_month() per month. So the date you end up with is correct today.

What is not correct is that the whole map is generated before that clock ever moves. Cities, town
halls, attractions and city buildings are all picked through get_timeline_year_month(), so a game
configured for December 1900 was generated with the object availability of January 1900 and only then
jumped forward eleven months.

Measured on pak64:

Code Select Expand

          final date        timeline used while generating
before    1900 / month 11  22800  (January 1900)
after    1900 / month 11  22811  (December 1900)

The patch

init() now takes the month from the settings before anything is generated, and the step_month() call
afterwards is removed, so the starting date is decided in one place and no month is applied twice.

Removing those month steps is safe, and the reason is already in the code: a game starting in January
calls step_month(0) and runs no new_month() at all, so January has always been the proof that a new
world does not need them to be properly initialised. init() calls reset_timer() itself, which was the
other thing step_month() did.

This does change generated maps, and I would rather say so plainly

For a start outside January the generated map can differ, because it is now built with the right
month instead of January's. That is the point of the patch - objects introduced between January and
the starting month can now appear - but it is a real observable change, not a no-op. The final date
is identical; the generation is what gets corrected.

r12132 still holds

The month cannot push a date back before the earliest town hall: r12132 rounds that bound up to a
whole year, so the floor is January of that year and adding 0 to 11 months only moves the date later.
Checked as well as reasoned - pak64 and pak128.german, a year below the bound, exactly at it and well
above, months 0, 5 and 11, timeline on and off, 36 combinations, none of them landing before the
first town hall. r12132 does not need to become month granular.

Testing

Full suite 237/237 before and after, the [n/m] sequence identical line by line, and the same warnings
in both builds. Savegames are untouched - karte_t::load() does not go through init() - and there is
no protocol, pakset or script change. Timeline off behaves exactly as before.

As with the previous patch there is no automated regression test: the suite runs inside a loaded
world and a script cannot create a new game, so nothing in it reaches karte_t::init(). The numbers
above come from a temporary harness calling the same init() the new map tool calls, removed before
the final build and test run.

The welcome map still steps five months by hand to start in June. It has its own settings object and
is not a player's new game, so I left it as it is.

Diff attached - two lines. I am not asking for it to go in, I would rather hear whether you agree
with correcting the generation month rather than only the clock.

[prissi]

The patch looks ok, please submit.

Although, seeing this: We have never checked for too large datas though, if one want to make it really foolproof ...

[victor_18993]

Committed as r12133. Thanks for the review.

[victor_18993]

I looked into your point about start dates that are too large, and there is a real wraparound
case, but it is not where I expected, so I would rather ask before writing anything.

current_month is sint32 and has plenty of room. The narrow spot is the accessor:

  uint16 get_timeline_year_month() const { return use_timeline ? current_month : 0; }

The first wrap is year 5461, month 4: 5461*12+3 = 65535 still works, 5461*12+4 = 65536 becomes 0
as a uint16. Since is_available(0) treats everything as available, the timeline can quietly behave
as if it were switched off. Measured on r12133, same in pak64 and pak128.german. simuconf.tab
accepts a starting_year up to 32767, so that case is reachable, while -startyear stops at 2999 and
the new world dialog only limits what it shows.

Year 2999 is a different thing and I do not want to mix them up: there is no wrap there, it just
happens to be DEFAULT_RETIRE_YEAR and the current -startyear limit, so objects with default dates
are gone from that point on.

So before I prepare a patch: should the upper starting-year limit be the gameplay-oriented limit
of 2999, or only the technical timeline limit around 5461? A third possibility would be to keep
2999 for the UI and the command line and enforce 5461 only as the engine safety limit.

I have not picked a number myself because that seems like your call rather than a technical one.

[prissi]

I think it should be the same year as the townhall retires.