I finally had time to sit down, pull your latest version of experimental, and play.
I've found a few bugs and fixed them. Please merge my "jp-devel-fixes" branch. The two bugs are:
(1) weight limits were not set correctly when the game built bridges; this has been fixed
(2) behavior of expanding the map with 0 added cities was VERY broken; it now behaves itself.
There are still some serious problems in the map generation code related to city generation; the algorithm is extremely slow when it should not be and it needs a rewrite. But at least now it doesn't build cities when it's been told not to.
I also found a bug, and made some tweaks, in pak128.britain-experimental. Please merge my new-experimental-fixes branch. This is a somewhat longer list of changes, but they 're all small.
(1) Raise the top speed of both shire horses (rail) to match that of road vehicles. This gives horse railways a chance of actually being useful; with all modes competing "equally" on speed, the very low speed limits crippled horse railways unrealistically.
(2) Add weight limits to bridges which didn't have them. The limits were selected somewhat arbitrarily based on position in the timeline and type of bridge.
(3) Add height limits to some road bridges which didn't have them, based on the limits on the same type of railway bridges. Rationalize some of the rail vs. road bridge height/length limit differences, by assuming the rail bridges are correct.
(4) Fix a bunch of typos which caused length limits to not be enforced (because "lenght" limits don't count...)
(5) Fix the Makefile to ship the sound (won't affect you since you use MSVC)
(6) Make the Georgian Mansion a city attraction rather than a countryside attraction. It looks a *lot* better that way, and also this causes it to actually appear in games started before its introduction date.
(7) Fix an error in the .dat file for the staging inn which caused two of the snow images to be swapped front to back.
I'm not really "back", I just had a week free. Hence the easy fixes only.
...by the way, I'm enjoying all of the new buildings.
The database of average travel times between stops is often wildly bogus for long multi-month ship trips, which means I really need to update my patch which directly measures trip time and averages it over passenger/freight count (whatever baroque thing you're doing instead, it isn't working).
Welcome back Nathanael! It's good to see you again.
Just a note, the Georgian Mansion is designed as a countryside attraction. That's not to say it might not work in a city but a building like that would typically be surrounded by open fields...
Thank you very much for your work - I have integrated your code related patches and pushed them. This is much appreciated. Please note that I am planning in any event to rewrite the city generation part of the game, although focussed on city growth in an ongoing game rather than the initial generation.
I shall revert later in relation to the issues concerning the pakset, which I have not had time to look into yet.
As to this:
Quote
The database of average travel times between stops is often wildly bogus for long multi-month ship trips, which means I really need to update my patch which directly measures trip time and averages it over passenger/freight count (whatever baroque thing you're doing instead, it isn't working).
Can you give me some more details as to this problem, and preferably a saved game? I think that your original patch to which you refer was written some time ago, before the point to point journey time system was implemented. That point to point journey system is quite fundamental to how things now work, and is greatly superior to a system based on average speeds for the whole line. If there are bugs in it they of course need to be fixed, but we really can't go back to the old and greatly inferior way of doing things now.
Quote from: jamespetts on April 06, 2013, 02:51:50 PM
Can you give me some more details as to this problem, and preferably a saved game? I think that your original patch to which you refer was written some time ago, before the point to point journey time system was implemented. That point to point journey system is quite fundamental to how things now work, and is greatly superior to a system based on average speeds for the whole line.
It certainly is greatly superior.
The trouble is that it doesn't seem to be creating average station-to-station times correctly when the average is well over a month. When an individual trip is finished, how is the data for that trip time recorded, and how is this used to create an average station-to-station time? That's the process which seems to be breaking for really long trips.
The relevant code is contained in convoi_t::laden, which I reproduce below. The most significant part in this connexion is contained in the do...while loop:
void convoi_t::laden() //"load" (Babelfish)
{
// Calculate average speed and journey time
// @author: jamespetts
halthandle_t halt = haltestelle_t::get_halt(welt, fpl->get_current_eintrag().pos,besitzer_p);
id_pair pair(last_stop_id, halt.get_id());
// The calculation of the journey distance does not need to use normalised halt locations for comparison, so
// a more accurate distance can be used. Query whether the formula from halt_detail.cc should be used here instead
// (That formula has the effect of finding the distance between the nearest points of two halts).
const uint32 journey_distance = shortest_distance(fahr[0]->get_pos().get_2d(), fahr[0]->last_stop_pos);
//last_stop_pos will be set to get_pos().get_2d() in hat_gehalten (called from inside halt->request_loading later
//so code inside if will be executed once. At arrival time.
if(journey_distance > 0)
{
arrival_time = welt->get_zeit_ms();
sint64 journey_time = welt->ticks_to_tenths_of_minutes(arrival_time - departures->get(last_stop_id).departure_time);
if(journey_time <= 0)
{
// Necessary to prevent divisions by zero.
journey_time = 1;
}
const sint32 journey_distance_meters = journey_distance * welt->get_settings().get_meters_per_tile();
const sint32 average_speed = (journey_distance_meters * 3) / ((sint32)journey_time * 5);
// For some odd reason, in some cases, laden() is called when the journey time is
// excessively low, resulting in perverse average speeds and journey times.
if(average_speed <= speed_to_kmh(get_min_top_speed()))
{
book(average_speed, CONVOI_AVERAGE_SPEED);
bool reverse = !reverse_schedule;
uint8 current_stop = fpl->get_aktuell();
fpl->increment_index(¤t_stop, &reverse);
grund_t* gr = welt->lookup(fpl->eintrag[current_stop].pos);
if(gr)
{
pair.x = gr->get_halt().get_id();
}
else
{
pair.x = 0;
}
const uint8 starting_stop = current_stop;
id_pair idp;
idp.y = pair.y;
idp.x = pair.x;
do
{
// Book the journey times from all origins served by this convoy,
// and for which data are available, to this destination.
bool allow_resetting_line_average = false;
if(!departures->is_contained(idp.x))
{
fpl->increment_index(¤t_stop, &reverse);
grund_t* gr = welt->lookup(fpl->eintrag[current_stop].pos);
if(gr)
{
pair.x = gr->get_halt().get_id();
idp.x = pair.x;
continue;
}
}
journey_time = welt->ticks_to_tenths_of_minutes(arrival_time - departures->get(pair.x).departure_time);
average_tpl<uint16> *average = average_journey_times->access(idp);
if(!average)
{
average_tpl<uint16> average;
average.add(journey_time);
average_journey_times->put(idp, average);
}
else
{
// Check for anomalies as might be created by exotic timetable arrangements
// (e.g. - D shaped timetables or multiple branching and re-joining)
// and apply a quick and somewhat dirty workaround.
if(journey_time > average->get_average() * 2 || journey_time < average->get_average() / 2)
{
// Anomaly detected - check to see whether this can be caused by odd timetabling.
// If not, then this must be caused by traffic fluctuations, and should remain.
const uint8 fpl_count = fpl->get_count();
uint32 this_stop_count = 0;
for(uint8 i = 0; i < fpl_count; i ++)
{
const grund_t* gr_2 = welt->lookup(fpl->eintrag[i].pos);
if(gr_2 && gr_2->get_halt().get_id() == idp.x)
{
this_stop_count ++;
}
}
if(this_stop_count >= 2)
{
// More than one entry - might be a timetable issue.
if(journey_time > average->get_average() * 2)
{
dbg->message("void convoi_t::laden()", "Possible timetable anomaly detected. Skipping inserting journey time (convoy).");
}
else if(journey_time < average->get_average() / 2)
{
dbg->message("void convoi_t::laden()", "Possible timetable anomaly detected. Resetting average journey times (convoy).");
average->reset();
allow_resetting_line_average = true;
goto write_basic;
}
}
else
{
goto write_basic;
}
}
else
{
write_basic:
average_journey_times->access(pair)->add_check_overflow_16(journey_time);
}
}
if(line.is_bound())
{
average_tpl<uint16> *average = get_average_journey_times()->access(idp);
if(!average)
{
average_tpl<uint16> average;
average.add(journey_time);
get_average_journey_times()->put(idp, average);
}
else
{
// Check for anomalies as might be created by exotic timetable arrangements
// (e.g. - D shaped timetables or multiple branching and re-joining)
// and apply a quick and somewhat dirty workaround.
if(journey_time > average->get_average() * 2 || journey_time < average->get_average() / 2)
{
// Anomaly detected - check to see whether this can be caused by odd timetabling.
// If not, then this must be caused by traffic fluctuations, and should remain.
const uint8 fpl_count = fpl->get_count();
uint32 this_stop_count = 0;
for(uint8 i = 0; i < fpl_count; i ++)
{
const grund_t* gr_2 = welt->lookup(fpl->eintrag[i].pos);
if(gr_2 && gr_2->get_halt().get_id() == idp.x)
{
this_stop_count ++;
}
}
if(this_stop_count >= 2)
{
// More than one entry - might be a timetable issue.
if(journey_time > average->get_average() * 2)
{
dbg->message("void convoi_t::laden()", "Possible timetable anomaly detected. Skipping inserting journey time (line).");
}
else if(allow_resetting_line_average && journey_time < average->get_average() / 2)
{
dbg->message("void convoi_t::laden()", "Possible timetable anomaly detected. Resetting average journey times (line).");
average->reset();
goto write_basic_line;
}
}
else
{
goto write_basic_line;
}
}
else
{
write_basic_line:
get_average_journey_times()->access(idp)->add_check_overflow_16(journey_time);
}
}
}
fpl->increment_index(¤t_stop, &reverse);
grund_t* gr = welt->lookup(fpl->eintrag[current_stop].pos);
if(gr)
{
pair.x = gr->get_halt().get_id();
}
else
{
pair.x = 0;
}
idp.x = pair.x;
}
while(starting_stop != current_stop && idp.x != idp.y);
}
}
bool clear_departures = false;
minivec_tpl<uint8> departure_entries_to_remove(fpl->get_count());
if(!is_circular_route())
{
uint8 stop = fpl->get_aktuell();
uint8 previous_stop = stop;
bool rev = reverse_schedule;
bool anti_rev = !rev;
halthandle_t stop_hh;
halthandle_t previous_stop_hh;
for(int i = 0; i < fpl->get_count(); i ++)
{
fpl->increment_index(&previous_stop, &anti_rev);
fpl->increment_index(&stop, &rev);
if(i == 0)
{
clear_departures = reverse_schedule != rev;
if(clear_departures)
{
break;
}
}
const grund_t* gr_this = welt->lookup(fpl->eintrag[stop].pos);
const grund_t* gr_previous = welt->lookup(fpl->eintrag[previous_stop].pos);
if(gr_previous && gr_this)
{
stop_hh = gr_this->get_halt();
previous_stop_hh = gr_previous->get_halt();
}
else
{
// Something has gone wrong.
dbg->error("void convoi_t::laden() ", "Cannot lookup halt");
continue;
}
if(previous_stop_hh.get_id() == stop_hh.get_id())
{
departure_entries_to_remove.append(stop_hh.get_id());
}
else
{
clear_departures = false;
break;
}
// If we reach the end of the list without breaking,
// we can simply clear the whole list.
clear_departures = true;
}
}
// Recalculate comfort
const uint8 comfort = get_comfort();
if(comfort)
{
book(get_comfort(), CONVOI_COMFORT);
}
FOR(departure_map, & iter, *departures)
{
// Accumulate distance
const grund_t* gr = welt->lookup(fpl->get_current_eintrag().pos);
if(gr && is_circular_route() && iter.key == gr->get_halt().get_id())
{
// If this is a circular route, reset distances from this halt,
// as the list of departures is never reset for a circular route,
// so, without this resetting, the distance would accumulate for
// ever!
iter.value.reset_distances();
}
else
{
iter.value.add_overall_distance(journey_distance);
}
}
if(state == FAHRPLANEINGABE) //"ENTER SCHEDULE" (Google)
{
return;
}
if (halt.is_bound())
{
const koord k = fpl->get_current_eintrag().pos.get_2d(); //"eintrag" = "entry" (Google)
//const spieler_t* owner = halt->get_besitzer(); //"get owner" (Google)
const grund_t* gr = welt->lookup(fpl->get_current_eintrag().pos);
const weg_t *w = gr ? gr->get_weg(fpl->get_waytype()) : NULL;
bool tram_stop_public = false;
if(fpl->get_waytype() == tram_wt)
{
const grund_t* gr = welt->lookup(fpl->get_current_eintrag().pos);
const weg_t *street = gr ? gr->get_weg(road_wt) : NULL;
if(street && (street->get_besitzer() == get_besitzer() || street->get_besitzer() == NULL || street->get_besitzer()->allows_access_to(get_besitzer()->get_player_nr())))
{
tram_stop_public = true;
}
}
const spieler_t *sp = w ? w->get_besitzer() : NULL;
if(halt->check_access(get_besitzer()) || (w && sp == NULL) || (w && sp->allows_access_to(get_besitzer()->get_player_nr())) || tram_stop_public)
{
// loading/unloading ...
// NOTE: Revenue is calculated here.
halt->request_loading(self);
}
}
if(clear_departures)
{
// If this is the end of the schedule, the departure times need to be reset
// so as to avoid over-writing good journey time data with data comprising
// the time between the last departure on the previous run to the current
// stop, which will give excessively long times.
departures->clear();
}
else
{
// In many cases, simply clearing the list does not help, such as a
// Y shaped route. In such cases, halts must be pruned
// selectively.
ITERATE(departure_entries_to_remove, i)
{
departures->remove(departure_entries_to_remove[i]);
}
}
if (wait_lock == 0 ) {
wait_lock = WTT_LOADING;
}
if(line.is_bound())
{
line->calc_is_alternating_circular_route();
}
}
I have now integrated a number of these changes for Pak128.Britain-Ex on my Github branch. The changes relating to the bridges, however, I have not implemented, as, more or less concurrently with your work here, I had, as I had planned for a number of months, integrated the bridges from Standard properly to avoid duplication, and, more importantly, avoid using the same graphics for multiple bridge types now that we have multiples.
Thank you very much for this - it is most helpful.