[NoGo] Neighbours are important

[lukasz1985]

The script is not working: the city is not growing.

Tried using versions 1.2.3 1.3.0 rc2, rc3 of OTTD.

[Zuu]

Did you only use version 6 and 7 of the script or also older versions? (version 6 was released 17 march)


Edit: Can you upload a savegame using stable, testing or nightly OpenTTD which shows your problem?

[lukasz1985]

I forgot - I used version 7 only.
The attachment contains a savegame made with RC3 but the problem appears also in 1.2.3

[Zuu]

Thank you. I'll look into it as soon as I have time. It might be that I've accidentally broken something in the update with version 6.

[stinkyfax]

I am having problems playing this script with friends on a dedicated server.

3 of us, playing for around 2 hours.

I seem to be fulfilling requirements, but it keeps saying town is not growing? What's wrong.
Script is new, just downloaded it using in-game online content manager, OpenTTD 1.2.3.



Here is a save-game: http://josh.c-servers.com/web/debug_save.sav

[Zuu]

Thank you both stinkyfax and lukasz1985 for reporting this quite serious bug in the script version 6 and 7 that stopped it from working. The cause was that I did wrongly set the OpenTTD town growth rate to "off" instead of "normal" which previous versions of this script did. Now it works again and towns should require the mentioned cargo amounts and each month when all requirements are satisfied it will grow.

Update - Version 8

Changes:

• Fix: Towns didn't grow in version 6 and 7 of the script
• Fix: Don't check towns for congestion twice when updating them

[stinkyfax]

Thank you both stinkyfax and lukasz1985 for reporting this quite serious bug in the script version 6 and 7 that stopped it from working. The cause was that I did wrongly set the OpenTTD town growth rate to "off" instead of "normal" which previous versions of this script did. Now it works again and towns should require the mentioned cargo amounts and each month when all requirements are satisfied it will grow.

Update - Version 8

Changes:

• Fix: Towns didn't grow in version 6 and 7 of the script
• Fix: Don't check towns for congestion twice when updating them

Fast Response and fix Thanks

[ikonus]

This is a great script. It really adds extra depth to OTTD. I have been playing with it and FIRS. I noticed that TE_GOODS in the sub arctic climates is petrol. What are your thoughts about adding other consumer goods such as milk, alcohol and goods from industry renewals such as FIRS and ECS?

[aantono]

This is a great script. It really adds extra depth to OTTD. I have been playing with it and FIRS. I noticed that TE_GOODS in the sub arctic climates is petrol. What are your thoughts about adding other consumer goods such as milk, alcohol and goods from industry renewals such as FIRS and ECS?

+1 on that request, was just about to ask this question myself.

[Zuu]

This is a great script. It really adds extra depth to OTTD. I have been playing with it and FIRS. I noticed that TE_GOODS in the sub arctic climates is petrol. What are your thoughts about adding other consumer goods such as milk, alcohol and goods from industry renewals such as FIRS and ECS?

My thoughts on this is that it requires that the script take a more manual control over town growth. Right now it sets cargo requirements and a growth rate which OpenTTD will apply when the cargo requirement is fulfilled. The script do not need to monitor if the cargo requirements are fulfilled, OpenTTD does this all automatically from the script point of view.

The draw back with this method is that the cargo requirements can not be given on CargoID level. Only at TownEffect level. (eg. TE_GOODS)


This script was initially published at a time when there was close to zero GSes at bananas and GS had just merged with trunk. Now, there are other scripts which support finer detail in the cargo requirements out there. My intention is not to spend too much time on duplicating existing script features into this script when there is plenty of other town growth scripts out there that does this. If any of those want to look into the code of this script and copy the neighbourhood factor or the congestion penalty, the code is open and GPL v2.

I'm also open if anyone submits patches to add new functionality to the script as long as they have some basic level of sanity. Previously in the history there was one person who said he was developing an extension but never published any patches.

[aantono]

Would it be possible to add support for stockpiling the delivered cargoes if they don't meet the monthly requirement or if there is more delivered in a month than needed?

[stinkyfax]

This is a great script. It really adds extra depth to OTTD. I have been playing with it and FIRS. I noticed that TE_GOODS in the sub arctic climates is petrol. What are your thoughts about adding other consumer goods such as milk, alcohol and goods from industry renewals such as FIRS and ECS?

I actually could add Alcohol, fruits and other goods into requirements through settings. As long as you can deliver that product to a town-owned territory you can do it.

Would it be possible to add support for stockpiling the delivered cargoes if they don't meet the monthly requirement or if there is more delivered in a month than needed?

If you delivered more than required, it is being stocked.
If you did not meet the requirements, cargo is being consumed anyway without a town growth. I am uncertain why would you let cargo stockpiled if not enough was delivered, as it is unnatural cheating.
Imagine scenario you have a huge megapolis and it requires 1k food monthly, you deliver 700, town is dying in hunger and you would rather stock the 700 food for the next month?

[Zuu]

This is a great script. It really adds extra depth to OTTD. I have been playing with it and FIRS. I noticed that TE_GOODS in the sub arctic climates is petrol. What are your thoughts about adding other consumer goods such as milk, alcohol and goods from industry renewals such as FIRS and ECS?

I actually could add Alcohol, fruits and other goods into requirements through settings. As long as you can deliver that product to a town-owned territory you can do it.

Which Neighbours are important settings did you use?


Neighbours are important uses GSTown.SetGoal(town_id, towneffect_id, goal).

I could be wrong, but according to the API docs, that method do not accept raw cargos. Thus it cannot support specific cargoes as town growth requirements and still using the SetGoal way of controlling town growth.

An alternative way is to not use GSTown.SetGoal, and change the raw growth rate given using GSTown.SetGrowthRate based on in-GS conditions. The GS can then use GSCargoMonitor to monitor delivery and then use GSTown.SetGrowthRate to set it to 0 if goal was not met or desired growth rate if goal was met. The later method requires more work on the GS side which causes: a) more development time, b) more bugs to fix, c) less responsive Game Script.

If you delivered more than required, it is being stocked.
If you did not meet the requirements, cargo is being consumed anyway without a town growth. I am uncertain why would you let cargo stockpiled if not enough was delivered, as it is unnatural cheating.
Imagine scenario you have a huge megapolis and it requires 1k food monthly, you deliver 700, town is dying in hunger and you would rather stock the 700 food for the next month?

Adding stockpiling requires that Neighbours are important is changed to use the more manual town growth control. It is not impossible, just not something that I will probably spend my time to implement. And if anyone wants to attempt to do this, I would guess that code will look very similar to the same code in eg. CityBuilder GS. So why not see if that code can become a GS Library so that the town growth engine can be shared among several GSes and bugs in the actual engine only needs to be fixed once.

[stinkyfax]

Oh I am completely sorry, I confused the topic with CityBuilder topic.

[aantono]

I've taken liberty at trying to implement the over-delivered stockpile logic for cargos, so here is the attached file, hopefully to be incorporated in to the next version release.

The logic is this:
Every cycle the town gets checked for required cargo deliveries,
Those are compared against set goals, and whatever was over-delivered, gets stored.
The stored value gets compared with calculated goals for next cycle
If there is enough cargo to fulfill them, the adjusted goal is 1, otherwise the stockpile gets emptied and the goal adjusted to be the difference

Attached are town.nut and lang\english.txt

Thanks,
Alex

[Zuu]

I want to tell you that I've seen your post and you shouldn't feel neglected. It looks quite good from what I can see by reading your code.

Some reflections:
- If stockpiles can get large, players will probably be disappointed that you don't save them in the savegame and restore upon loading a savegame.
- Remove lines instead of commenting them out. (yes there is a bad example of my own of that in the code already)
- Use integer calculations when you can. Only use floating point when it is really needed. No need to do (A.tofloat()/B.tofloat()*100).tointeger() when you can do just A*100/B if A and B is already integer numbers.
- for this.cargo_stockpile and this.cargo_stockpile_ratios I would use a table and then use .rawget, .rawin and .rawset to allow using the TownEffect enums as keys. This way the code will not break if a new TownEffect is added in the future.
- When you call GoalTown::SetCargoGoal from an instance, use this.SetCargoGoal(..). In Squirrel static/member call is decided at run-time. So I try to be clear with this.Func() or MyClas::Func() for member vs. static call in the code. Only non-class functions are called without a prefix.
- The code lines in SetCargoGoal are very long. Use 80 char columns as a guideline. If it fits within 80, then it is perfect. If it extends to 120 or so that is ok if it is needed for that part of the code. (Yes, I do have 1920x1024 24" monitor, but too long lines of code make it harder to read no matter the size of your screen)
- SetCargoGoal could get some headings that mark the key blocks of code in the method. Now it is just a long list of text unless you read it carefully.
- You add some strings to english.txt that are not used.
- If you clone http://hg.openttdcoop.org/gs-neighbours-are-important, then you can use "hg diff > my.patch" to generate a patch-file that contains all your modifications to the files that you checked out. This file will describe all changes required to town.nut and english.txt. (sorry, the repository was not up to date previously, but now all way to version 8 has been pushed)


Before I add the changes to Neighbours are important these things need to be addressed. Some of these things are possible for me to do, but I'm currently short in time so it will take some time before I have time to do that. So if you want to take a second round based on my review above, that will help.

[aantono]

Thank you so much for such a detailed reply and suggestions. I'm currently looking at saving/loading support (though strangely enough, I've tried saving and reloading the game few times, and the stockpiles remain as they were before. - Any idea as to why that might be happening?)
Will go through the code and clean in up.

As for the long lines in SetCargoGoal, they are all from Log.Info lines where I print out some debugging info. Should I just make it a multi-line entry, separated by '+' string concatenation blocks?

Will do the repo clone, and submit the patch once you are happy with the final result.

Thanks, will be submitting a new draft in a few days.

Alex

[Zuu]

For save/load, I would recommend taking a look on eg. NoCarGoal or even MinimalGS for how I usually implement save/load. Basically, in GoalTown include a member function SaveAsTable() which exports a table to append to the table saved by main.nut. In GoalTown, also add a static function CreateFromTable(table, version) which will return a GoalTown instance initialized using the table. In main.nut the Load() function should just store the table received from openttd in this.savegame_data or similar which is then checked by Init. This is because OpenTTD will kill the GS if it takes too long time in Load(). Thus we postpone reading of the loaded data to when Start() gets called.

Keep in mind that you can save tables and arrays, but they must not contain floating point numbers. Only integers can be saved. Strings may also work, I don't know for sure. But floats are not allowed.


If you want, I can write the save/load part later when the other parts are done.

[aantono]

I've fixed the issues (with the exception of using array for cargo storage).

I do need help with saving and loading. I've implemented the functionality and it seems to work, except that it does not seem to load the numbers that were saved.
I've put debug logs into town.nut:502 and :514 lines to print out how much cargo stockpile was saved and loaded, and for some reason I get different numbers.

I'm attaching the changed files, and if you can take a look that would be great.

[Zuu]

Here is a patch-file based upon the changes in the files you posted. I haven't made any changes. It is just that this uses the patch/diff format rather than posting there full files.