[NoGo] Neighbours are important

[aantono]

I know that it took a bit longer than I thought, but I think I've finally got the functionality working as desired, i.e. to use the GSCargoMonitor.GetTownDeliveryAmount call and properly handle the cycle check duration offset to "pro-rate" the goals. The patch is attached, so would love to hear feedback as well as validation of the math to make sure it behaves at least reasonably close to what's expected.

Thanks,
Alex

[aantono]

Zuu, any word on the new patch? Did you get a chance to play with it to see if its looking good and meets your expectations?

Thanks,
Alex

[Zuu]

Hello,

I have now included your patch in the script. Below follows some feedback.

You have unnecessary casts from float to integer at some places which I have stripped out. The general rule is that AIs/GSes should only use integer math. If you need float math, then I prefer to only there use explicit casts to integers when needed so that it is clear when floats are involved and not.

Also, I have removed the usage of MaxAsInt in your patch. MaxAsInt was included only to shorten a large block of repetitive code so that they will fit within a reasonable code width. In general it is a bad idea to join two things into one method like this. Also, it has in a few places possible lead you to the mistake of calling MaxAsInt instead of only max() or .tointeger() in situations when only one of them is needed.

Code: Select all

a = a + something;

can be shorten as

Code: Select all

a += something;

From what I can see, cargo_delivery_timetable just holds the last time a delivery check was made. The word "time table" however in transportation as well as in OpenTTD usually means that something is scheduled, which doesn't seem to be the case here. Thus I have renamed this variable to something that doesn't give false ideas on what it is used for.


You had a bug that towns got the cargo requirement of zero in the first manage. This is critical in the sense that the first manage will happen before the first call to GSController.Sleep() and thus happen when the world gen progress bar is show, before the game has started. When the GS run in world-gen it can execute DoCommands without loosing the remaining OpCodes of that tick. Thus it can easily set up all goals so they are ready and visible when he world gen has finished.

Another bug I found and fixed was that when towns had 0 as requirement of a cargo effect, the stockpile ratio would always be 0. I fixed this by using 50 as a minimum stockpile limit.

[Zuu]

Update - Version 9

Version 9:

• Add: Stockpiles (thanks to aantono for the patch)
• Add: Display about information in the story book
• Fix: Detection of dessert towns was not working. On 1.3.1-RC1 and newer this works just as default OpenTTD desert detection. For older versions, a fallback mechanism is used to detect desert which will not always give the correct answer for towns at the border between grass and desert, but still works better than nothing.
• Fix: default values for easy/hard was mixed up for the congestion setting

German, French and Russian translation needs updating
I have added an additional {STRING} to these translations so that the English stockpile string is used. However, if you know any of these languages and want to help out you are welcome to update the translation to include all missing strings. (which could be different depending on the age of the translation)

If you want to contribute a translation of a new language, you are welcome to do also that. Just remember to submit the translation using UTF8 encoding. You may wish to play-test your translation before submitting it to see how it turns out in the game and that you didn't broke anything. (I will not validate that you didn't omit a parameter etc.)

[ziond]

Updated Russian translation

russian.txt

(7.75 KiB) Downloaded 129 times

Look like some bug found. Steps to reproduce:
1. Start new game with only one town
2. Check town info window: No neighbours
3. Cheat some money to build two new towns nearby
4. Wait for one month of game time
5. Check first town info window: It is its own neighbour (see screenshot)
6*. Loading save crashes script

[krinn]

ziond: I would say starting a game (except a scenario) with only 1 town isn't possible. So your bug is an effect of a cheat, or a scenario that couldn't of course use a script that use more than 1 town.

Zuu: you should called for language update before publishing to bananas, if anyone wish the v9 with ziond russian update, it would need to wait and download v10.
If any other language gets update after that, again with v11, just because of a translation file update...
I won't get offend if you pm me to query an update.
Or adding a #email comment within language files, so you could query them to get an update.

[Zuu]

Look like some bug found. Steps to reproduce:
1. Start new game with only one town
2. Check town info window: No neighbours
3. Cheat some money to build two new towns nearby
4. Wait for one month of game time
5. Check first town info window: It is its own neighbour (see screenshot)
6*. Loading save crashes script

Thanks for the quick and detailed report. It turned out that neighbour detection was more broken than that. - All towns had the same neighbours as the first town.

Also, thanks for the update of russian.txt.

ziond: I would say starting a game (except a scenario) with only 1 town isn't possible. So your bug is an effect of a cheat, or a scenario that couldn't of course use a script that use more than 1 town.

Discarding a bug before looking at it is dangerous. It could (as in this case) be symptoms on a problem that can occur on different conditions where it is more severe.

Zuu: you should called for language update before publishing to bananas, if anyone wish the v9 with ziond russian update, it would need to wait and download v10.
If any other language gets update after that, again with v11, just because of a translation file update...
I won't get offend if you pm me to query an update.
Or adding a #email comment within language files, so you could query them to get an update.

You don't need to wait for v10 to translate. However, as russian.txt was ready when publishing the bugfix now, I have included russian.txt too. If any other translation would have been ready, that translation would have also been included.

If I would have waited on every translator, that would delay stockpiles even more. Thus I though it wouldn't hurt to publish it now and update it as translations arrive. All it causes is that some users who want to start using stockpiling already will need to read the English stockpile strings which I don't really see as a big problem when the alternative is to not release anything for an additional week or two. By getting it out on bananas, you also easier get feedback if anything broke that you didn't find yourself.

[Zuu]

Update - Version 10

Code: Select all

Version 10:
- Fix: Update town display when the stockpile setting is toggled
- Fix: Don't display stockpile info in town window and debug log when 
stockpiling is disabled
- Fix: Neighbour calculation was broken in version 9. All towns got the same
neighbours.
- Update: Russian translation

[ziond]

ziond: I would say starting a game (except a scenario) with only 1 town isn't possible.

It is possible And it is quite old feature of OpenTTD

1.png (244.88 KiB) Viewed 6126 times

2.png (241.1 KiB) Viewed 6126 times

So your bug is an effect of a cheat, or a scenario that couldn't of course use a script that use more than 1 town.

Any bugreport have right to exist (even this stupidiest one ). And as Zuu discovered this one was only part of more big one.

Anyway guys thanks for your work for this awesome script. I use it in my every single game

-------------

Script still crush (version 10) after loading of savegame
Steps to reproduce:
1. Start new game.
2. Save it.
3. Load it. Script crushes.

save.sav

(30.82 KiB) Downloaded 122 times

[krinn]

You don't need to wait for v10 to translate. However, as russian.txt was ready when publishing the bugfix now, I have included russian.txt too. If any other translation would have been ready, that translation would have also been included.

Maybe checking your pm

It is possible And it is quite old feature of OpenTTD

I never notice it was possible

[planetmaker]

You don't need to wait for v10 to translate. However, as russian.txt was ready when publishing the bugfix now, I have included russian.txt too. If any other translation would have been ready, that translation would have also been included.

Maybe checking your pm
I never notice it was possible

I know it from myself: additions to projects of mine are much better submitted publicly in either in the development topic here at tt-forums or preferentially in their bug tracker at the DevZone. Other forms of contributions easily get without bad intentions lost, forgotten or overseen. Especially the need to check private messages is tedious - they are IMHO not meant to communicate patches to open source project.

[Kogut]

Random rambling from viewpoint of user:

Description is not fitting into default-sized window for selecting GS. In readme default window width is a bit smaller than length of lines. It may be nice to mention whatever it is compatible with FIRS, ECS etc.

Some towns have two neighbours, some three. I wonder about effects.

I like interface in town window (but "0 passengers stored" sounds wrong).

Story book - why disabled must be in angry red?

[Kogut]

And it crashed on loading save.

[Zuu]

Description is not fitting into default-sized window for selecting GS. In readme default window width is a bit smaller than length of lines. It may be nice to mention whatever it is compatible with FIRS, ECS etc.

FIRS and ECS should work as the script uses Town Effects. How well stockpiling works with multiple cargos per town effect is unknown to me.

Some towns have two neighbours, some three. I wonder about effects.

The script picks the 3 closest towns. However there is also a maximum distance allowed. In your case, that is probably why there is a town with only two neighbours.

I like interface in town window (but "0 passengers stored" sounds wrong).

What is your suggestion? Different wording for passengers or not storing passengers at all?

Story book - why disabled must be in angry red?

I though it would be useful to show which features that are disabled in that list. Red is regular a colour used for disabled, so I picked that. To not add more clutter than necessary, there is no "enabled" labels. Please suggest a better colour if you find that red is too strong.

[Zuu]

Update - Version 11

Code: Select all

Version 11:
- Fix: Loading of savegames created by version 9 or 10 caused the script to
crash. Now these files (and files from version 11) loads again.
- Fix: Adopt wording of the per town growth toggle feature in the story book
to stay closer with the wording in the settings window.
- Update: French translation

Thanks ziond and Kogut for reporting the save/load issue that is now fixed.

[lugo]

German Translation attached.



Edit: German Readme attached.

[R-TEAM]

Hi,

suggestions :
Make an parameter to set the min size a town musst have to affected ... very tiny towns should grow alone ...
Make an option to affect towns over snowline or in desert more ....(maybe adjustable ...)
(both suggestions i have from the spring patch pack who have an similar system ....)

Regards
R-TEAM

[Zuu]

Hi,

suggestions :
Make an parameter to set the min size a town musst have to affected ... very tiny towns should grow alone ...
Make an option to affect towns over snowline or in desert more ....(maybe adjustable ...)
(both suggestions i have from the spring patch pack who have an similar system ....)

Regards
R-TEAM

Currently, in desert climate, towns not in deserted areas require more food than normal to compensate for that they cannot require any water to grow (or rather, if they would require water, it would be hard to satisfy as those towns can't have a water tower).

Having further differentiated cargo requirements based on unfriendly climate could be yet another option. Although the script already got quite many options already.

[Bandanner]

Hi Zuu,

I have recently come back to OpenTTD after a a long break and I have been trying out some of the new scrips etc. I am having a few issues (or what seem to be issues) with Neighbours are important.

I have been trying to create a 2048 x 2048 map with and without FIRS, on my first few attempts with autosave every month I was getting the following error on the 1st of Feb (see attached image).


Since this was not working I tried each of the following, all had the same result of the script crashing with the message "This script took too long to save" and each was a new game.

1st game: Auto save after 3 months
2nd game: Auto save after 6 months (Script completed at 12900 ticks)
3rd game: Removed auto save but saved one month after script completed (Script completed 13930 ticks)
4th game: Script completed, waited a year then saved manually.

Any idea whats going wrong, is it something I am doing?

[krinn]

Bandanner
It happen because you tried to save the game while the GS was initializing itself. And a missing check trigger that bug.
So nothing you can do yourself

For your second issue, it will get more complex to fix, as the GS save really too much information because of the number of town the map have.
This time so, you can do something
Use a smaller map and/or lower number of towns inside the map, not perfect but that's the way to avoid the bug.

You have already done the first thing to do : reporting the bug.