Station coverage: a proposal

[neroden]

Two more thoughts.  I believe passengers have a "just walk across town, don't take public transport or drive, walking is faster" option?  Is that still implemented?

The same should be true of freight.  I've ended up with a Pub only 4 squares from a Brewery, and similar things happen quite often.  The infrastructure for trucking is overkill here and takes up too much space.  For such short trips, "private freight transport" should be possible.  (Note that at the current scale of 125 meters/tile, 1 km/hr would only be a half hour trip in the example I gave.)

Also of note: the ability for passengers to *change stations* by walking seems to have gone away at some point; walking only seems to be working at origin and destination.  Changing stations on foot may be too stressful on the pathfinder, so I understand if it was removed for that reason.

[Junna]

Two more thoughts.  I believe passengers have a "just walk across town, don't take public transport or drive, walking is faster" option?  Is that still implemented?

The same should be true of freight.  I've ended up with a Pub only 4 squares from a Brewery, and similar things happen quite often.  The infrastructure for trucking is overkill here and takes up too much space.  For such short trips, "private freight transport" should be possible.  (Note that at the current scale of 125 meters/tile, 1 km/hr would only be a half hour trip in the example I gave.)

This works in standard, I think it was changed at some point in experimental.

[neroden]

Playing my 1750 sample game again, freight coverage needs to be at least 3 tiles and should really be 4 tiles.  This is due to the fairly frequent need to jam in roads, railways, and canals around hills and fields.  With a coverage of less than 4 the earthworks become monumental.

The 1 km/h penalty for travel to a not-directly-adjacent station still sounds good.

[jamespetts]

Nathaneal - the passengers do indeed walk accross town (and, indeed, even between towns) if walking is faster and the journey is within the passengers' journey time tolerance; this is a feature exclusive to Experimental. In Standard, passengers walk if and only if their origin and destination are within the coverage radius of a single halt.

Can you give me some examples of the monumental earthworks to which you refer? The reality as I understand it is that freight depots are usually more or less next to the industries that they serve.

[neroden]

Nathaneal - the passengers do indeed walk accross town (and, indeed, even between towns) if walking is faster and the journey is within the passengers' journey time tolerance; this is a feature exclusive to Experimental. In Standard, passengers walk if and only if their origin and destination are within the coverage radius of a single halt.

Can you give me some examples of the monumental earthworks to which you refer? The reality as I understand it is that freight depots are usually more or less next to the industries that they serve.

In Simutrans, farms frequently place themselves on top of a hill (often 2-3 tiles of hill) and surround themselves with fields (usually 1-2 tiles depth).  Good luck building a directly adjacent freight depot.

In real life, this situation often features a depot at the bottom of the hill, with the farmers walking their produce to the depot.  Now, there are some oddities related to the two scales of simutrans (the real hill would be the size of the one on the visual scale, not the one on the 'virtual' scale of 125 m / tile).  But you have to make some compromises in design due to that.

[neroden]

Nathaneal - the passengers do indeed walk accross town (and, indeed, even between towns) if walking is faster and the journey is within the passengers' journey time tolerance; this is a feature exclusive to Experimental.

Do it with freight too.  Just make the freight really really slow (1 km/hr would be fine).

[jamespetts]

Right - I have now fixed the original problem. Thank you both for pointing it out. I have also increased the station coverage for factories from 1 to 3 in the simuconf.tab file for Pak128.Britain-Ex and removed the relationship between this figure and ship docking, making that a fixed figure of 1 tile.

Also, I forgot to respond to this earlier:

Also of note: the ability for passengers to *change stations* by walking seems to have gone away at some point; walking only seems to be working at origin and destination.  Changing stations on foot may be too stressful on the pathfinder, so I understand if it was removed for that reason.

This still exists, but it is disabled by default: look for the simuconf.tab setting: allow_routing_on_foot = 0 and change it to 1. This can also be changed in the in-game advanced settings GUI (key "i" in Pak128.Britain-Ex).

This was originally disabled after Knightly pointed out a number of discrepancies that this created in the day when passengers could not walk long distances to their final destination - I wonder whether it would be wise to enable it by default now?

I am about to look into adding transshipment times.

Edit: Transshipment times added, at a presumed speed of 1km/h, to the Github repository. Note that this only works for the origin stop - because of the way in which goods are distributed amongst routes, it would be difficult to take into account the timing at the destination stop. This should suffice for the present, however.

Edit 2: Actually, looking again at the code, it is indeed possible to add transshipment time to the destination stop, which I have just added to my Github branch.

[neroden]

This still exists, but it is disabled by default: look for the simuconf.tab setting: allow_routing_on_foot = 0 and change it to 1. This can also be changed in the in-game advanced settings GUI (key "i" in Pak128.Britain-Ex).

This was originally disabled after Knightly pointed out a number of discrepancies that this created in the day when passengers could not walk long distances to their final destination - I wonder whether it would be wise to enable it by default now?

Huh.  Yeah, it would have caused discrepancies when people couldn't walk to their final destination.

Yeah, it probably should be enabled by default now that you've fixed that.  :-)  Make sure that the same transfer time penalty for walking between stations applies as for walking to final destinations or from origins, though, or the same discrepancies will come back.

Thanks for everything you've done on this -- you clearly know this part of the code a lot better than me. I can see what to do from a design point of view, but can't find where to program it.

--- added information --
Note that you want to modify the code to allow freight to "walk" as well as passengers (but at 1 km/h).  That isn't enabled in the current code.  The time delay for transferring should take care of all the "oddities" in both

I just tested your most recent version. 

First thing to know: The separate station coverage for freight is STILL not working right.  Told you it was going to be hard.

I have a setup with a Grain Mill, and 4 tiles away a Bakery.  There is a station directly next to the Grain Mill.  There is also a station *6* tiles away from the Bakery.  I have a boat line running from the Grain Mill station to the station *6* tiles away from the Bakery.

Right now, freight will happily queue up at the origin station, happily be transported to the destination station, and then... vanish!  It doesn't arrive at the Bakery.  However, it happily waltzes around getting transported by my ship before vanishing into thin air.

You haven't found all the places where the coverage distance is used.

Second thing to know: There's some new sort of breakage in freight generation (which may be responsible for the aforementioned problem).  The Grain Mill is now generating freight for Bakery locations which are thousands of squares away and completely disconnected from the transportation network.  (On different islands, in fact, so they couldn't even go by private car, even if that were implemented, which it isn't).

This means that even the passenger coverage distance isn't being respected right now.  We wanted freight to be able to "walk" short distances... not cross-country.

My origin station is 112, 743 and I'm getting flour generated with a destination of 1150, 816 -- 64 km away, and 64 hours at 1 km/h.  Hmm?

I think you have to dig into the path-search code which delivers factory products to make sure that it doesn't try to send products across routes which will turn out to be impossible or beyond the time tolerances.  (This may also be an issue with passenger and mail generation code, I haven't made a test case yet.)

(If you can't get the distance based coverage working, it's also possible that this could be handled by setting a time tolerance for freight in the freight generation code, but that may be harder.)

[neroden]

Doing a little debugging.

This patch is no good, because it triggers at the very end.  That's too late, and it's generating the nasty bug I just described, with disappearing goods.  You have to back this patch out.

You have to check whether the stop is within range of the factory much, MUCH earlier, during pathfinding.

Code Select Expand


@@ -1612,8 +1612,17 @@ void haltestelle_t::add_pax_no_route(int n)

void haltestelle_t::liefere_an_fabrik(const ware_t& ware) const //"deliver to the factory" (Google)
{
- fabrik_t *const factory = fabrik_t::get_fab( welt, ware.get_zielpos() );
- if(  factory  ) {
+ fabrik_t *const factory = fabrik_t::get_fab(welt, ware.get_zielpos());
+ if(factory)
+ {
+   if(ware.is_freight())
+   {
+     // Check to see whether this is within range.
+     if(!fab_list.is_contained(factory))
+     {
+       return;
+     }
+   }
    factory->liefere_an(ware.get_besch(), ware.menge);
  }
}


Specifically, you want to look at the part where stations decide what factories are within range (for destinations) and where factories decide what stations are within range for origins.  Don't mess with liefere_an.

[neroden]

Look, I did some further research.  You simply didn't implement the different freight station coverage distance at all.

The code for station coverage is in simplan.h and simplan.cc.  Mostly in the planquadrat_t code.

If you really want separate freight coverage, you need  to have a separate freight_halt_list distinct from the halt_list for passengers.  You need to duplicate all the halt_list management code in plan_quadrat_t.  Then you need to go find every single place where the halt_list is referenced (which is all OVER the code) and separate them out into freight and passenger uses.

Particularly path_explorer.cc and path_explorer.h

But also
bauer/fabrikbauer.cc
dings/gebaeude.cc (!!!)
simcity.cc
simhalt.cc

I do *not* think it is worth the work involved.

I *strongly* advise ripping out all the code related to a separate station coverage size for freight, since you never actually implemented the feature, and it really will involve ripping apart most of the codebase, including some of the most sensitive bits. 

But if you still want to implement freight_coverage radiuses, you now know where to start.  It will be weeks of work.

Implementing a time penalty for "walking" freight instead, which we're already doing, should be much easier to do and much easier to debug, since you've already managed to implement that for passengers; you just have to run through the code enabling it for freight.

[neroden]

Oh.  Another bug related to the walking code, while you're at it.  The passenger and mail walking time for attractions -- at least the one displayed in the info window -- is based on the top left hand corner.  It should be based on whichever corner of the attraction is closest.

I can't tell because it isn't displayed in the info window -- but I suspect factories, and likely all multi-tile buildings (headquarters, etc.) have the same problem.

This also needs to be addressed for the freight transport time, of course.

[jamespetts]

Thank you very much for testing this and looking into it. I do not have time to examine these things in detail now or act on them, but I shall give these matters due consideration in due course over the next few days when I get a few minutes.

(I should add that the main difficulty, which I think that you have found, with the separate freight coverage area, is that the coverage area is based on marking a number of tiles, and subsequently checking those tiles, rather than actual distance checking at each step).

Edit: One potential problem with allowing goods from factories to "walk" (be hand-hauled) to their destination some distance away is that that would substantially reduce the economic impact of players' transportation. For passengers, this is not an issue, as passengers have a journey time tolerance, so many will not travel at all if the journey is too long. However, this is not so for freight, so goods would in fact be shipped instantly all over the map if the same rules were used for freight as for passengers, which would mean that all factories would always be fully supplied. This would not be serviceable, I think.

[neroden]

Edit: One potential problem with allowing goods from factories to "walk" (be hand-hauled) to their destination some distance away is that that would substantially reduce the economic impact of players' transportation. For passengers, this is not an issue, as passengers have a journey time tolerance, so many will not travel at all if the journey is too long. However, this is not so for freight, so goods would in fact be shipped instantly all over the map if the same rules were used for freight as for passengers, which would mean that all factories would always be fully supplied. This would not be serviceable, I think.

(1) The freight would need a journey time tolerance as well.  If the "walking speed" of freight is slow, the journey time tolerance can be high and still prevent most cross-map freight transfers....
(2) There should be a walking distance limit for freight.  It would be perfectly acceptable if this limit were the same as the station coverage limit.  In fact, the limit for walking from a factory to a station should *already* be this low, for passengers *as well as* freight.  Isn't it?  If not, why not?  This should act as a computational cutoff: walking should not be considered beyond this distance.  I'm going to have to look at how you implemented the walking code.

(3) I do have a suggestion as to how to implement "walking distances" more efficiently.  Instead of simply storing a list of connected halts at each square on the map... store the halt *and* the walking distance to the halt (one walking distance for passengers, one for freight).  Then this only has to be updated when halts are built or deleted, in the same place where the halt_list is currently updated.  You don't have to make two halt_lists, and you don't have to recheck all the users of halt_list, only the parts measuring journey time.  It should be faster than computing walking distance for each trip...
(4) Finally, passenger and freight "direct" walking probably has to use separate code from walking between halts, which only looks within the coverage radius.  I'm not sure whether you did this.

(5) I just looked.  As far as I can tell, you haven't implemented walking from the halt to the final destination, or walking from the origin to the starting halt, either.  It's just cosmetic; it doesn't actually seem to be implemented.  If I'm wrong, point me to the actual code which you think does the work.  Yeesh!

(6) I am clearly going to have to program the walking code myself.   

[jamespetts]

Thank you for your reply (and for your fix to the disappearing freight issue). Some of these topics are somewhat fundamental, so need careful consideration, I think. I shall deal with your points in numbered order.

(1) I am not sure that a journey time tolerance for freight makes sense. Passengers are people who could be doing something else if they were not travelling. People will only travel if the time that they invest in getting to their destination is worth the reward that they obtain when they reach it. Freight is different. Some goods might be perishable, in which case there is no point in dispatching them at all if they cannot reach their destinations in a certain time, but most goods are not. A factory that demands coal does not care one whit whether its coal left the mine an hour ago, a day ago, a week ago, a month ago or a year ago, provided that it has a steady supply of the stuff. In the days of canals, goods hauled about the country often took several weeks to reach their destinations. A week's journey time tolerance at 1km/h is 168km. On a 512x512 map at 125m/tile, there would be no point in connecting any factories at all: everything would get to where it is going within a week without a single road, canal or railway.

(2 and 5) The two are interrelated. The idea behind the walking system for passengers was to use the existing coverage radius (rather than have to check every single building tile on the map for every single halt, which would not perform well), but just add the time that passengers spend walking from their origin to the origin stop and the destination stop to the destination to the journey time calculations. This way, the walking feature could be implemented with minimal changes to the code and minimal impact on performance. To make this feature work correctly, we needed to increase the coverage area for passengers from 3 or 4 tiles to 16-24 or so, which in turn prompted the idea of having a separate freight coverage radius, as cargo loading points are, with good reason, almost inevitably right next to factories, rather than many kilometres away.

The code for the walking/freight haulage time from the destination stop to the ultimate destination is here:

Code Select Expand


uint16 haltestelle_t::find_route(minivec_tpl<halthandle_t> *ziel_list, ware_t &ware, const uint16 previous_journey_time, const koord destination_pos)
{
uint16 journey_time = previous_journey_time;

if(ziel_list->empty())
{
//no target station found
ware.set_ziel(halthandle_t());
ware.set_zwischenziel(halthandle_t());
return 65535;
}

// Now, find the best route from here.
// Added by : Knightly
// Adapted from : James' code
// Further adapted to incorporate walking
// times calculation: @jamespetts, January 2013

uint16 test_time;
halthandle_t test_transfer;

halthandle_t best_destination;
halthandle_t best_transfer;

const uint32 transfer_journey_time_factor = ((uint32)welt->get_settings().get_meters_per_tile() * 6) * 10;
const uint32 walking_time_divider = 100 * (uint32)welt->get_settings().get_walking_speed();
koord destination_stop_pos = destination_pos;

const uint8 ware_catg = ware.get_besch()->get_catg_index();
uint32 long_test_time;

for (uint8 i = 0; i < ziel_list->get_count(); i++)
{
path_explorer_t::get_catg_path_between(ware_catg, self, (*ziel_list)[i], test_time, test_transfer);

long_test_time = (uint32)test_time;

if(destination_pos != koord::invalid)
{
// Walking time is not relevant for freight.
if((*ziel_list)[i].is_bound())
{
destination_stop_pos = (*ziel_list)[i]->get_next_pos(destination_pos);
}

// Add the walking distance from the destination stop to the ultimate destination.
long_test_time += (shortest_distance(destination_stop_pos, destination_pos) * transfer_journey_time_factor) / walking_time_divider;
}

else if(ware.is_freight())
{
// For freight, we instead calculate a transshipment time based on a notional 1km/h dispersal speed.
if((*ziel_list)[i].is_bound())
{
destination_stop_pos = (*ziel_list)[i]->get_next_pos(ware.get_zielpos());
}

// Add the walking distance from the destination stop to the ultimate destination.
long_test_time += (shortest_distance(destination_stop_pos, ware.get_zielpos()) * transfer_journey_time_factor) / 100;
}

if(long_test_time > 65535)
{
test_time = 65535;
}
else
{
test_time = long_test_time;
}

if(test_time < journey_time)
{
best_destination = (*ziel_list)[i];
journey_time = test_time;
best_transfer = test_transfer;
}
}

if(journey_time < previous_journey_time)
{
ware.set_ziel(best_destination);
ware.set_zwischenziel(best_transfer);
return journey_time;
}

return journey_time;
}


The code for the walking time for passengers from their ultimate origin to their origin stop is here:

Code Select Expand


while(route_status != public_transport && route_status != private_car && route_status != on_foot && current_destination < destination_count)
{
const uint32 straight_line_distance = shortest_distance(origin_pos, destinations[current_destination].location);
const uint16 walking_time = (straight_line_distance * walking_journey_time_factor) / 100u;
car_minutes = 65535;

const bool can_walk = walking_time <= (quasi_tolerance / walking_tolerance_divider);

if(!has_private_car && !can_walk && start_halts.empty())
{
/**
* If the passengers have no private car, are not in reach of any public transport
* facilities and the journey is too long on foot, do not continue to check other things.
*/
current_destination ++;
continue;
}

// Dario: Check if there's a stop near destination
const planquadrat_t* dest_plan = welt->lookup(destinations[current_destination].location);
const halthandle_t* dest_list = dest_plan->get_haltlist();

// Knightly : we can avoid duplicated efforts by building destination halt list here at the same time

// 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.


for (int h = dest_plan->get_haltlist_count() - 1; h >= 0; h--)
{
halthandle_t halt = dest_list[h];
if (halt->is_enabled(wtyp))
{
destination_list[current_destination].append(halt);
}
}

uint16 best_journey_time = 65535;

if(start_halts.get_count() == 1 && destination_list[current_destination].get_count() == 1 && start_halts[0] == destination_list[current_destination].get_element(0))
{
/** There is no public transport route, as the only stop
* for the origin is also the only stop for the desintation.
*/
start_halt = start_halts[0];
}
else
{
// Check whether public transport can be used.
// Journey start information needs to be added later.
pax.reset();
pax.set_zielpos(destinations[current_destination].location);
pax.menge = pax_left_to_do;
pax.to_factory = ( destinations[current_destination].factory_entry ? 1 : 0 );
//"Menge" = volume (Google)

// Search for a route using public transport.

uint8 best_start_halt = 0;
uint32 current_journey_time;
koord destination_stop_pos = destinations[current_destination].location;

ITERATE(start_halts, i)
{
halthandle_t current_halt = start_halts[i];

current_journey_time = current_halt->find_route(&destination_list[current_destination], pax, best_journey_time, destinations[current_destination].location);

// Add walking time from the origin to the origin stop.
// Note that the walking time to the destination stop is already added by find_route.
current_journey_time += (shortest_distance(start_halts[i]->get_next_pos(origin_pos), origin_pos) * walking_journey_time_factor) / 100u;
if(current_journey_time > 65535)
{
current_journey_time = 65535;
}
// 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)
{
best_journey_time = current_journey_time;
best_start_halt = i;
}
if(pax.get_ziel().is_bound())
{
route_status = public_transport;
}
}

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

if(best_journey_time > walking_time && can_walk)
{
// If walking is faster than public transport, passengers will walk.
route_status = on_foot;
}

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

if(route_status == public_transport && best_journey_time >= tolerance)
{
route_status = too_slow;

if(!too_slow_already_set)
{
best_bad_destination = destinations[current_destination].location;
best_bad_start_halt = best_start_halt;
too_slow_already_set = true;
}
}
else
{
// All passengers will use the quickest route.
if(start_halts.get_count() > 0)
{
start_halt = start_halts[best_start_halt];
}
}
}

INT_CHECK("simcity.cc 3333");

if(has_private_car)
{
// time_per_tile here is in 100ths of minutes per tile.
// 1/100th of a minute per tile = km/h * 6.
uint16 time_per_tile = 65535;
switch(destinations[current_destination].type)
{
case 1:
//Town
time_per_tile = check_road_connexion_to(destinations[current_destination].object.town);
break;
case FACTORY_PAX:
time_per_tile = check_road_connexion_to(destinations[current_destination].object.industry);
break;
case TOURIST_PAX:
time_per_tile = check_road_connexion_to(destinations[current_destination].object.attraction);
break;
default:
//Some error - this should not be reached.
dbg->error("simcity.cc", "Incorrect destination type detected");
};

if(time_per_tile < 65535)
{
// *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 > 0 ? car_mins : 1;

// 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).

if(s.get_assume_everywhere_connected_by_road() || destinations[current_destination].object.town == this)
{
// 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/congestionindex/2012-0704-TomTom%20Congestion-index-2012Q1europe-mi.pdf

//Average congestion of origin and destination towns.
uint16 congestion_total;
if(destinations[current_destination].type == 1 && destinations[current_destination].object.town != NULL && destinations[current_destination].object.town != this)
{
// Destination type is town and the destination town object can be found.
congestion_total = (city_history_month[0][HIST_CONGESTION] + destinations[current_destination].object.town->get_congestion()) / 2;
}
else
{
congestion_total = city_history_month[0][HIST_CONGESTION];
}

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

car_minutes += congestion_extra_minutes;
}
}
}

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

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 && private_car_chance > always_prefer_car_percent)
{
// If walking is faster than taking the car, passengers will walk.
route_status = on_foot;
}
else
{
route_status = private_car;
}
}

else if(private_car_chance <= always_prefer_car_percent || car_minutes <= best_journey_time)
{
route_status = private_car;
}
}

INT_CHECK("simcity 3419");
if(route_status == no_route || route_status == too_slow)
{
// Do not increment the counter if there is a good status,
// or else entirely the wrong information will be recorded
// below!
current_destination ++;
}
} // While loop (route_status)

bool set_return_trip = false;
stadt_t* destination_town;

switch(route_status)
{
case public_transport:

if(destinations[current_destination].factory_entry)
{
register_factory_passenger_generation(&pax_left_to_do, wtyp, target_factories, destinations[current_destination].factory_entry);
destinations[current_destination].factory_entry->factory->book_stat( pax.menge, ( wtyp==warenbauer_t::passagiere ? FAB_PAX_DEPARTED : FAB_MAIL_DEPARTED ) );
}
pax.arrival_time = welt->get_zeit_ms();
pax.set_origin(start_halt);
start_halt->starte_mit_route(pax);
start_halt->unload_repeat_counter = 0;
merke_passagier_ziel(destinations[current_destination].location, COL_YELLOW);
set_return_trip = will_return != no_return;
// create pedestrians in the near area?
if (s.get_random_pedestrians() && wtyp == warenbauer_t::passagiere)
{
haltestelle_t::erzeuge_fussgaenger(welt, origin_pos_3d, pax_left_to_do);
}
// We cannot do this on arrival, as the ware packets do not remember their origin building.
if(wtyp != warenbauer_t::post)
{
// The generated passengers were *4 above to allow for this discrimination by preference of
// destination. 1st choice (current_destination == 0): 100% - 2nd choice: 75%; 3rd or subsequent
// choice: 50%.
const int multiplier = current_destination == 0 ? 4 : current_destination == 1 ? 3 : 2;
if(range == local)
{
gb->add_passengers_succeeded_local(pax_left_to_do * multiplier);
}
else
{
gb->add_passengers_succeeded_non_local(pax_left_to_do * multiplier);
}
}
break;

case private_car:

if(destinations[current_destination].factory_entry)
{
register_factory_passenger_generation(&pax_left_to_do, wtyp, target_factories, destinations[current_destination].factory_entry);
destinations[current_destination].factory_entry->factory->book_stat( pax.menge, ( wtyp==warenbauer_t::passagiere ? FAB_PAX_DEPARTED : FAB_MAIL_DEPARTED ) );
}
destination_town = destinations[current_destination].type == 1 ? destinations[current_destination].object.town : NULL;
set_private_car_trip(num_pax, destination_town);
merke_passagier_ziel(destinations[current_destination].location, COL_TURQUOISE);
#ifdef DESTINATION_CITYCARS
erzeuge_verkehrsteilnehmer(origin_pos, car_minutes, destinations[current_destination].location);
#endif
set_return_trip = will_return != no_return;
if(wtyp != warenbauer_t::post)
{
// The generated passengers were *4 above to allow for this discrimination by preference of
// destination. 1st choice (current_destination == 0): 100% - 2nd choice: 75%; 3rd or subsequent
// choice: 50%.
const int multiplier = current_destination == 0 ? 4 : current_destination == 1 ? 3 : 2;
if(range == local)
{
gb->add_passengers_succeeded_local(pax_left_to_do * multiplier);
}
else
{
gb->add_passengers_succeeded_non_local(pax_left_to_do * multiplier);
}
}
break;

case on_foot:

if(destinations[current_destination].factory_entry)
{
register_factory_passenger_generation(&pax_left_to_do, wtyp, target_factories, destinations[current_destination].factory_entry);
// workers who walk to the factory or customers who walk to the consumer store
destinations[current_destination].factory_entry->factory->book_stat( pax_left_to_do, ( wtyp==warenbauer_t::passagiere ? FAB_PAX_DEPARTED : FAB_MAIL_DEPARTED ) );
destinations[current_destination].factory_entry->factory->liefere_an(wtyp, pax_left_to_do);
}

// Walking passengers are not marked as "happy", as the player has not made them happy.

merke_passagier_ziel(destinations[current_destination].location, COL_DARK_YELLOW);
if (s.get_random_pedestrians() && wtyp == warenbauer_t::passagiere)
{
haltestelle_t::erzeuge_fussgaenger(welt, origin_pos_3d, pax_left_to_do);
}

if(wtyp == warenbauer_t::passagiere)
{
add_walking_passengers(pax_left_to_do);
}
set_return_trip = will_return != no_return;
if(wtyp != warenbauer_t::post)
{
// The generated passengers were *4 above to allow for this discrimination by preference of
// destination. 1st choice (current_destination == 0): 100% - 2nd choice: 75%; 3rd or subsequent
// choice: 50%.
const int multiplier = current_destination == 0 ? 4 : current_destination == 1 ? 3 : 2;
if(range == local)
{
gb->add_passengers_succeeded_local(pax_left_to_do * multiplier);
}
else
{
gb->add_passengers_succeeded_non_local(pax_left_to_do * multiplier);
}
}
break;

case too_slow:

merke_passagier_ziel(best_bad_destination, COL_LIGHT_PURPLE);

if(destinations[0].factory_entry)
{
register_factory_passenger_generation(&pax_left_to_do, wtyp, target_factories, destinations[0].factory_entry);
}

start_halt = start_halts[best_bad_start_halt];
if(start_halt.is_bound())
{
start_halt->add_pax_too_slow(pax_left_to_do);
}

break;

case no_route:

if(destinations[0].factory_entry)
{
register_factory_passenger_generation(&pax_left_to_do, wtyp, target_factories, destinations[0].factory_entry);
}

bool crowded_halts = false;
int destinations_checked;

if(start_halts.get_count() > 0)
{
/** Passengers/mail cannot reach their destination, but there are some stops in their locality.
* Record their inability to get where they are going at those local stops accordingly.
*/

start_halt = start_halts[best_bad_start_halt];
if(start_halt.is_bound())
{
start_halt->add_pax_no_route(pax_left_to_do);
}
}
else if(plan->get_haltlist_count() > 0)
{
/** The unhappy passengers will be added to any potential starting stops
  * that are crowded, and were therefore excluded from the initial search.
  * However, there might be no possible starting stop too.
  */

// Re-search for start halts, which must have been crowded, or else
// they would not have been excluded from the first search.
ware_t test_passengers(wtyp);
for(int h = plan->get_haltlist_count() - 1; h >= 0; h--)
{
destinations_checked = 0;
halthandle_t halt = halt_list[h];
for(; destinations_checked <= destination_count; destinations_checked ++)
{
// Only mark passengers as being unable to get to their destination due to crowded stops if the stops
// could actually have got the passengers to their destination if they were not crowded.
if(halt->is_enabled(wtyp) && halt->find_route(&destination_list[destinations_checked], test_passengers) < 65535)
{
halt->add_pax_unhappy(num_pax);
// Only show as being overcrowded if there are, in fact, potentially suitable but overcrowded stops.
crowded_halts = true;
break;
}
}
}
}

if(crowded_halts)
{
// If the passengers cannot get to a stop that they might reach because of
// overcrowding, and all other stops are no route, mark the specific destination
// unavailable to the passengers because of overcrowding.
merke_passagier_ziel(destinations[destinations_checked].location, COL_RED);
}

else
{
merke_passagier_ziel(destinations[0].location, COL_DARK_ORANGE);
}
};


(3) This is not a bad idea, actually - it would get rid of quite a few computations, including divisions, that are performed quite frequently. Would using a struct here be helpful, do you think (struct nearby_stop_t { halthandle_t halt; uint16 walking_distance; }?)

(4) This is indeed done for passengers in simcity.cc - I have not done anything for freight yet.

(6) It would be very kind if you could assist with this - your contributions are much appreciated. We just have to be sure that we get the economics/fundamentals right before starting any major work (especially the issue with hand haulage of freight, which is the trickiest issue, I think).

[neroden]

Sorry I dropped off the net again, real life interfered as it so often does.   

Thanks very much for pointing me to the code.  The destination to ultimate destination code in haltestelle_t looks OK.  I kind of whish it wasn't in haltestelle_t, however.

The ultimate orgin to origin stop code -- which file is that in?  Is it in simcity.cc?

Regarding the "code cleanup" ideas:

Code Select Expand

struct nearby_stop_t { halthandle_t halt; uint16 walking_distance; }
should, indeed do the trick.  That should clean up an awful lot of code.

The other thing to do is to stop duplicating this code:

Code Select Expand

const uint16 walking_time = (straight_line_distance * walking_journey_time_factor) / 100u;

Instead there should be an inline function, probably in simunits.h:

Code Select Expand

const uint16 walking_time_for_distance(uint32 distance) { return distance * walking_journey_time_factor) / 100u; };

And another one for the freight version of walking:

Code Select Expand

const uint16 hand_hauling_time_for_distance(uint32 distance)

If you make the functions inline it's just as fast as typing it out, but it makes it a lot easier to make changes if we need to mess around with data types or units.  Subroutines!  Subroutines!

[jamespetts]

Hello - thank you for your reply. I have been working on this recently, and have just pushed a change to 112.x-private-car-merge which should deal with the factory coverage radius issue. I have made the walking_distance a uint8 figure, as there is no need to have coverages of more than 255 tiles. The ultimate origin to origin stop code is indeed in simcity.cc.

Do the changes in my latest commit seem sensible to you?

[neroden]

Unfortunately, you're patching around some sort of fundamental problem in the code which you haven't found yet.

All the factory code is a bit of a mess, honestly, because it's very old.  There's a reason I wanted to rewrite it to connect factories directly, rather than through "koord" entries, because I think that will make it a lot clearer what's going on.  Then we might be able to find out what the actual game logic is.  Just poking around at different bits of it is not going to solve the problem.

[jamespetts]

I don't think that there is a single, fundamental undiscovered problem: the latest bug reports that you sent me by e-mail were both specific different problems, both of which I have now fixed.

However, I wonder whether the system of storing the distance to the halt in a struct inside the tile is reducing performance: there is a report here of reduced performance in the latest version compared to RC 11.9004, and this is the only feature so far as I can see that is capable of having a significant impact on performance. The call to check what halts are connected to a tile is made very, very many times in a step, and now it is returning a struct with a uint8 and a halthandle, rather than just a halthandle. Is this a realistically likely culprit, do you think? If so, what would the best strategies for dealing with it be?