News:

Simutrans.com Portal
Our Simutrans site. You can find everything about Simutrans from here.

"Staff shortages" crippling early game freight in 1750

Started by neroden, April 07, 2023, 02:21:53 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

neroden

Screenshot attached.

The factories start out with full employment and then it just...dries up and disappears.  I'm supplying massive amounts of passenger transportation to this village from three nearby villages and locally.  The freight transportation is extensive and essentially total -- but it's being stifled because of the "staff shortages".  The village is growing very, very slowly, and as it grows... it isn't supplying any workers to the factories.

I have a number of theories on what's wrong but I'm not up to date on the code base.

My first suspect is the passenger trip generation system. This is an early-period game with a very large map, I'm not going to be able to connect the entire world up front in the first year, the trips would take too long anyway, and if the only trips to these factories are being generated from the other side of the planet, it'll never work.  If that's the issue, I'll try to rewrite commuter trip generation to be more local.

If they are getting *local* trip generation, then my second suspect is city growth.  The city is loaded with factories with large numbers of jobs but doesn't seem to be generating housing for the workers.  If this is the issue, I'll try to rewrite city growth to handle "factory worker demand", which is a very real thing historically (entire cities would spring up within weeks to fill factory worker needs).

Or it could just be some other bug.  If someone else solves it before I do, that's cool.  If nobody else works on it, I'm mostly putting this bug report here as a reminder to myself for when I'm able to really look into the codebase.

Matthew

Quote from: neroden on April 07, 2023, 02:21:53 PMScreenshot attached.

The factories start out with full employment and then it just...dries up and disappears.  I'm supplying massive amounts of passenger transportation to this village from three nearby villages and locally.  The freight transportation is extensive and essentially total -- but it's being stifled because of the "staff shortages".  The village is growing very, very slowly, and as it grows... it isn't supplying any workers to the factories.

I haven't noticed a worker shortage in recent (real-life) years, but compared to Bridgewater-Brunel, that village has a relatively large amount of factories. On B-B there is generally massive unemployment; this shot from one of the largest cities in the current B-B game (1872) is typical:



You can see from all the red that most people outside the centre are unemployed, despite the presence of bus, rail, train & tram networks (though a lot of these are probably too expensive for Very Low commuters).

QuoteI have a number of theories on what's wrong but I'm not up to date on the code base.

It has indeed changed a lot since your earlier contributions. Are you aware that the city_threshold_size, city_threshold_percentage, growthfactor_villages, etc. parameters in simuconf.tab can be changed to speed up population growth?

QuoteMy first suspect is the passenger trip generation system. This is an early-period game with a very large map, I'm not going to be able to connect the entire world up front in the first year, the trips would take too long anyway, and if the only trips to these factories are being generated from the other side of the planet, it'll never work.  If that's the issue, I'll try to rewrite commuter trip generation to be more local.

If they are getting *local* trip generation, then my second suspect is city growth.  The city is loaded with factories with large numbers of jobs but doesn't seem to be generating housing for the workers.  If this is the issue, I'll try to rewrite city growth to handle "factory worker demand", which is a very real thing historically (entire cities would spring up within weeks to fill factory worker needs).

Three thoughts on this. I am a coding newbie, but can maybe fill in some context:
  • About 5 years ago, James put a huge amount of effort into making the passenger generation code multi-threaded. So you'd need to think about the thread implications of any changes to the algorithm
  • James is planning to rework the town growth algorithm after his current major project (modifiable consists) is completed. At current rates of progress, that will probably be the late 2020s. But here has already been some preliminary discussion, of which this post is most relevant to this problem, since it shows that James does want to make growth more responsive to factory location.
  • There's a reliable large-scale study showing 45 minutes for commuters on Beijing public transport, and self-report studies show 27 minutes for the UK (24 minutes by car but higher for public transport), 39 minutes for London (probably reflecting much higher use of public transport and broadly consistent with the Beijing figures), and 25 minutes for the USA (again ~45 minutes for urban public transport). I also heard a radio programme that said that the 45 minutes time had also been replicated in Africa and with historical data. So if your algorithm fiddles with journey time tolerances, it would be nice to reproduce that.

QuoteOr it could just be some other bug.  If someone else solves it before I do, that's cool.  If nobody else works on it, I'm mostly putting this bug report here as a reminder to myself for when I'm able to really look into the codebase.

It would be great if you were able to return to active coding again. I can see that you made some really critical contributions in your previous involvement and we can always use more.
(Signature being tested) If you enjoy playing Simutrans, then you might also enjoy watching Japan Railway Journal
Available in English and simplified Chinese
如果您喜欢玩Simutrans的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

neroden

My biggest suspicion is that something has gone wrong with commuter trip generation. 

I didn't put it in the screenshot, but the town hall and the fountain *also* have no employees and 100% vacancies, and there are no industrial buildings in the town -- nobody in this town is going to work at all!  (Well, OK, five people.)  And they're all within walking distance of several hundred jobs!

I know there are supposed to be separate trip generations for 'visitor' trips (which I'm not going to mess with) and 'commuter' trips, and something has gone badly wrong with commuter trips, because none are being generated locally.

This may actually be the *same* cause as the unemployment in other games, if it's a failure to generate commuter trips.

Another possibility: I know the trip updates only update part of the map at a time.  I don't think this is it because I was able to get passenger travel generated.  And because the factories all started out with workers, who then disappeared, despite full freight supply.

If I actually get back into coding, I'm unfortunately not going to work on this first.  I have a couple of projects which were suspended in 2013.  One is to completely rewrite rotation so that map rotation is merely a client-side "viewport" display question and has nothing to do with the game state, which will enable map rotation in network games, as well as cleaning out massive amounts of crud in the code.  I'll probably do that first.  That's why this is more of a reminder than anything else; it won't be the first thing I work on if I get back to coding.

However, since the game is unplayable for me until there's a workaround -- for now, I think the staff_shortage "feature" should simply be disabled, until it's working properly.  Is there a simuconf.tab setting to turn it off so that the factories will work?

Matthew

Quote from: neroden on April 09, 2023, 09:41:27 AMIf I actually get back into coding, I'm unfortunately not going to work on this first.  I have a couple of projects which were suspended in 2013.  One is to completely rewrite rotation so that map rotation is merely a client-side "viewport" display question and has nothing to do with the game state, which will enable map rotation in network games, as well as cleaning out massive amounts of crud in the code.  I'll probably do that first.  That's why this is more of a reminder than anything else; it won't be the first thing I work on if I get back to coding.

That seems like an enormous project. As you said it would be very useful in network games and would also prevent a whole class of bugs that commonly catch out even experienced coders. Good luck, because I think you will need it!

QuoteHowever, since the game is unplayable for me until there's a workaround -- for now, I think the staff_shortage "feature" should simply be disabled, until it's working properly.  Is there a simuconf.tab setting to turn it off so that the factories will work?

Yes, you can set factory_enforce_demand to 0.
(Signature being tested) If you enjoy playing Simutrans, then you might also enjoy watching Japan Railway Journal
Available in English and simplified Chinese
如果您喜欢玩Simutrans的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

neroden

Awesome, thanks.  Now I just have to figure out how to change it in the running game so I don't have to redo my map.  Probably the eastiest way for me is to implement a command-line option to make the game do a force-override load of simuconf.tab for existing save files... this is a useful debugging feature in general probably.

neroden

Quote from: Matthew on April 09, 2023, 03:27:51 PMYes, you can set factory_enforce_demand to 0.
OK that didn't actually work.  So I made a patch to make it work again, and fixed two other bugs I stumbled across while I was at it. 
(See https://github.com/jamespetts/simutrans-extended/pull/603 and https://github.com/jamespetts/simutrans-extended/pull/602 ).

neroden

So turning off factory_enforce_demand now solves the problem for gameplay in the near term.

I'm experimenting with this (with factory_enforce_demand turned back on) and the issue seems to be caused by a multiplicity of factors.
- The first, and largest, is that commuters pick one building, or a few alternative buildings, at random across the entire map, and if those don't have jobs, they give up and don't get a job.  This makes it impossible to have a localized transportation network, and it's also quite unrealistic.  I tried addressing this in a very straightforward way (not the fastest implementation), by changing the core code so that a commuter will just keep looking until they find a building which has a job (with no limit).  This made a significant difference.  (I did not change the algorithm for visitors.)  This is on my fab-experiments branch.
- The second is that commuters will only pick a few buildings with jobs, again, at random across the map.  So if they pick a bunch of faraway buildings and can't get to any of them, they give up.  Again, unrealistic.  I tried addressing this in a straightforward (and again, not particularly fast) way by massively raising max_alternative_destinations_per_job_millionths from 5000 to 500,000 and similarly raising min_alternative_destinations_per_job_millionth from 2500 to 250,000.  I haven't made a branch of pak128-britain for this yet.

These two changes together were quite effective.  The next village over, which has one industry (a fishery), was also having constant staff shortages, and these simply stopped, it now always has staff.

I'm fairly sure these two points are also the source of the late-game unemployment problems.

The game remains "fast enough" and playable for me.

These may be too detrimental on performance to do directly, but it's worth considering designing a system which effectively does the same thing with less "brute force" -- perhaps a system for commuter trips which starts by picking random *factories* to go to, since the list of factories is always much shorter than the list of buildings, and only starts looking at other buildings (the local "industrial buildings") if it doesn't find a factory it can get to.  Going through all the possible factories might be tolerable where going through all the buildings isn't.  This would also eliminate the weird and immersion-breaking phenomenon where staff shortages are fixed by demolishing industrial districts.  As for realism, call it "government industrial policy" to prioritize these factories rather than other industries, and it seems realistic *enough*.

Perhaps one could also implement a system which starts looking at nearer factories first, but I'm not sure whether that would actually be faster or whether it would be desirable. 

Butcherton, with its excessive number of industries compared to population, still has a problem -- there are still some staff shortages -- but every building has at least SOME staff now, and the crucial slaughterhouse is slowly approaching operational functionality (it gets enough staff by August, as opposed to "never" with previous settings).  I think any remaining problem could be addressed through the city growth algorithms, later.

jamespetts

Thank you for your thoughts on this. First of all, I have merged your pull requests. The factory_enforce_demand option is actually an option that appears to have been from a somewhat different system in Standard. The Extended industry staffing features were added later. However, I note that this option remained in the code without doing anything, that a reasonable user would conclude that it would in fact do what you have made it do and that it is a worthwhile option to have, so your code is a sensible improvement. Thank you for that.

Secondly, as to your original problem with your saved game, I am not sure what is causing this, and I do not have the save file for testing. When implementing the industry logistics system (that allows for staff shortages), I did test somewhat extensively and did not end up having the sort of problems that you describe here, at least at default pakset settings. Did you change any settings in simuconf.tab from their default states? It may also be that a very high number of industries relative to towns might cause this.

Thirdly, as to passenger generation generally, this is a large and complex topic. As you may know, Simutrans was the first transport based game to enforce passenger destinations: passengers had one specific destination and would only travel to there, not just anywhere as had been the case in games such as Transport Tycoon that preceded it. Extended (or Experimental as it was at the time) added alternative destinations to mitigate the exponential nature of the network effect otherwise created. There was initially (and I believe still is in Standard) a system which generated more passengers for geographically local destinations, but this caused serious anomalies (reported by confused players in the forums at the time) in that geographically closer but harder to reach destinations (e.g., towns on islands) would demand more passengers than geographically farther but easier to reach destinations. So, I abolished the distance based generation, and instead introduced journey time tolerances as well as vastly increasing the number of alternative destinations and, later, splitting commuting and visiting passengers to give them different tolerances. The industry logistics feature was introduced later still, the aim being to give players an incentive to provide the worker transport on which many 19th century and later industries depended. The testing that I have done shows this to work well in practice, at least with default values (and these are deliberately set to scale dynamically with the size of the population/number of jobs). As you will see, it also works well on the Bridgewater-Brunel server (I suggest trying using this map to test for things that might significantly affect game performance), as there are no staff shortages.

One thing that I am a little unclear about is the following:

Quote- The first, and largest, is that commuters pick one building, or a few alternative buildings, at random across the entire map, and if those don't have jobs, they give up and don't get a job.  This makes it impossible to have a localized transportation network, and it's also quite unrealistic.  I tried addressing this in a very straightforward way (not the fastest implementation), by changing the core code so that a commuter will just keep looking until they find a building which has a job (with no limit).  This made a significant difference.  (I did not change the algorithm for visitors.)  This is on my fab-experiments branch.
- The second is that commuters will only pick a few buildings with jobs, again, at random across the map.  So if they pick a bunch of faraway buildings and can't get to any of them, they give up.  Again, unrealistic.  I tried addressing this in a straightforward (and again, not particularly fast) way by massively raising max_alternative_destinations_per_job_millionths from 5000 to 500,000 and similarly raising min_alternative_destinations_per_job_millionth from 2500 to 250,000.  I haven't made a branch of pak128-britain for this yet.

In particular, I am not sure what the difference is between the first and second of these things. More specifically still, I am not convinced that we need to change the code if we can simply increase min_alternative_destinations_per_job_millionth to such a value that no longer results in excessive staff shortages where there are also ample people who are willing to take jobs. A value that is less than functional infinity (as the first of your changes would entail) will allow us to simulate the fact that not every person is going to be capable of doing every sort of job (even within the same wealth bracket), and so a system that ensures that all fillable vacancies are always filled will not be entirely accurate. However, a high value that ensures that most vacancies are filled would simulate the fact that people tend to prioritise getting a job over getting the ideal job. (One planned feature of the intended new town growth system is that people will leave towns where the commuting success rate is low and move to towns where the commuting success rate is higher).

I should note as an aside that there was a code change a year or two that significantly improved performance in the passenger generation code by not attempting to look at the journey time data for journeys that would in the most favourable possible conditions be outside passengers' journey time tolerance (e.g., a journey of 1,000km where the journey time tolerance is 5 minutes and the fastest form of transport currently available at the present point in the timeline travels at 100km/h). A higher value for this setting may thus now be tolerable than when this was first introduced. However, this should be tested with a large map (e.g., one from the Bridgewater-Brunel server).

In terms of picking subsets of buildings: the way in which the passenger generation system works is that it picks a building from a list of commuter targets. Commuter targets are all buildings that have jobs. These include industries, but also include offices, town halls, depots, station buildings, signal boxes, etc.. One future balancing change planned to coincide with the planned town growth modification is to reduce the number of non-industry commuter targets of the city building type (especially non-industry shops and "city" industrial buildings) and increase the number of (functional) industries. The intention is more closely to integrate industries with town growth (using the same placement mechanism for industries as for other town buildings), and also remove the duplication inherent in the code from Standard relating to town growth and population and industry operations (especially the "boost" features) and the core Extended code relating to passenger generation and industry logistics. As to looking at "nearer" commuter targets first, as set out above, this is similar to how the game used to work, and this worked badly and was intentionally changed to the current system to reflect the fact that people care about journey time, not geographical distance, and that journey time is something that players (realistically) have some degree of control over.

In any event, thank you for your contributions in this regard.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

neroden

I just want to confirm my agreement with what you've said -- I believe most of the problems I encountered can be fixed simply by raising the tolerance levels (the number of retries and alternative destinations) so that passengers will look harder for "any job" rather than the ideal job. 

The remaining issues are related to city growth.  Through sheer accident during game generation, a very small town ended up with two markets and a slaughterhouse, which is a massive demand for workers which its local population cannot possibly meet, not even with commuters from the towns which are within range using 1750 technology.  This can happen by coincidence during map generation.  There should be some system for expanding the number of residents to account for the job openings, and if there is, that system clearly isn't working fully, but that is a larger project.  Alternatively, there could be some way to prevent this from happening during map generation (with a different algorithm for assigning new industries to towns) which would also be a larger project.

For map context:  I tend to prefer starting with "sparser" maps with smaller cities, fewer cities, and fewer industries per square kilometer total than Bridgewater-Brunel, and more wide open spaces.  This is a matter of taste and I think it should be possible to keep pak128.britain and experimental viable with this map style. 

The game should be capable of adjusting to this as long as the city/industry proportion is OK, but apparently it isn't quite capable of doing so right now -- as I say, I think most of it is just a matter of tweaking the retry numbers higher, and the rest is a matter of making city growth more responsive to job openings.  I *have* experienced serious staff shortage problems in the Bridgewater-Brunel server too (within the last couple of months), though they resolved themselves after a couple of gameplay years.  I'm not quite sure what was going on there.  It takes a long time for an industry with a staff shortage to resolve itself and start getting workers even after you start shipping workers to it with a lot of taxicabs.

One more thought.  If a preference for closer destinations was reintroduced, I think one would want it to (a) only apply to workers, not to visitors (visitors will intentionally travel to the Grand Canyon from England, workers won't), (b) be based on travel time, and (c) be simple.  I'm not actuallly sure whether it would help since I think there may simply not have been enough population in my city to supply all the workers.  But if I did implement such a preference, I would simply try to fill jobs in industries in the same city before filling jobs somewhere else: I wouldn't make it more complicated than that.


neroden

I'm trying a different fix, in the pakset. 

The problem arose in a village with THREE markets.  The market in pak128.britain-ex has a *very* large number of jobs for 1750.  In fact, I would say, an unrealistically large number of jobs.  The number of jobs means that each market seller serves only... 3 customers?  It is far out of proportion with everything else.  When staff_shortage is on, you have to fill 80% of this bloated number of jobs in order to keep the market operating, which is hard... it requires the entire population of a 750-person village. This becomes especially hard when villages will spawn multiple markets (which is actually totally reasonable, even a small village might have multiple markets).

So I tried cutting the number of total jobs in the market down by a factor of 4.  So each market seller is serving 13 customers in 1750, which seems more plausible.  One market should then only require the entire population of a 200-person village, which should make the villages more viable. We'll see how this works. 

This experiment is on the "fewer-early-market-jobs" branch: https://github.com/neroden/simutrans-pak128.britain

neroden

I am now satsified that this solves the problem.  Pull request at https://github.com/jamespetts/simutrans-pak128.britain/pull/150 ... but it was developed at the end of some other branches, so please review and my other pull requests *first*.

neroden

Well, OK, reducing the capacity of the Market didn't solve the problem by itself.  I replicated the problem in a village with two Apothecaries.

So, I went back to the code.  I now have a patch which makes the game work harder to find jobs.  For commuters (and only commuters), if the first building targeted has no available jobs or has no input supply, OR if it's beyond the maximum possible travel distance, the game will keep looking for other buildings until it finds one which is within range and has jobs -- not quite infinitely but many, many tries if it has to.

The problem was this:
(1) The village is out of commuting range of any other village, by the fastest vehicle, period.
(2) The commuters from the village were trying to commute to buildings which were out of range, and never trying the buildings in range.

I have tested and this appears to fix it, reliably.  They will now find the buildings in range and go to work.
https://github.com/jamespetts/simutrans-extended/pull/612

(The job capacity of the Market still needs to be reduced, as well -- that patch needs to go into the pakset.  The village does not generate enough commuters to supply a 240-job Market, period.)

I will now wait for James to catch up with my flurry of patches to both the game and the pakset....

Matthew

Neroden, thank you for your many patches, which I am starting to explore.

I tried testing a branch containing all of the recent patches from you and ceeac to the main branch (so excluding the ex-15 ones) and I can make new maps without difficulty, but I can't load savegames from before these patches. Obviously the next step is to isolate the problem to one of these branches, but I thought I'd mention it as something for other testers.

My understanding is that none of these patches require any pakset changes. Is that correct?
(Signature being tested) If you enjoy playing Simutrans, then you might also enjoy watching Japan Railway Journal
Available in English and simplified Chinese
如果您喜欢玩Simutrans的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

Matthew

Hmm. All of the individual pull request branches seem to load savegames without difficulty. So the most likely explanation is that I have somehow botched the merges.

More usefully, when I loaded a recent B-B save into the find-jobs-clean branch, it was immediately apparent that performance was significantly worse; the game was visibly pausing before each (?)step.
(Signature being tested) If you enjoy playing Simutrans, then you might also enjoy watching Japan Railway Journal
Available in English and simplified Chinese
如果您喜欢玩Simutrans的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

jamespetts

Thank you both for your replies. First of all, Matthew: the issue with the saved games was that there was an error in the code in one of Nathanael's patches relating to the save versioning. Since Nathaniel last worked on Extended, the minor version and save game version have been split, so that, to change the saved game, one needs now to change the minor version not the saved game version. I have fixed this by incrementing the saved game version. Note that incrementing the major version has the same effect as before: only minor versions are now different.

As to the gameplay issues: Nathanael, can I check: what does the patch do (in practical terms) that increasing the number of alternative destinations for commuters to an extremely high level does not do? I am not sure that I quite follow at this stage the advantage of hard-coding this over just changing the simuconf.tab setting.

In terms of gameplay, fundamentally, I think, the correct solution to this is in the town generation/growth algorithm and making sure that town growth (and decline) is to a large extent based on available jobs, just as in reality.

In terms of the market, remember, as with the city buildings, the population (and job) density of the buildings is intended to represent the population (and job) density of the buildings that would fit into the area occupied by the building at 125 meters per tile: so, the market has to have the number of jobs that the number of markets that would in reality fit into a space of 125 x 250 meters would accommodate.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Matthew

Quote from: jamespetts on May 03, 2023, 11:57:10 PMThank you both for your replies. First of all, Matthew: the issue with the saved games was that there was an error in the code in one of Nathanael's patches relating to the save versioning. Since Nathaniel last worked on Extended, the minor version and save game version have been split, so that, to change the saved game, one needs now to change the minor version not the saved game version. I have fixed this by incrementing the saved game version. Note that incrementing the major version has the same effect as before: only minor versions are now different.

Thank you for taking the time to explain that, James. So it seems that the major version is only increased where there is a major new group of features such as the ex-15 branch?

Could you please add a comment in simversion.h summarizing this versioning policy? I guess that you've already announced or agreed this on the forum, but putting a note at the point of decision seems helpful for newbies and old hands alike in this case.
(Signature being tested) If you enjoy playing Simutrans, then you might also enjoy watching Japan Railway Journal
Available in English and simplified Chinese
如果您喜欢玩Simutrans的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

neroden

Quote from: jamespetts on May 03, 2023, 11:57:10 PMThank you both for your replies. First of all, Matthew: the issue with the saved games was that there was an error in the code in one of Nathanael's patches relating to the save versioning. Since Nathaniel last worked on Extended, the minor version and save game version have been split, so that, to change the saved game, one needs now to change the minor version not the saved game version. I have fixed this by incrementing the saved game version. Note that incrementing the major version has the same effect as before: only minor versions are now different.

As to the gameplay issues: Nathanael, can I check: what does the patch do (in practical terms) that increasing the number of alternative destinations for commuters to an extremely high level does not do? I am not sure that I quite follow at this stage the advantage of hard-coding this over just changing the simuconf.tab setting.

In terms of gameplay, fundamentally, I think, the correct solution to this is in the town generation/growth algorithm and making sure that town growth (and decline) is to a large extent based on available jobs, just as in reality.

In terms of the market, remember, as with the city buildings, the population (and job) density of the buildings is intended to represent the population (and job) density of the buildings that would fit into the area occupied by the building at 125 meters per tile: so, the market has to have the number of jobs that the number of markets that would in reality fit into a space of 125 x 250 meters would accommodate.
Aaargh, thanks for explaining that the versioning has changed -- this always makes a nightmare.
In gameplay terms, the patch (git commit ffbcb9d ) by itself both fixes a bug, bluntly, and makes the game run faster.
In current code, increasing the number of alternative destinations for the passenger will just keep finding more out-of-range destinations which can't possibly be reached by the fastest vehicle.  Once the maximum number of alternative destinations (which can't possibly be reached) are found, the game gives up and the passenger stops looking for a job.
With the patch, alternative destinations which can't possibly be reached aren't counted, and the passenger will find a number of alternative destinations which are in theoretically-reachable range.  I think this makes more sense.  In order to get the same result by jacking up the number of alternative destinations, you would have to jack it up very high, which would *work*, but would be finding unnecessary alternative destinations for the case where the passenger already has found a bunch of alternative destinations within range... thus running much slower. 
With the patch, we stop if we find the number of "real" alternative destinations set in the pakset, which keeps the number of searches down, so that it doesn't use much computer time.  The cases where it needs to find a lot of alternative destinations are all on the shortcut code route of "it's too far away".
I tried a lot of different things to fix this.  This is the one which worked.
I apologize for having only provided the pull request as a pull on top of the other patches -- it can be cherry-picked if needed.

Regarding the market, 240 is just gameplay-breaking, it fails catastrophically in low-population cities.  Since you have only 20% of trips as work trips -- and the parish hall has 40 jobs -- in order to have the market supplied with 80% of jobs, you effectively you need a town of size 1200 to have a decent chance of not having a staff shortage, and that's if it has *no* other industries (it will have other industries).  Smaller towns than that are generated.  It's gameplay-breaking.

In terms of realism, 240 is also completely out of whack.  My local farmers' market is about 200 m x 125 m, so about the same size as the one in game.  Perhaps you forgot to adjust for the fact that markets *usually don't operate 7 days a week*.  Mine is only open twice a week: it has about 170 workers in the stalls on a packed day (so 240 is way too high even then) and only about half that number on the "off" day.  (So if you adjust for average numbers of workers per day, it's about 36.  That makes 40 a reasonable number.) 

Though in fact, the workers are almost all *also workers on the farms* so it's double-counting to have them on the market and the farms.

I did realize while I was thinking about this that there's a long-term logical way to handle the problem of getting workers to the farms without just turning off "staff shortage" for rural industries: in reality, the workers lived on the farms.  I'm not sure if the current codebase allows for a factory building to have both population and workers, but if it doesn't, it would be a possible project to (a) change it so it does, and (b) allow workers to go to work at the same building they live at.    Meanwhile other sorts of rural industries would not have workers living on site and would actually require that workers travel to them (possibly from the farms!).  This would then automatically simulate the rather interesting phenomenon that, as transportation gets better, the people living on the farms may start working in town or at the mine instead of on the farm...

These changes, along with forcibly increasing the minimum buffer size to 7 (as noted elsewhere) were enough to make gameplay work.  But I have run into another problem, which I'm going to report in a separate bug report.  :sigh:


Matthew

Quote from: neroden on May 06, 2023, 12:15:47 PMThough in fact, the workers are almost all *also workers on the farms* so it's double-counting to have them on the market and the farms.

I did realize while I was thinking about this that there's a long-term logical way to handle the problem of getting workers to the farms without just turning off "staff shortage" for rural industries: in reality, the workers lived on the farms.  I'm not sure if the current codebase allows for a factory building to have both population and workers, but if it doesn't, it would be a possible project to (a) change it so it does, and (b) allow workers to go to work at the same building they live at.    Meanwhile other sorts of rural industries would not have workers living on site and would actually require that workers travel to them (possibly from the farms!).  This would then automatically simulate the rather interesting phenomenon that, as transportation gets better, the people living on the farms may start working in town or at the mine instead of on the farm...

I have been waiting several years for the town growth rework to make this point.....  ::'(

I have so many thoughts about this and drafted a Big Post in my mind many times in the shower or on the bus ;D, But I did not want to make James or anyone else feel hassled. However since you have raised it...

This is the way that the Industrial Revolution historically began and it is well within Sim-Ex's capabilities to model it closely. Like you, my initial ideas revolved around having Farms that were both residential and Factories. However, the recently released Victoria 3 had a different approach which I think might be somewhat easier to code. They distinguish between Subsistence Farms and regular 'Commercial' Farms. Subsistence Farmers would live on their farm and primarily feed themselves. In game terms, this would be a new type, similar to a 'residential citybuilding', but placed outside cities and generating Visitors but not Commuters. But subsistence farmers do produce a small surplus and sell it if they are sufficiently connected to the wider economy. We should not model this in Sim-Ex as freight, because the quantities were too small for this in real life (taking a pig to market once a year, that kind of thing) and we want to avoid the complexity of a building that is a Factory-citybuilding hybrid. However, if Subsistence Farms met a certain threshold of success (e.g. 100% Visitor success rate over some time period) then they should trigger wider economic growth. There's more than one way to proceed after that, but in my view the best way would be to generate a Migrant packet that then moves to another citybuilding (or an empty tile from a pre-selected list) and makes it level up, so that only tiles with adequate transport developed. You could then also re-use those functions to have certain citybuildings have a very small chance of generating an Entrepeneur packet who heads off to build a Factory. Of course, this might include a Commercial Farm, which would work the same as existing in-game Farms.
(Signature being tested) If you enjoy playing Simutrans, then you might also enjoy watching Japan Railway Journal
Available in English and simplified Chinese
如果您喜欢玩Simutrans的话,那么说不定就想看《日本铁路之旅》(英语也有简体中文字幕)。

neroden

I'm gonna point out that even in the modern day and age the owners of commercial farms frequently live on it, so I kind of want a building with both residents + employees anyway.  Long term project though, I think there was some questionable data structure design inherited from early versions of Standard which would have to be untangled.

neroden

ANYway, I would really like to get this patch in.  What it does:
-- when looking for alternative commuter destinations, the game will find the appropriate (pak-specified) number of alternative commuter destinations which are
(a) within the range of the fastest possible vehicle,
and (b) actually have jobs.

This means that as transportation improves, it is still possible for villages to empty out and stop supplying local jobs (as the residents desire to go work in some other city) but when it's physically impossible for them to get there, they won't.

The additional time spent searching for additional destinations will apply primarily in early game and so this should be faster in the late part of a long game than adding additional destinations generically at all times.

James -- if we do go with the alternative of just jacking up the number of alternative destinations massively, which I believe would be slower, we should then also take out all the special-case code which is trying to look for additional destinations when the destination doesn't have jobs.  I'm building on code which *already* decides to look for additional destinations when the destination doesn't have jobs -- I'm just adding that it looks for additional destinations when the destination is unreachable with current technology.  The special casing is already there, I'm just improving it.

I cleaned up and refactored the branch (using cherry-pick, it'll all work right in git), so you can apply this patch before the one about minimum factory capacity 7 (which could use some work to make it less dependent on the hardcoded number 7). 

They're conceptually independent.  Here's the new cleaned-up pull request.

https://github.com/jamespetts/simutrans-extended/pull/613

P.S.  Thanks for applying the market capacity patch.

jamespetts

Thank you all for your input. I will address this topic under separate headings.

Subforum for this topic

I have moved this to future development discussion, as the current behaviour is not a bug: the code is working as designed. What is under discussion now is whether the design could and should be improved and, if so, how.


Versioning

This has been the case for many years, and is already documented in simvesrion.h by whoever implemented it years ago:

// Beware: SAVEGAME minor is often ahead of version minor when there were patches.
//  These have no direct connection at all!
#define SIM_SAVE_MINOR      7
#define SIM_SERVER_MINOR    7

#define EX_VERSION_MAJOR 14
#define EX_VERSION_MINOR 21
#define EX_SAVE_MINOR 61

I should note that the major version is indeed intended to be reserved for very major overhauls of the code all done at once, of which the ex-15 branch is the current next step. The idea is that we can implement major features slowly in the ex-15 branch using the major version and still implement new minor features on the master branch using the save/minor versions without having to keep altering the save game code on the major branch. The idea is that major version number increases are for code changes that have to be done as a very major project and will take such a long time to work on that a large number of minor changes and bug fixes to the current master branch will become necessary while work on the major branch is continuing.

Commuting passengers' destination searching algorithm

As I understand the proposed code, it is summarised in this commit. The critical part is this:

if (trip == commuting_trip && extend_count < destination_count * 1024)
{
// Keep looking for jobs in the case of a commuting trip.
// This is critically important in early game on a large map
// with a disconnected network and slow travel times.
extend_count++;
}

This is only invoked when the following condition is true (retained from the existing code):

if (skip_route_checks || (!has_private_car && !can_walk && start_halts[passenger_generation_thread_number].empty()))
In other words, the number of alternative destinations effectively increases by 1 for commuting passengers every time that the passengers either (1) are too far to walk, have no private car and are not within range of any passenger enabled public transport stop; or (2) skip_route_checks is true, which means that the passengers are in range of a public transport stop, but the destination is too far away to be possibly reachable by public transport within the passengers' journey time tolerance.

I am struggling to see the economic sense in this. In reality, the fact that a public transport stop has opened in the vicinity which in theory could but in fact does not serve possible workplaces or the fact that a new, faster form of transport has been invented which in theory could, but in fact does not, connect to possible workplaces could have no possible effect on whether any given worker fills any given job that is in fact with commutable range. Why should the invention of the Concorde affect what local jobs get filled?

You suggest that this is faster than simply increasing the number of alternative destinations, but can I check whether you have tested how much faster that this is in reality?

If the function that we want is for the algorithm to keep looking for jobs much longer before it gives up, the most sensible way of achieving this would be to alter the simuconf.tab settings dedicated to this function.

The current values are:

# Passengers who are not able to find a route will be content to go to an alternative destination.
# This is set separately for visiting passengers and commuting passengers. It can either be set in
# absolute figures using max_alternative_destinations_visiting and max_alternative_destinations_commuting,
# or set as a proportion of the total number of jobs and total level of visitor demand, using
# max_alternative_destinations_per_visitor_demand_millionths and max_alternative_destinations_per_job_millionths.
# Using the relative settings is recommended, as this scales much better to the map size.
#
# This is specified as the maximum number of alternative destinations, as the actual number for any
# given packet of passengers will be randomised between zero and this number.
#
# This figure strongly affects how many passengers will travel, and also can have a significant effect on
# the computational load.

#max_alternative_destinations_commuting = 6
#max_alternative_destinations_visiting = 12
#min_alternative_destinations_commuting = 2
#min_alternative_destinations_visiting = 1

max_alternative_destinations_per_visitor_demand_millionths = 2750
max_alternative_destinations_per_job_millionths = 5000

min_alternative_destinations_per_visitor_demand_millionths = 750
min_alternative_destinations_per_job_millionths = 2500

All of these values are stored as unsigned 32 bit integers, meaning that we can have much larger values than the current values of a minimum of 2,500 and a maximum of 5,000.

Can I ask whether you have tested simply increasing these values (use the advanced settings dialogue, accessible using the "i" key, to change the values in a running game: changing simuconf.tab will only affect new games)? I am certainly open to increasing these values if we are having balance problems, albeit on the basis of testing. As indicated in an earlier post, I had many years ago now tested the logistics and passenger generation systems before implementation using the default settings (including default town sizes) and had not found the problem that you had described. It may be that, if you are decreasing the town size when you start a map, you need to increase significantly the min and max alternative destinations per job millionths settings.

As also indicated in a previous post, an important thing that we need to simulate is the fact that not everybody is suitable for every job (even within the same broad wealth bracket as simulated), so a system in which all commuting passengers will inevitably fill all reachable jobs is not something to aim for: to get 100% of jobs filled, you should need slightly more than 100% of the workers necessary within a reachable range.


Size of the market

I believe that you may be right that I had not accounted for the typical market not being open all week. We do not take account of week-ends generally in Simutrans-Extended, but markets are often open fewer days per week than ordinary shops. Assuming that a market will typically run on two out of every five days and that a modern day market would have 170 stallholders, extrapolating that back through the sizes of markets in the game, that gives us an employment capacity of about 35 (in the .dat file: this is translated into a higher value in-game) for the earliest version of the market and 65-75 for the later versions.

I have made the modification to the pakset, which should be available in the next nightly build.


People living on farms

This is actually something that I had been planning for the town growth overhaul (i.e., allowing buildings that have both residents and jobs, including industrial buildings), although it would need substantial work. The town growth overhaul should also greatly improve balancing issues relating to staff shortages, since people would tend to leave areas of high unemployment (i.e., areas with a low commuting success rate) and move to areas of low unemployment (i.e., areas with a high commuting success rate), which would also strongly tend to be places with the most unfilled jobs.

One of the difficulties at the present with progressing towards the town growth overhaul is that the current major work on the ex-15 branch needs to be done first. This is a particularly large project. Substantial work was done on this earlier in the year and also last year, but we are now hampered by the fact that our UI coder is away indefinitely (I mean no criticism of Ranran, of course). This will, however, make it much more difficult for me to progress this work. If anyone would like to assist with the UI for the ex-15 branch (even a much more basic UI than Ranran would have designed will be ample; at present, I cannot even get the UI working to a state where I can test what I have already coded), that would be most welcome (but should probably be discussed in another thread).


Minimum buffer size

As to this commit, from what I understand, all that it does is override the .dat file settings of any industry whose input and/or output storage is set to be <7 and force them to be exactly 7.

Whilst I can understand that this sort of code is potentially useful for testing, I do not think that this is a sensible way of maintaining the codebase. If we have a setting for the input storage in the .dat files, then the game should respect that setting. If we find that, as a matter of game balance, it is best to have a value of at least 7, then we should increase the values in the .dat files to at least 7, rather than write code to override the settings in the .dat files. The code should do what it purports to do rather than silently overriding values set by pakset designers.

As to the number 7, can I check how you arrived at this? We may well need to make changes to the pakset to increase the numbers to make industry production flow better and it would help me to understand your thinking so that I know how best to compute this.


**

In any event, thank you very much for your work and contributions on this topic. Even though I do not entirely agree with the approach adopted in some respects, as set out above, I do very much appreciate your contributions.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

neroden

OK, so here's the problem with trying to solve the capacity issue in the dat files.  I tried to increase the values in the .dat files to at least 7, but THE GAME DOESN'T USE THE VALUES IN THE DAT FILES.

Sorry for yelling.  This was driving me nuts.  The game is scaling the values in the .dat files by so many different factors that it was flatly impossible to figure out which ones I needed to change in order to increase the final, computed capacity to at least 7.

Are you willing to eliminate all the scaling and have the game use the RAW values in the .dat files?  No screwing around with meters_per_tile, no screwing around with multiplying by the production_factor, none of that?  If we do that, then I can simply set the values in the .dat files to at least 7.  I would actually really like this, but it would be some major changes.

If not, sorry, it has to be overridden in the core code.  It's too error-prone to try to go through the dat files and figure out which ones are going to end up having capacities lower than 7 after all the scaling factors are applied.  I tried.  I failed.

The number 7 is simply the capacity of the wagon for perishable goods.  It needs to be possible to deliver a full wagonload in order to make the wagon economically viable, and it's supposed to be economically viable.  I checked the later game, and again, the small trucks don't get much bigger than this.  (For whatever reason, the wagon size is 6 for most other goods, but 7 for perishable goods; IDK; but having the extra buffer of 1 is a good thing anyway.)

If the listed capacity numbers are going to be scaled every which way to Friday so that the in-game numbers look nothing like the numbers in the .dat file, it's far too easy to make a mistake and end up with a too-low-capacity industry.  There needs to be a sanity check.  A parameter in simuconf.tab for the pak with an absolute minimum capacity would be the way to do the sanity check; the hardcoded "7" should really be a simuconf.tab parameter.

I would certainly prefer it if all the scaling factors were removed and the .dat file numbers were the same ones appearing in game. But do we have a practical pathway to do that?  Won't it break other things?

jamespetts

Quote from: neroden on May 08, 2023, 02:04:12 PMOK, so here's the problem with trying to solve the capacity issue in the dat files.  I tried to increase the values in the .dat files to at least 7, but THE GAME DOESN'T USE THE VALUES IN THE DAT FILES.

Sorry for yelling.  This was driving me nuts.  The game is scaling the values in the .dat files by so many different factors that it was flatly impossible to figure out which ones I needed to change in order to increase the final, computed capacity to at least 7.

Are you willing to eliminate all the scaling and have the game use the RAW values in the .dat files?  No screwing around with meters_per_tile, no screwing around with multiplying by the production_factor, none of that?  If we do that, then I can simply set the values in the .dat files to at least 7.  I would actually really like this, but it would be some major changes.

If not, sorry, it has to be overridden in the core code.  It's too error-prone to try to go through the dat files and figure out which ones are going to end up having capacities lower than 7 after all the scaling factors are applied.  I tried.  I failed.

The number 7 is simply the capacity of the wagon for perishable goods.  It needs to be possible to deliver a full wagonload in order to make the wagon economically viable, and it's supposed to be economically viable.  I checked the later game, and again, the small trucks don't get much bigger than this.  (For whatever reason, the wagon size is 6 for most other goods, but 7 for perishable goods; IDK; but having the extra buffer of 1 is a good thing anyway.)

If the listed capacity numbers are going to be scaled every which way to Friday so that the in-game numbers look nothing like the numbers in the .dat file, it's far too easy to make a mistake and end up with a too-low-capacity industry.  There needs to be a sanity check.  A parameter in simuconf.tab for the pak with an absolute minimum capacity would be the way to do the sanity check; the hardcoded "7" should really be a simuconf.tab parameter.

I would certainly prefer it if all the scaling factors were removed and the .dat file numbers were the same ones appearing in game. But do we have a practical pathway to do that?  Won't it break other things?

The long-term solution is likely to be the rewrite of the distance/time scaling that you are planning for next year. One thing that I think is important to keep the project maintainable is never to have short-term goals conflict with long-term goals. Thus, if we are to have a different scaling system in the future that will eliminate the need for setting raw values anywhere, having code that overrides raw values with a hard-coded number that is calibrated to the old system conflicts with this.

What might be better is to have a simuconf.tab setting for minimum_industry_input_storage_raw and minimum_industry_output_storage_raw to address this; if we set this to a default of 0, it will have no effect. Only if set to a non-zero number will this have any effect. That way, we can deal with the issues relating to the unit translations without storing up problems for the future or making the code delibreately fail to do what it is claiming to do.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

neroden

I have tried expanding the number of alternative commuting destinations by a factor of 1000 in simuconf.tab, to 5,000,000 

It... helped some, I guess... but it did not work as reliably as the code fix.  I'm not 100% sure why.

If we are to do it that way, clearly it must be expanded far more than that.  A minimum setting might be 1,000,000,000 (one billion) per millionth.  I We are getting close to the limits of int32 at this point, which cannot hold a value of 5 billion.

If we are going to do it that way, we should rip out all the extend_count code entirely.  It dated from when the number of alternative destinations was in the vicinity of 5 to 12.

I had not spotted the skip_route_checks aspect of the check.  I was focused on the other half of the check.

neroden

Quote from: jamespetts on May 08, 2023, 02:13:42 PMThe long-term solution is likely to be the rewrite of the distance/time scaling that you are planning for next year. One thing that I think is important to keep the project maintainable is never to have short-term goals conflict with long-term goals. Thus, if we are to have a different scaling system in the future that will eliminate the need for setting raw values anywhere, having code that overrides raw values with a hard-coded number that is calibrated to the old system conflicts with this.

What might be better is to have a simuconf.tab setting for minimum_industry_input_storage_raw and minimum_industry_output_storage_raw to address this; if we set this to a default of 0, it will have no effect. Only if set to a non-zero number will this have any effect. That way, we can deal with the issues relating to the unit translations without storing up problems for the future or making the code delibreately fail to do what it is claiming to do.
Agree 100%.  I simply haven't had the bandwidth to modify my patch to replace "7" with something which reads those new simuconf.tab settings.

neroden

Quote from: jamespetts on May 08, 2023, 01:17:07 PMThank you all for your input. I will address this topic under separate headings.

Subforum for this topic

I have moved this to future development discussion, as the current behaviour is not a bug: the code is working as designed. What is under discussion now is whether the design could and should be improved and, if so, how.


Versioning

This has been the case for many years, and is already documented in simvesrion.h by whoever implemented it years ago:

// Beware: SAVEGAME minor is often ahead of version minor when there were patches.
//  These have no direct connection at all!
#define SIM_SAVE_MINOR      7
#define SIM_SERVER_MINOR    7

#define EX_VERSION_MAJOR 14
#define EX_VERSION_MINOR 21
#define EX_SAVE_MINOR 61

I should note that the major version is indeed intended to be reserved for very major overhauls of the code all done at once, of which the ex-15 branch is the current next step. The idea is that we can implement major features slowly in the ex-15 branch using the major version and still implement new minor features on the master branch using the save/minor versions without having to keep altering the save game code on the major branch. The idea is that major version number increases are for code changes that have to be done as a very major project and will take such a long time to work on that a large number of minor changes and bug fixes to the current master branch will become necessary while work on the major branch is continuing.
I do not understand the current policies on version_minor versus save_minor.  Is version_minor used for, um, anything now?  Or should I just use save_minor in all cases where I have to update the save game format?

jamespetts

Quote from: neroden on May 08, 2023, 02:19:43 PMI have tried expanding the number of alternative commuting destinations by a factor of 1000 in simuconf.tab, to 5,000,000 

It... helped some, I guess... but it did not work as reliably as the code fix.  I'm not 100% sure why.

If we are to do it that way, clearly it must be expanded far more than that.  A minimum setting might be 1,000,000,000 (one billion) per millionth.  I We are getting close to the limits of int32 at this point, which cannot hold a value of 5 billion.

If we are going to do it that way, we should rip out all the extend_count code entirely.  It dated from when the number of alternative destinations was in the vicinity of 5 to 12.

I had not spotted the skip_route_checks aspect of the check.  I was focused on the other half of the check.


Thank you for sharing your test results - that is interesting. We probably need to understand the reason for the difference in behaviour that you were getting between increasing this number to one million and your code patch to understand why the latter seemed to be so much more effective.

As to "extend_count", I think that you are right that this is from another time; I had forgotten what this was intended to do until I went back and checked the commit history. This is from 2017 with the commit comment,

QuoteCHANGE: Do not count "destination unavailable" in commuting trips towards the total destination count until this number reaches 4x the originally assigned number of alternative destinations.

This does seem to be at odds with current behaviour. I have now reverted this. I have also more than doubled the number of alternative destinations for commuting passengers in the pakset.

Quote from: neroden on May 08, 2023, 02:27:10 PMI do not understand the current policies on version_minor versus save_minor.  Is version_minor used for, um, anything now?  Or should I just use save_minor in all cases where I have to update the save game format?

The minor version number is intended to be incremented when any non-trivial new feature is implemented. This is distinct from a bugfix, performance improvement, code cleaning exercise, minor rebalancing, etc.. We have probably not always remembered to increment this number, however. It is not fundamentally important, so forgetting is not a major problem, but helps players to understand when something has changed.

Quote from: neroden on May 08, 2023, 02:20:35 PMAgree 100%.  I simply haven't had the bandwidth to modify my patch to replace "7" with something which reads those new simuconf.tab settings.


Don't worry - we're all busy people.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

neroden

Quote from: jamespetts on May 08, 2023, 01:17:07 PMCommuting passengers' destination searching algorithm

As I understand the proposed code, it is summarised in this commit. The critical part is this:

if (trip == commuting_trip && extend_count < destination_count * 1024)
{
// Keep looking for jobs in the case of a commuting trip.
// This is critically important in early game on a large map
// with a disconnected network and slow travel times.
extend_count++;
}

This is only invoked when the following condition is true (retained from the existing code):

if (skip_route_checks || (!has_private_car && !can_walk && start_halts[passenger_generation_thread_number].empty()))
In other words, the number of alternative destinations effectively increases by 1 for commuting passengers every time that the passengers either (1) are too far to walk, have no private car and are not within range of any passenger enabled public transport stop; or (2) skip_route_checks is true, which means that the passengers are in range of a public transport stop, but the destination is too far away to be possibly reachable by public transport within the passengers' journey time tolerance.

I am struggling to see the economic sense in this.
OK.  So the code first picks a completely random destination building somewhere on the map.  Then it checks:
(1) is it possible for the passengers to get there by walking?
(2) do the passengers have a private car?
(3) are the passengers in range of a public transportation stop, and the destination is theoretically reachable by public transport within the journey time tolerance if someone built a route?

If all three of these fail, then it finds another destination.  The economic sense here is that passengers will not even consider going to destinations where all three of these fail. (It should also fail if the passengers have a private car but it's too far to get there by private car within journey time tolerates, but this is not currently implemented.)

The concept is that commuters will try to get a job at a distant location if they could theoretically get there within journey time tolerance (yes, some people commuted by Concorde) even if nobody is currently serving the route.  You wrote this code, incidentally -- IIRC, it's your concept.  I liked it at the time.

By adding additional destination searches only for these totally hopeless destinations, this still allows commuters to fail to find a job.  If there are lots of jobs within commuting range but they aren't actually served by public transport (and no car, and not within walking range), then the commuters will fail to find a job.  It also allows commuters to choose to use a car (creating traffic) when there are jobs within commuting range but no transport.

If we instead simply jack the destination search total way up in general, we guarantee that commuters will keep looking until they find a job, for the entire game timeline.  They will *always* find a job. Maybe this is preferable!

We have to jack it WAY up.  I tried multiplying it by 1000 (from 2500 / 5000 to 2500000 / 5000000) and it wasn't enough.  (Not entirely sure why.)

The reason I expect this to make a difference in speed is the following.  The short-cut code paths which avoids the full path searching code is fast.  The full path searching code is not fast.  You implemented the shortcut code path to avoid checking routes if it was beyond theoretical range for *this exact reason*.

If we jack up the destination search in total, it WILL, in late game, be running through the full path searching code, looking for jobs within theoretical range (which aren't served properly / aren't fast enough), many more times than it currently does.  If we implement something like my current code patch, we will only increase the count on the shortcut code path, which won't take much processing time.

I realize it's not perfect realism, but this is on the critical path for the speed of the program.  Incrementing a counter and looping back fast on the shortcut path (within cache) is not going to impact the speed of the program.  Running through the entire route_status loop, which is *long* and *involved*, a lot more times, IS going to impact the speed of the program. 

The key point regarding speed here is that I am only increasing the number of destinations on code paths which are followed immediately by
continue; and not on code paths which are doing this:

 
// Check for a suitable stop within walking distance of the destination.

// Note that, although factories are only *connected* now if they are within the smaller factory radius
// (default: 1), they can take passengers within the wider square of the passenger radius. This is intended,
// and is as a result of using the below method for all destination types.

//minivec_tpl<const planquadrat_t*> const &tile_list_3 = current_destination.building->get_tiles();

// The below is not thread safe
//if(tile_list_3.empty())
//{
// tile_list_3.append(access(current_destination.location));
//}
#ifdef MULTI_THREAD
destination_list[passenger_generation_thread_number].clear();
#else
destination_list.clear();
#endif
//vector_tpl<halthandle_t> destination_list(tile_list_3
[0][o]->get_haltlist_count() * tile_list_3.get_count());

 if (current_destination.building->get_tiles().empty())
 {
 const planquadrat_t* current_tile_3 = access(current_destination.location);
 const nearby_halt_t* halt_list = current_tile_3->get_haltlist();
 for (int h = current_tile_3->get_haltlist_count() - 1; h >= 0; h--)
 {
 halthandle_t halt = halt_list[h].halt;
 if((trip == mail_trip && halt->get_mail_enabled()) || (trip != mail_trip && halt->get_pax_enabled()))
 {
 // Previous versions excluded overcrowded halts here, but we need to know which
 // overcrowded halt would have been the best start halt if it was not overcrowded,
 // so do that below.
#ifdef MULTI_THREAD
 destination_list[passenger_generation_thread_number].append(halt);
#else
 destination_list.append(halt);
#endif
 }
 }
 }
 else
 {
 FOR(minivec_tpl<const planquadrat_t*>, const& current_tile_3, current_destination.building->get_tiles())
 {
 const nearby_halt_t* halt_list = current_tile_3->get_haltlist();
 if (!halt_list)
 {
 continue;
 }
 for (int h = current_tile_3->get_haltlist_count() - 1; h >= 0; h--)
 {
 halthandle_t halt = halt_list[h].halt;
 if ((trip == mail_trip && halt->get_mail_enabled()) || (trip != mail_trip && halt->get_pax_enabled()))
 {
 // Previous versions excluded overcrowded halts here, but we need to know which
 // overcrowded halt would have been the best start halt if it was not overcrowded,
 // so do that below.
#ifdef MULTI_THREAD
 destination_list[passenger_generation_thread_number].append(halt);
#else
 destination_list.append(halt);
#endif
 }
 }
 }
 }

 best_journey_time = UINT32_MAX_VALUE;
 uint32 current_journey_time;
#ifdef MULTI_THREAD
 if (start_halts[passenger_generation_thread_number].get_count() == 1 && destination_list[passenger_generation_thread_number].get_count() == 1 && start_halts[passenger_generation_thread_number].get_element(0).halt == destination_list[passenger_generation_thread_number].get_element(0))
#else
 if (start_halts.get_count() == 1 && destination_list.get_count() == 1 && start_halts
[0][o].halt == destination_list.get_element(0))
#endif
 {
 /** There is no public transport route, as the only stop
 * for the origin is also the only stop for the destintation.
 */
#ifdef MULTI_THREAD
 start_halt = start_halts[passenger_generation_thread_number].get_element(0).halt;
#else
 start_halt = start_halts
[0][o].halt;
#endif

 if (can_walk)
 {
 const grund_t* destination_gr = lookup_kartenboden(current_destination.location);
 if (destination_gr && !destination_gr->is_water())
 {
 // People cannot walk on water. This is relevant for fisheries and oil rigs in particular.
 route_status = on_foot;
 }
 }
 }
 else
 {
 // Check whether public transport can be used.
 // Journey start information needs to be added later.
 pax.reset();
 pax.set_zielpos(destination_pos);
 pax.menge = units_this_step;
 //"Menge" = volume (Google)

 // Search for a route using public transport.

 uint32 best_start_halt = 0;
 uint32 best_journey_time_including_crowded_halts = UINT32_MAX_VALUE;

 sint32 i = 0;


#ifdef MULTI_THREAD
 FOR(vector_tpl<nearby_halt_t>, const& nearby_halt, start_halts[passenger_generation_thread_number])
#else
 FOR(vector_tpl<nearby_halt_t>, const& nearby_halt, start_halts)
#endif
 {
 current_halt = nearby_halt.halt;

#ifdef MULTI_THREAD
 // Start with the walking time to the start halt.
 // Note that the walking time to the destination stop is already added by find_route.
 current_journey_time = walking_time_tenths_from_distance(start_halts[passenger_generation_thread_number].get_element(i).distance);

#else
 current_journey_time = walking_time_tenths_from_distance(start_halts[i].distance);
#endif
 if (current_journey_time < best_journey_time && (current_journey_time < walking_time || !can_walk) && current_journey_time < tolerance)
 {
 // Do not hit the database with a request if even walking to the local stop takes longer than the tolerance time, is worse than the best journey time, is worse than simply walking to the destination
 // or if the impicit speed taking into account the time taken to walk to the origin stop.
 const uint32 distance_this_origin_to_destination = shortest_distance(nearby_halt.halt->get_basis_pos(), current_destination.location);
 const uint32 distance_this_origin_to_destination_km = (distance_this_origin_to_destination * get_settings().get_meters_per_tile()) / 1000u;
 const uint32 origin_stop_specific_implicit_minimum_speed_kmh = tolerance > current_journey_time ? tolerance - current_journey_time > 0 ? (distance_this_origin_to_destination_km * 600) / tolerance : 0 : UINT32_MAX_VALUE;

 if(!((tolerance > settings.get_min_wait_airport() && origin_stop_specific_implicit_minimum_speed_kmh > max_convoy_speed_air) || origin_stop_specific_implicit_minimum_speed_kmh > max_convoy_speed_ground))
 {
#ifdef MULTI_THREAD
 const uint32 public_transport_journey_time = current_halt->find_route(destination_list[passenger_generation_thread_number], pax, best_journey_time, destination_pos);
#else
 const uint32 public_transport_journey_time = current_halt->find_route(destination_list, pax, best_journey_time, destination_pos);
#endif
 if (public_transport_journey_time < UINT32_MAX_VALUE)
 {
 if (public_transport_journey_time < (UINT32_MAX_VALUE - current_journey_time))
 {
 current_journey_time += public_transport_journey_time;
 }
 else
 {
 current_journey_time = UINT32_MAX_VALUE;
 }
 }
 else
 {
 current_journey_time = UINT32_MAX_VALUE;
 }
 }
 else
 {
 current_journey_time = UINT32_MAX_VALUE;
 }
 }
 else
 {
 current_journey_time = UINT32_MAX_VALUE;
 }

 // Because it is possible to walk between stops in the route finder, check to make sure that this is not an all walking journey.
 // We cannot test this recursively within a reasonable time, so check only for the first stop.
 if (current_journey_time < UINT32_MAX_VALUE && pax.get_ziel() == pax.get_zwischenziel())
 {
 haltestelle_t::connexion* cnx = current_halt->get_connexions(wtyp->get_catg_index(), pax.get_class())->get(pax.get_zwischenziel());

 if (current_halt->is_within_walking_distance_of(pax.get_zwischenziel()) && (!cnx || (!cnx->best_convoy.is_bound() && !cnx->best_line.is_bound()) || (((cnx->best_convoy.is_bound() && !cnx->best_convoy->carries_this_or_lower_class(pax.get_catg(), pax.get_class())) || (cnx->best_line.is_bound() && !cnx->best_line->carries_this_or_lower_class(pax.get_catg(), pax.get_class()))))))
 {
 // Do not treat this as a public transport route: if it is a viable walking route, it will be so treated elsewhere.
 current_journey_time = UINT32_MAX_VALUE;
 }
 }

 // TODO: Add facility to check whether station/stop has car parking facilities, and add the possibility of a (faster) private car journey.
 // Use the private car journey time per tile from the passengers' origin to the city in which the stop is located.

 if(current_journey_time < best_journey_time)
 {
 if(!current_halt->is_overcrowded(wtyp->get_index()))
 {
 best_journey_time = current_journey_time;
 if(pax.get_ziel().is_bound())
 {
 route_status = public_transport;
 }
 }
 best_journey_time_including_crowded_halts = current_journey_time;
 best_start_halt = i;
 }
 i ++;
 }

 if(best_journey_time == 0)
 {
 best_journey_time = 1;
 }

 if(can_walk && walking_time < best_journey_time && (walking_time <= walking_time_preference_threshold || best_journey_time > tolerance))
 {
 // If walking is faster than public transport, passengers will walk, unless
 // the public transport journey is within passengers' tolerance and the walking
 // time exceeds passenger' walking time preference threshold.
 const grund_t* destination_gr = lookup_kartenboden(current_destination.location);
 if(destination_gr && !destination_gr->is_water())
 {
 // People cannot walk on water. This is relevant for fisheries and oil rigs in particular.
 route_status = on_foot;
 }
 }

 // Check first whether the best route is outside
 // the passengers' tolerance.

 if(best_journey_time_including_crowded_halts < tolerance && route_status != public_transport && walking_time > best_journey_time)
 {
 route_status = overcrowded;
 if(!overcrowded_already_set)
 {
 best_bad_destination = destination_pos;
 best_bad_start_halt = best_start_halt;
 overcrowded_already_set = true;
 }
 }
 else if((route_status == public_transport || route_status == no_route) && best_journey_time_including_crowded_halts >= tolerance && best_journey_time_including_crowded_halts < UINT32_MAX_VALUE)
 {
 route_status = too_slow;

 if(!too_slow_already_set && !overcrowded_already_set)
 {
 best_bad_destination = destination_pos;
 best_bad_start_halt = best_start_halt;
 too_slow_already_set = true;
 }
 }
 else
 {
 // All passengers will use the quickest route.
#ifdef MULTI_THREAD
 if(start_halts[passenger_generation_thread_number].get_count() > 0)
 {
 start_halt = start_halts[passenger_generation_thread_number].get_element(best_start_halt).halt;
#else
 if (start_halts.get_count() > 0)
 {
 start_halt = start_halts[best_start_halt].halt;
#endif
 }
 }
 }

 if(has_private_car)
 {
 // time_per_tile here is in 100ths of minutes per tile.
 time_per_tile = UINT32_MAX_VALUE;
 switch(current_destination.type)
 {
 case town:
 //Town
 if(city)
 {
 time_per_tile = city->check_road_connexion_to(current_destination.building->get_stadt());
 }
 else
 {
 // Going onward from an out of town attraction or industry to a city building - get route backwards.
 if(current_destination.type == attraction)
 {
 time_per_tile = current_destination.building->get_stadt()->check_road_connexion_to(current_destination.building);
 }
 else if(current_destination.type == factory)
 {
 time_per_tile = current_destination.building->get_stadt()->check_road_connexion_to(current_destination.building->get_fabrik());
 }
 }
 break;
 case factory:
 if(city) // Previous time per tile value used as default if the city is not available.
 {
 time_per_tile = city->check_road_connexion_to(current_destination.building->get_fabrik());
 }
 break;
 case attraction:
 if(city) // Previous time per tile value used as default if the city is not available.
 {
 time_per_tile = city->check_road_connexion_to(current_destination.building);
 }
 break;
 default:
 //Some error - this should not be reached.
#ifdef MULTI_THREAD
 int mutex_error = pthread_mutex_lock(&karte_t::step_passengers_and_mail_mutex);
 assert(mutex_error == 0);
 (void)mutex_error;
#endif
 dbg->error("simworld.cc", "Incorrect destination type detected");
#ifdef MULTI_THREAD
 mutex_error = pthread_mutex_unlock(&karte_t::step_passengers_and_mail_mutex);
 assert(mutex_error == 0);
#endif
 };

 if(time_per_tile < UINT32_MAX_VALUE)
 {
 // *Hundredths* of minutes used here for per tile times for accuracy.
 // Convert to tenths, but only after multiplying to preserve accuracy.
 // Use a uint32 intermediary to avoid overflow.
 const uint32 car_mins = (time_per_tile * straight_line_distance) / 10;
 car_minutes = car_mins + 30; // Add three minutes to represent 1:30m parking time at each end.

 // Now, adjust the timings for congestion (this is already taken into account if the route was
 // calculated using the route finder; note that journeys inside cities are not calculated using
 // the route finder).
#ifndef FORBID_CONGESTION_EFFECTS
 if(settings.get_assume_everywhere_connected_by_road() || (current_destination.type == town && current_destination.building->get_stadt() == city))
 {
 // Congestion here is assumed to be on the percentage basis: i.e. the percentage of extra time that
 // a journey takes owing to congestion. This is the measure used by the TomTom congestion index,
 // compiled by the satellite navigation company of that name, which provides useful research data.
 // See: http://www.tomtom.com/lib/doc/trafficindex/2013-0129-TomTom%20Congestion-Index-2012Q3europe-km.pdf

 //Average congestion of origin and destination towns.
 uint16 congestion_total;
 if(current_destination.building->get_stadt() != NULL && current_destination.building->get_stadt() != city)
 {
 // Destination type is town and the destination town object can be found.
 congestion_total = (city->get_congestion() + current_destination.building->get_stadt()->get_congestion()) / 2;
 }
 else
 {
 congestion_total = city->get_congestion();
 }

 const uint32 congestion_extra_minutes = (car_minutes * congestion_total) / 100;

 car_minutes += congestion_extra_minutes;
 }
#endif
 }
 }

 // Cannot be <=, as mail has a tolerance of UINT32_MAX_VALUE, which is used as the car_minutes when
 // a private car journey is not possible.
 if(car_minutes < tolerance)
 {
 const uint32 private_car_chance = simrand(100, "void stadt_t::generate_passengers_and_mail() (private car distribution_weight?)");

 if(route_status != public_transport)
 {
 // The passengers can get to their destination by car but not by public transport.
 // Therefore, they will always use their car unless it is faster to walk and they
 // are not people who always prefer to use the car.
 if(car_minutes > walking_time && can_walk && walking_time <= walking_time_preference_threshold && private_car_chance > settings.get_always_prefer_car_percent())
 {
 // If walking is faster than taking the car, and the walking time is below passengers'
 // walking time preference threshold, passengers will walk.
 route_status = on_foot;
 }
 else
 {
 route_status = private_car;
 }
 }
 else if(private_car_chance <= settings.get_always_prefer_car_percent() || car_minutes <= best_journey_time)
 {
 route_status = private_car;
 }
 }
 else if(car_minutes != UINT32_MAX_VALUE)
 {
 route_status = too_slow;

 if(!too_slow_already_set && !overcrowded_already_set)
 {
 best_bad_destination = destination_pos;
 // too_slow_already_set = true;
 // Do not set too_slow_already_set here, as will
 // prevent the passengers showing up in a "too slow"
 // graph on a subsequent station/stop.
 }
 }

 if((route_status == no_route || route_status == too_slow || route_status == overcrowded || route_status == destination_unavailable) && n < destination_count + extend_count - 1)
 {
 // Do not get a new destination if there is a good status,
 // or if this is the last destination to be assigned,
 // or else entirely the wrong information will be recorded
 // below!
 current_destination = find_destination(trip, pax.get_class());
 }

neroden

So that's the explanation for why extend_count makes sense. 

This is on the critical path for program speed.  Adding extra runs through the entire pathfinding code is expensive.  But adding extra runs through the shortcut path is cheap.

It is probably worth timing the difference between jacking up the base commuter count (by a factor of at least 10,000, probably more) and using extend_count to jack it up only when needed.

jamespetts

Thank you for your reply. I should note that I have now removed extend_count entirely as discussed above.

I should also note that the skip_route_checks code path was specifically and carefully designed to make sure that it had absolutely no effect on the actual in-game outcomes of the passenger generation. It was specifically designed to do precisely the same thing as the code that preceded it, just faster. It was never intended to be part of the economic simulation and has not been written in a way that would be suitable for this as it does not in fact represent anything that people would ever take into account when making decisions.

Also, if I have understood correctly, what your code does is that, if we are on a code path that does not involve a further route search for the particular iteration, it ignores that iteration for the purposes of counting towards alternative destinations. This means that the code will, in fact, go through more iterations than it would otherwise go through that do call upon the full routing database, so I do not see how this makes the code any faster than simply increasing the number of alternative destinations.

For example, with the current algorithm, suppose that we have four destinations in total. For each of the iterations, we may have:

1 - building not available (no database search);
2 - reachable but too slow (database search);
3 - building not within range of transport, too far to walk (no database search); and
4 - reachable but too slow (database search).

This is two database searches for these iterations. Suppose that we do not increment the count in the cases that you describe. Instead we would have:

1 - building not available (no database search);
1 - reachable but too slow (database search);
2 - building not within range of transport, too far to walk (no database search);
3 - reachable but too slow (database search);
4 - building not reachable (no database search);
4 - reachable but too slow (database search).


In this example, we have one extra database search because the iterations did not in fact end at what was originally number 4 because two of the iterations did not increase the counter. This is the same result as if we had simply increased the number 4 to 6.

In any event, I do not think that it is sensible to implement a mechanic that adds anomalies and takes the simulation further from reality than it is at present: that really makes no sense in a realism-oriented game. I do not think that it is necessary to do so in order to engage with and address underlying balance issues: quite the converse, in fact.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

neroden

OK, I can tell you why the change you made to simuconf.tab won't work to solve the problem: it's just not nearly large enough.

When you have an isolated village and you're trying to get the people in the village to find the factory with jobs in that village, which is probably less than 1/1500 of the number of factories on the whole map -- and there are also hundreds of "industrial" city buildings -- so it's probably less than 1/3000 of the number of job buildings on the whole map -- then you need to look for at least 3,000 destinations.  Specified in millionths, that means something like 3,000,000,000.  So a range from 1 to 5 billion attempts should be enough to find the job we're looking for.  I'm going to try it.

Pak patch:
https://github.com/jamespetts/simutrans-pak128.britain/pull/155

If this is fast enough, then... OK.  But it might make more sense to simply set the commute attempts to effective infinity.

Alternative code patch, revised to apply to current master, probably faster in late game (but go ahead, test it, maybe it just doesn't matter):
https://github.com/jamespetts/simutrans-extended/pull/615/

You may not see why this makes the code faster than increasing the number of alternative destinations.  It does.  It does so in the cases where things were working OK before.  Suppose that we are on a code path which DOES involve a further route search for the particular destination.  If we jack up the number of alternative destinations.... we go through another code path which does involve a further route search.  We try again.

My implementation limits the number of routing database searches to the "alternative destinations" count *while* solving the problem even if the alternative destinations count is low (and it may be low for performance reasons).  The current implementation limits the number of routing database searches to the "alternative destinations" count, period.  By jacking up the alternative destinations count we do more routing database searches.

Effectively, my implementation eliminates the extra database searches for the *OTHER* cases, the cases where it isn't strictly necessary to make the gameplay function.  But it is a compromise from realism.  If it's not necessary for performance, great.

If jacking up the destination count number to suitable size runs fast enough, I'm actually happy to do that.

Test it out with the 1,000,000,000 - 5,000,000,000 number on Bridgewater-Brunel.  If it becomes unacceptably slow, well, come back here and I'll show you how to make it fast enough.

I can tell you right now 12,000 isn't going to do anything. (I tested that already.  It's not high enough.) 5,000,000 helped some but didn't solve the problem.  The number needs to be HIGH.

---- Added thought:
Everyone has fast 64-bit computers now, unlike back when the extend_count code was written, so it's possible the performance optimization just isn't needed any more and the pak setting will Just Work. 

If you do turn out to need the performance gain, however, I have another proposal: we can do essentially the same thing my code is doing but write it differently so that it's clear that we're doing a performance-based hack, with a setting like "maximum database calls per routing attempt", so that we have the world-modelling number separated clearly from the performance intervention to prevent the game from crawling.  I think that would make sense and I know how to do it.

Let's try the clean implementation though.  1 to 5 billion tries per millionth.

neroden

While we're here I would like to remind you of the patch to make the fishing port into a "shore_city", which now works since my code for factory placement is now merged.  https://github.com/jamespetts/simutrans-pak128.britain/pull/148 

This should improve fishing port placement vastly (at least, it does in my testing, I'm playing with it active).  This was part of a project you did a long time ago which you didn't manage to finish because of the buggy code which I just fixed.  You may wish to make other factories into shore_city and river_city placement too, but I was sure about the fishing port.

This matters because it substantially reduces the chance of generating a fishing port way outside a city (which can never get enough staff in early game) or one which isn't on the water (which is completely unusable without major terraforming).

neroden

Just one thing.  I sat down to play a game.  I started with early game freight in 1750, as I like to do.  It was broken and unplayable in multiple ways.

I have been doing essentially nothing but bugfixes (only edge-avoidance is really a feature).  I am professionally offended by saying that this code (pak settings are also code) was "working as designed".  It was not.  The design is for early game freight to be playable.  These are bugs, not "working as designed":
-- if it's impossible for a factory in a village to get its needed commuters due to commuters not trying hard enough to find jobs, which it was, it breaks early-game freight
-- if fishing ports are being misplaced so they're not on the shore, or not in a city so it can't get commuters, it breaks early-game freight (thanks for merging the code fix, hope the pak fix will follow soon)
-- if factory chains for markets are never built, it breaks early-game freight (thanks for merging the fix for this)
-- if markets demand so many commuters they always have staff shortages, it breaks early-game freight (thanks for merging the fix for this)
-- if factories have buffers lower than a truckload, then it usually makes it impossible to satisfy production demand in an economically reasonable way in early-game freight (thanks for merging the slaughterhouse fix, I know you want to improve the general minimum-capacity fix before merging and please do so)

I understand the desire for the perfect patch, but I'm just trying to get the game into a playable state  for early-game freight right now.  I like debugging, but I would rather actually be playing the game; it was not my plan to do a bunch of coding right now.  And that's why I was professionally offended, as someone who has written programs to specific design criteria, by hearing a bug-ridden state described as "working as designed".

The current behavior is a bug.  It is not working as designed.  The *design* was to make early-game freight playable.  It was not playable.  Bugs happen, that's fine, but please don't describe them as "working as designed".

Rant over.

jamespetts

Thank you for that. First of all, yes, I had forgotten about the "shore_city" setting. I have now merged that patch: that is most helpful.

As to the calibration of the number of alternative destinations: the numbers that you gave appear to be based on the belief that the min_alternative_destinations_per_job_millionths is just millionths of the alternative destinations figure (in that you suggested a figure of three billion in order to ensure that the passenger destination algorithm will search three thousand buildings), but this is not what this number represents. The algorithm is number of alternative destinations = (a random value between min_alternative_destinations_per_job_millionths and max_alternative_destinations_per_job_millionths) * total number of jobs on the map / 1,000,000. The part in bold I think is the part that you may have omitted. Thus, the 3,000 * 1,000,000 = 3,000,000,000 would only be the correct calculation if the number of jobs per building were 1. In fact, you would need to divide the 3,000,000,000 figure by the average number of jobs per building in order to get a figure that does what I believe that you are aiming to do with this figure.

In terms of the question of whether the current calibration is broken, are you able to upload a saved game in which you can reliably reproduce this? As indicated earlier, I had tested this when generating maps at default settings and had not found this problem. If you have discovered a case where generating maps with other settings does give rise to problems, then I will need to look at this, but it is not practical for a single amateur spare time developer to test new features in all conditions.

I should also note that there is a very fundamental difference between saying, on the one hand, the algorithm in the code is broken, and, on the other, saying that the algorithm in the code when used with certain data in configuration files and certain other player selected parameters produces an unbalanced result that makes the game unplayable in that specific case. The latter does not entail the former, whether or not you can find a way of fixing the latter problem by altering the algorithm. That it is possible to fix the balance issue by amending the algorithm does not by itself mean that amending the algorithm is the optimum fix for the balance problem. That it is hypothetically possible that a fix for the balance problem that does not involve amending the algorithm would have a performance impact, which there is a non-zero chance would be an unacceptable performance impact, does not mean that the algorithm is broken, nor (without establishing that this is in fact the case) that there is a reason to attempt to solve the balance problem by amending the algorithm, especially if there is the potential for doing so to have adverse effects, such as causing the algorithm less to fulfil its core purpose of simulating a specific aspect of reality.

That I am not convinced by the arguments that you make for a particular solution does not mean that I do not accept that you have identified a genuine problem, although I will need a reproduction case to be able to test this properly in order that I can truly understand the parameters of this problem and thus how best to solve it without creating worse problems elsewhere.

In any event, thank you again for your work on this: it is much appreciated.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

neroden

I can reproduce this quite reliably on any map to the point where I am not going to even attempt to run a game without patching either the pak or the game for it.  It's not worth wasting my time or energy.

You're right about my calculation error, which is minor.  So if there are 5  jobs per industrial city building in the early years (and there are a lot of industrial city buildings, probably more than half the buildings with jobs), with similarly low numbers of jobs for farms, then I guess you can divide my 1 - 5 billion by 5 and set the numbers to min 200,000,000, max 1,000,000,000.  That should work.  This has been applied to the more-alternative-destinations branch for pak128.britain.

I'm glad you accept that there's a real problem.  I have proof that this is a recurring problem and I hope you can trust me rather than demanding that I generate a bunch of maps for you to prove what I already know and what I already spent weeks of frustration dealing with.

It is quite possible that just jacking up the numbers will work fine.  This is the clean solution, and I agree, it's preferable.  You want to go for realism first and only do performance-based compromises if necessary, and that's a good idea.

So please commit this patch for pak128.britain ASAP, because it is proven necessary to make the game playable, at least on my preferred large maps (4096x4096, ~1500 industries). 

I have been playing with the pak patch for high numbers and without the code patch for a while, and this *does* work.

https://github.com/jamespetts/simutrans-pak128.britain/pull/155

At least in the 1750 game it runs fast enough (I've played a bunch of 1750 games).  If it creates slowdowns later (which I suspect, but I could be wrong or they could easily be minor enough to be unimportant), we'll find out about them later and can deal with them later.

neroden

Quote from: neroden on May 08, 2023, 02:20:35 PMAgree 100%.  I simply haven't had the bandwidth to modify my patch to replace "7" with something which reads those new simuconf.tab settings.

OK.  So on this topic (the other patch which has to go in to make the early freight game playable), I have a couple of things:

(1) a patch for pak128.britain.  This doesn't actually do anything until the code patch goes in, but should be safe to commit.  (Note that I initially got the name of the config option wrong, but that's fixed now -- there should be two commits in this pull request)

https://github.com/jamespetts/simutrans-pak128.britain/pull/157

(2) an updated patch for the code.  This checks the settings from simuconf.tab.  (And it is now working, once I fixed my naming error in the pak patch.)

https://github.com/jamespetts/simutrans-extended/pull/611

What I have not done here is to store the settings in the savegame file.  I would really rather not store the settings in the savegame file; this is the sort of workaround where a newly loaded savegame should be updated whenever the pak is updated.  If that's OK, I think this should work.  If there's some sort of network sync issue (which there shouldn't be, because everyone should need to have the same version of the pak, right? -- but I don't know) then I'm going to have to leave it to someone else to patch it into the savegame.  I've always had trouble with stuff which involves patching new data into the savegame.

jamespetts

Thank you for that. I do not need you to generate a large number of maps - if you can just upload one map where you can reproduce the problem reliably (even one that you have already generated), that would be very helpful. Even some map generation settings from which you can reproduce the problem reliably would help. I really do need to be able to see the problem myself so that I can see exactly how it manifests and what affects it in order to be able to conceptualise the right kind of solution and calibrate it properly.

In terms of simuconf.tab settings, it is necessary to have these saved with the saved games for network sync purposes: although players should have the same pakset version, players may have customised their simuconf.tab file, so it is always necessary to save any simuconf.tab setting that affects the gamestate in any way.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

neroden

Great.  Then I'm going to ask you to figure out how to save the simuconf.tab settings in the savefile.  This has always been an area of the code I have a lot of trouble with, and it'll be easier for YOU to do it.  For single-player purposes, it's far better to not have it in the savefile so that it's overwritten by a new version of the pak.

In the long run, we really need to separate pakset-specific settings from settings designed to be overwritten by players, which would eliminate this mess, but this is a very hairy problem since they've been totally entangled for decades.

The game is playable (in single-player) with my patch and unplayable without it. If you could make the effort to get my "make the game playable" patch in by adding what you feel you need for network play, I would appreciate it.  I don't have the time.  I will play my "it actually works" version; I do not have the time this year to spend on adding things you want, which do not benefit me, to the patch.  Please add those yourself.  Thank you in advance for doing the work.

I will probably push a branch on my repo called "playable-version" or something for other people who have the same problem.  I'm getting irritated, if you can't tell.

neroden

One of the savegames proving the staff-shortage problem linked at https://nerode.org/temporary/

If it doesn't load, well, that's version incompatbility due to the varioous updates which have happened.  I don't have time to generate a new map.

jamespetts

Thank you very much for that: I have downloaded the map and confirm that I can open it with the latest build. I will investigate this when I have some time.

As to saving new settings, the procedure is as follows.

  • Add the datum for the new setting in settings.cc and the code for initialising the default value and reading/writing this from simuconf.tab (I assume that you know how to do this, so I have not explained this in detail).
  • In simversion.h, increment the EX_SAVE_MINOR by 1.
  • In settings_stats.cc, ensure that static const char *revision_ex[] includes the character sequence for a number at least this high, and, if not add it. This allows the manual selection of this version as a save version in the advanced settings dialogue. (This step is often forgotten).
  • Add a line for reading/writing the value, conditional on the saved game version being at least or greater than the current combination of major and minor save version, in void settings_t::rdwr(loadsave_t *file). For example, if the current major version is 14 and the current save version is 61 (and you have thus just incremented the value to 62 for your new addition), you will need to add the line reading/writing your new datum inside a code block for the following if statement: if(file->is_version_ex_atleast(15,62)). Note that the is_version_ex_atleast() method is relatively new. A lot of code is written in the older style of if (file->get_extended_version() >= 15 || (file->get_extended_version() == 14  && file->get_extended_revision() >= 62). The line for reading/writing the value itself depends on the value type. For example, if the datum were of a uint32 type, then the code would be: file->rdwr_long(new_value);.
  • Add the new setting to the advanced settings dialogue so that it can be changed in-game. Do this in settings_stats.cc. The easiest thing to do here is just copy the code for a similar type of data.
  • Add the getters and setters: I assume that you know how to do this.

A good way of checking that you have added all of the necessary code is just to use your IDE to search for all instances of a pre-existing datum. Once you have ignored all of the instances that are specific to the data type (and these will be few, if any, as the getters and setters will normally be used for actual running code), what remains should be what you need to add for the new value. Note that this only works for step 4 and onwards above.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

jamespetts

I have now had the chance to carry out a controlled test of this. I took the saved game, which had the default values, and, using the advanced settings dialogue, modified it in one instance of the save to match Nathaneal's suggested modified values (fixed so that the minimum and maximum were no longer inverted). Note that I had to make a small change to the advanced settings dialogue to allow such large numbers to be inserted manually.

The results are as follows.


The results do show differences between the two, albeit less dramatic than Nathaneal's posts might suggest. At the end of the 6 years, the actual industries served by the transport network had a roughly similar level of staffing, but a number of smaller towns had much better staffing levels in churches and town halls than previously, and correspondingly better commuter success rates for passengers. Overall, staffing rates were higher in a number of places. However, there were still many places where staffing rates were low because there were insufficient workers of the correct level of wealth within range and other places where commuting success rates were low because there were insufficient jobs of the correct level of wealth within range.

I have not yet had a chance to test whether such higher numbers have a substantial impact on performance. Subject to such testing, and subject to any feedback from this result, I am minded to commit the changes to commuting trip alternative destinations that I had implemented in this test scenario. I should be grateful for any feedback on this.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

neroden

Well, that's a pretty good outcome -- one does want staff shortages to happen in the "economically realistic" situations,  just not because everyone is turning down the job at their doorstep.  Better behavior in small poorly served towns is exactly what I was going for, it really helps the early game.