• Bug diagnosis
• Proposed fix
  • Pseudo-code draft for code path 3 (slope restricts speed without blocking)
• Debugging fix
  • Analysis of built-up areas check
• Release Candidate 1
• Release Candidate 2

Conveniently, some relevant code seems to be right at the top of /gui/way_info.cc:

Starting at line 33 of the latest commit:

static char const* const speed_resticted_text = "speed_restricted";

This first line should help us to confirm that we are looking at the right English localization. But looking at en.tab shows that "speed_restricted" should give us this error message:

speed_restricted
The speed of this way is restricted for reasons other than the nature of the way itself, e.g. by being in a built-up area.

That isn't the message we are seeing here. We are seeing "The speed is limited on account of being in a built-up area". That's a different message, (speed_restricted_by_city).

That localization key occurs only one place in this file, line 627. This appears to be the key passage so let's go through it line by line from 597:

		if (restricted_speed != SINT32_MAX_VALUE) {

restricted_speed initalizes to SINT32_MAX_VALUE but is set in line 461 from way->get_max_speed() if (way->get_desc()->get_topspeed() > way->get_max_speed(), which presumably means if the theoretical top speed of this way (or waytype?) is higher than the actual max speed applicable to this way. So this code is only called if there's a speed restriction.

			new_component<gui_empty_t>();
			add_table(2,1);
			{
				add_component(&speed_restricted);

This displays a GUI component. Confusingly, this speed_restricted is different from the one in line 33: it's not the text literal or the string that's in, but a gui_image_t so almost certainly the triangular warning icon.

				if( tunnel ) {

tunnel isn't a Boolean, but const tunnel_t *tunnel, which looks like a pointer to a tunnel object. If one exists then...

					if (way->get_max_speed() == tunnel->get_desc()->get_topspeed() || tunnel->get_desc()->get_topspeed_gradient_1() || tunnel->get_desc()->get_topspeed_gradient_2())
					{
						new_component<gui_label_t>("(speed_restricted_by_tunnel)", SYSCOL_TEXT_STRONG);
					}
				}

...we check whether the tunnel max speed is actually the limiting factor, and if it is, we display the appropriate error message.

				else if (bridge) {
					if (way->get_max_speed() == bridge->get_desc()->get_topspeed() || bridge->get_desc()->get_topspeed_gradient_1() || bridge->get_desc()->get_topspeed_gradient_2())
					{
						new_component<gui_label_t>("(speed_restricted_by_bridge)", SYSCOL_TEXT_STRONG);
					}
				}

This does the same thing again: if there's a pointer to a bridge object, and that's actually the limiting factor, display the appropriate error message.

				else if (wayobj) {
					if (way->get_max_speed() == wayobj->get_desc()->get_topspeed() || wayobj->get_desc()->get_topspeed_gradient_1() || wayobj->get_desc()->get_topspeed_gradient_2())
					{
						new_component<gui_label_t>("(speed_restricted_by_wayobj)", SYSCOL_TEXT_STRONG);
					}
				}

Same again, but this time a pointer to a wayobj instead.

				else if (way->is_degraded()) {
					new_component<gui_label_t>("(speed_restricted_by_degradation)", SYSCOL_TEXT_STRONG);
				}

Slightly different: this uses a getter to checker whether the way has the degraded flag, and displays an appropriate error message if so.

				else if (way->is_crossing()) {
					new_component<gui_label_t>("(speed_restricted_by_crossing)", SYSCOL_TEXT);
				}

Same again but for the crossing flag. So far, the tile in the bug report has not met any of these cases.

				else {
					new_component<gui_label_t>("(speed_restricted_by_city)", SYSCOL_TEXT_STRONG);
				}
			}
		}

So it falls through to the last remaining code-path, the de facto default: (speed_restricted_by_city).

So the bug is that the Info Window code does not check whether the speed restriction is caused by 'rapids', and falls through to an inappropriate error message.

That suggests to me that there are four, complementary, aspects to fully resolving this bug:

1. Change the current de facto default path so that it properly checks whether the speed restriction is actually caused by being in a city, just like the other code-paths above it do.
2. Add another code-path below that which checks whether the speed restriction is caused by rapids (how?§) and displays an appropriate message if so.
3. Locks also cause a speed restriction. So we might want to distinguish cases where the speed is zero from cases where the speed is merely reduced.
4. Add a proper fall-through default at the end, using the speed_restricted text, and perhaps make that message more generic, removing the reference to built-up areas.

Of course, suggesting things to add to the Simutrans codebase is much easier than actually doing it! 😩

While I looking for something else, I stumbled across a get_stadt() function in gui/wayinfo.cc:L1258-70, which at first sight looked promising:

					if (building->get_stadt() != NULL) {
						cont_road_routes.new_component<gui_label_t>(building->get_stadt()->get_name());
					}
					else {
						cont_road_routes.new_component<gui_empty_t>();
					}

					// region
					if (!welt->get_settings().regions.empty()) {
						lb = cont_road_routes.new_component<gui_label_buf_t>();
						lb->buf().printf(" (%s)", translator::translate(welt->get_region_name(k).c_str()));
						lb->update();
					}

However, this is a member function for a building of gebaeude_t*, so it's no use to us because we need something for a way.

There is nothing like get_stadt() in the variables or functions in boden/weg.h. The closest was bool should_city_adopt_this(const player_t* player);, which turns out to contain welt->get_city(pos), where pos is created by

const koord & pos = this->get_pos().get_2d();

This function itself is defined in simworld.cc as follows:

stadt_t *karte_t::get_city(const koord pos) const

The function body doesn't seem to contain anything unusual. To use this function in gui/wayinfo.cc's gui_way_detail_info_t::draw(), we're going to need to sort out three preliminary steps. Firstly, make sure simworld.h is included, which it is. Secondly, we will need to be able to use welt. Although it's used all over the file already, VSCode's IntelliSense thought that it wasn't recognized here, so let's fall back to calling world() instead. Thirdly, we need to see whether the koord of this tile has already been found in draw(). And looking for that led me to L300, which already uses get_city() inside draw(), like so:

world()->get_city(way->get_pos().get_2d())

So we need see whether way is still available at L627. That is within the if(way) block that starts at L56, which guarantees that way is indeed available.

Change L626 from:

				else {
					new_component<gui_label_t>("(speed_restricted_by_city)", SYSCOL_TEXT_STRONG);
				}

to

				else if( world()->get_city( way->get_pos().get_2d() ) ) {
					new_component<gui_label_t>("(speed_restricted_by_city)", SYSCOL_TEXT_STRONG);
				}

The info message needs to reflect the conditions that actually create rapids. So we need a way to check whether this way is a Small River (and possibly other ways, depending on pakset??) and whether it's a slope.

A look at pak128.britain's rivers.dat shows a relevant comment:

# Small river
# Navigable only on the flat

The machine-readable version of that is

topspeed_gradient_1=0	
topspeed_gradient_2=0

A further variation on that is:

# Small river - iteration 2
# Also navigable on shallow gradients
...
topspeed_gradient_1=5
topspeed_gradient_2=0

and likewise

# Medium river
# Navigable on the flat and shallow gradients
...
topspeed_gradient_1=10
topspeed_gradient_2=0

In reality, I don't think I've ever seen Extended generate a double-height river slope, but we're now going to have two cases to check for.

There are declarations of sint32s of the same name as the topspeed_gradient... parameters in the .dat in /descriptor/obj_base_desc.h:L90-91 for objects in the class obj_desc_transport_related_t. Following up the references suggests that this class only seems to be used for vehicles, not ways. Later on I found that this can't be right because they're used in weg_info.cc, but that's water under the bridge now, so let's park (or anchor!) this for now.

The only commit message referring to rapids is 8b85fb4c448d03e2dabc34b909a10b37d9b658ff CHANGE: Disapply canal height limit for slope tiles to allow canalising rapids, which adds a height check to wegbauer.cc L609. The old code was

	if(max_altitude > 0 && altitude > max_altitude)
	{
		// Too high
		return false;
	}

and the new code is

	if(max_altitude > 0 && altitude > max_altitude && (from->get_grund_hang() + to->get_grund_hang() == 0))
	{
		// Too high - exclude slope tiles from this to allow canalising rapids
		return false;
	}

so the rapids check is

 && (from->get_grund_hang() + to->get_grund_hang() == 0)

Unfortunately, this isn't really much help for the purposes of a GUI window, because we don't have the kind of from...to data that would be possessed by a function building a way.

The Standard documentation shows a class slope_t is declared in dataobj./ribi.h L20. That suggests that we might be able to use the same strategy as for the code-paths for bridges & tunnels: is there a pointer to an object of this class? (L108 also has an older version of that, slope4_t, so we might need to account for older savegames that use that as well.) It turns out that the bridge and tunnel pointers are initialized in L59-60 like this:

const bruecke_t *bridge = gr->find<bruecke_t>();
const tunnel_t  *tunnel = gr->find<tunnel_t>();

gr here is obviously going to be "Grund", something giving the tile and L52 confirms it: it's a koord3d. That seems like a model we might be able to follow. The find() function is in the *grund_t namespace, and unfortunately it's a one-line template at grund.h L606:

template<typename T> T* find(uint start = 0) const { return static_cast<T*>(objlist.suche(map_obj<T>::code, start)); }

A comment says that objlist is the "List of objects on this tile", which is exactly what we want. It seems that there's a reasonable chance that find<slope_t>() might be what we need. But unfortunately, VSCode says that's not present anywhere in the codebase. grund.h isn't immediately informative about what is added to objlist; I can't see anything obviously like objlist.add().

But while looking for it, I found in L730:

virtual slope_t::type get_weg_hang() const { return get_grund_hang(); }

And, lo and behold!, this is called in line 88 of way_info.cc, like so:

const slope_t::type hang = gr ? gr->get_weg_hang() : slope_t::flat;
const sint32 potential_way_speed = !hang ? way->get_desc()->get_topspeed() : hang == 1 ? way->get_desc()->get_topspeed_gradient_1() : way->get_desc()->get_topspeed_gradient_2();

hang is of course from German Hang, slope (I searched earlier for something like ist_hang() but got nowhere). So if the tile exists§, then the slope_t variable hang becomes a pointer to an object given by get_weg_hang() , otherwise it becomes the enum item(?) flat. We can't yet be sure if the hang variable still exists by the time we get to the warning, but it shows that how we check whether the present tile possesses a slope. We can also use way->get_desc()->get_topspeed_gradient_n() to check whether we have a topspeed_gradient_n of 0, in which case we show the error message.

§ This check mystifies me. L53 of this function already has

	if (!gr) {
		return;
	}

so if gr doesn't exist, then we should never reach anywhere near this far into the function. I guess that it's a standard pointer safety check inserted out of an abundance of caution to avoid crashes.

A search of en.tab suggests that there's no relevant localizations warning about slopes or rapids. So we'll have to re-learn how to add a localization to SimuTranslator, which is really a topic for the Simutrans Wiki, not here. I can't find the forum post explaining this, so for the initial PR, let's just edit en.tab.

The localization of way_blocked_by_slope in particular needs a bit of consideration. AFAIK this is only caused by rapids in pak128.Britain-Ex and we really want to hint to players that they can get around this by building canal locks, because that won't be intuitive to most players. But other paksets might set the way speed to 0 for other reasons. So maybe: This way is blocked by the slope; check if you can improve the way (e.g. upgrading a river to a canal).

In gui_way_detail_info_t::draw(), insert after gui/way_info.cc:L629:

         else if ( gr->get_weg_hang() ) {

We can definitely use gr, because it's created in the first line of this function, and there's a sanity check in L53.

                  if ( ( gr->get_weg_hang == 1 && ( way->get_desc()->get_topspeed_gradient_1() == 0 ) || ( gr->get_weg_hang == 2 && ( way->get_desc()->get_topspeed_gradient_2() == 0  ) ) {

If we are on a slope of 1 and the 1-slope speed limit is zero, or the slope of 2 equivalent, then display the appropriate new error message:

					new_component<gui_label_t>("(way_blocked_by_slope)", SYSCOL_TEXT_STRONG);

(Closing curly brackets are in the next section)

The difficulty here is working out what is the correct variable to compare get_topspeed_gradient_n to. We want the theoretical highest speed of the way without slopes, bridges or other impediments. topspeed seems to be set (from the .dat file??) alongside topspeed_gradient_n in obj_desc.h, which looks right. But what is maxspeed then? In boden/wege/weg.cc:L141, we find get_max_speed() defined thus:

sint32 weg_t::get_max_speed(bool needs_electrification) const
{
	if( needs_electrification && is_electrified() ) {
		if( grund_t* gr = welt->lookup(get_pos()) ) {
			// "is_electrified()==true" means tile has an overhead_line wayobj
			return min(max_speed, gr->get_wayobj(get_waytype())->get_desc()->get_topspeed());
		}
	}
	return max_speed;
}

So if the way is electrified, then it's the lower of max_speed or the topspeed of the wayobj (e.g. OLE), otherwise it's max_speed. max_speed is declared as a uint16 in /boden/wege/weg.h:L161 with ?Prissi's comment "max speed; could not be taken for desc, since other object may modify the speed". The first part of that sentence is not clear to me: I know desc is about parameters read from pakset files, but "could not be taken for" isn't clear to me. But the fact that "other object may modify the speed" suggests that it is the speed without bridges, slopes etc. It's also settable through the sint32 set_max_speed() in weg.h and it's definitely read from pakset files. The point is that it's a property of ways.

The most reliable answer is looking at what existing code in wayinfo.cc does. And it seems to use topspeed as the theoretical highest speed of the way, presumably listed in the .dat file, and maxspeed as the currently applicable highest speed in the current way at these particular co-ordinates.

So let's use topspeed:

                  } else if ( ( gr->get_weg_hang == 1 && ( way->get_desc()->get_topspeed_gradient_1() < way->get_desc()->get_topspeed ) || ( gr->get_weg_hang == 2 && ( way->get_desc()->get_topspeed_gradient_2() < way->get_desc()->get_topspeed ) ) {
                                        new_component<gui_label_t>("(speed_restricted_by_slope)", SYSCOL_TEXT);
                  }
         }

The text of speed_restricted_by_slope could be: ``The speed is restricted here by the slope`.

After the case above, add another else to act as a fallback if none of the previous error messages applies

				else {
					new_component<gui_label_t>("(speed_restricted)", SYSCOL_TEXT_STRONG);
				}

The error message could be changed to:

speed_restricted
The speed of this way is restricted for reasons other than the nature of the way itself; please report this as a bug.

The first attempt at the fix does not work.

All in my 2025Map save.

1. (936,730) is a Narrowboat Canal lock. No warning text is displayed. Hovering over the exclamation mark emoji produces a tooltip saying "...this way is restricted for reasons other the nature of the way itself, e.g. by reason of being in a built-up area".

2. (811,966) has Upstream Small River rapids created for testing this fix. The speed ins 0km/h. Again, no text, and the same tooltip as above.

3. Clicking on roads that are limited by being in a built-up area doesn't produce any speed warning at all. [Note: I later realized this is because I tested on a 19th century map, this actually works fine when correctly tested]

4. Clicking on the urban Barge Canal lock at (776,729) shows the warning "This speed is limited on account of being a built-up area". This is what I wrote in the code, and confirms that at least some of my code is working, but isn't a desirable outcome, because the speed is actually limited by the slope, so we've gone down the wrong code path.

5. Looking at the ford at (697,943), we get the untranslated text literal "(speed restricted by a crossing)". The warning emoji tooltip shows the "reasons other than" message so that seems not to vary.

6. Mothballed railway track seems to retain its top speed, so I can't use that for testing.

7. Railway tunnels continue to show the correct message about speed being restricted by the tunnel, so at least I haven't broken that.

So the issues there are :

1. Locks (low-speed canal slopes) not showing speed_restricted_by_slope

2. Rapids (zero-speed river slopes) not showing way_blocked_by_slope

3. Roads not showing speed_restricted_by_city [NB: not an issue, I tested on inappropriate 19th-century roads]

4. Urban locks showing speed_restricted_by_city instead of speed_restricted_by_slope

5. Crossings seem to show a text key, (speed restricted by a crossing), rather than localized text

• My first mistake is that we now have localization keys called speed_restricted and (speed_restricted); I thought existing code consistently used brackets, but it seems not. Let's leave speed_restricted unchanged as the generic message in the emoji tooltip and use (speed_restricted_fallback) as the fallback message.
• Swapping the code-paths for slopes and cities should fix (4) if we can fix the cause of (1) and (2).
• The common thread of (1) and (2) is that they go through the if ( gr->get_weg_hang() check and make use of if ( gr->get_weg_hang() == 1 ) etc. I now realize that really was pseudo-code not draft code. It seems that get_weg_hang() returns a slope_t::type, which according to /dataobj/ribi.h:L36 is a sint8. Note that as well as slope_t::type there is slope_t::_type, which is an enum with a very similar name. The comment for it says it's "Named constants for special cases.", i.e. of the values packed into slope_t::type, such as slope_t::flat, though I don't know how _type interacts with type. So I think I need to do a much more careful analysis of gui/wayinfo.cc:L88-89 and figure out exactly what's going on there. I was trying to stick to C++ that I was certain I understood but it seems that get_weg_hang() might require learning some things.

Really this is a continuation of A useful check we can steal! above. A reminder of L88:

const slope_t::type hang = gr ? gr->get_weg_hang() : slope_t::flat;

• This initializes constant hang (remember that "Hang" is German for "slope") of type slope_t::type (which is a sint8 type) will be initialized to one of two possibilities:
  • If gr exists, the we call the get_weg_hang() function on gr. This return a value of type slope_t::type.
  • If gr goes not exist, then we set hang to slope_t::flat`` which is a enum value, a special case for when slope_t::type== 0 (as perdataobj/ribi.h:L43`).
• I am not clear whether or not slope_t::flat is a scoped enum or not. It isn't declared using the enum class or enum struct keyword. But that appears to be a C++11 feature, and it's very plausible that Simutrans has reinvented the wheel here (in the same way that it has reimplemented printf and parts of the the Standard Library). If it behaves like a scoped enum, then you can't implicitly convert it to an integer. VSCode can't find any uses of _type except where it's declared, so it's not at all clear how it relates to type.

const sint32 potential_way_speed = !hang ? way->get_desc()->get_topspeed() : hang == 1 ? way->get_desc()->get_topspeed_gradient_1() : way->get_desc()->get_topspeed_gradient_2();

• This initializes a sint32 constant potential_way_speed.
• The ternary operator is used to check whether hang is nonzero. Remember that if hang is zero, that equals slope_t::flat. I need to come back and consider the case where hang is negative.
  • If hang is zero, then we have a pile-up of member functions: way->get_desc()->get_topspeed(). way is a pointer to the way, obviously the one for this Way Info Window. get_desc() appears to return a pointer to the description of the way (in the .dat stored in the .pak file). get_topseed() gets the top speed of the way from that pointer.
  • If hang is nonzero, then we have another ternary operator to check whether hang (which remember is a slope_t::type) is 1. Now, according to /dataobj/ribi.h:L48, the special case where slope_t::type == 1 is slope_t::southwest explained by the comment "SW corner". AFAIK you can never build a way on a tile that slopes to only its southwest corner, so this check will never find TRUE.
    • But if it did, we would call way->get_desc()->get_topspeed_gradient_1() and so potential_way_speed would be set to obj_desc_transport_related_t::topspeed_gradient_1, which the comment explains as "maximum allowed speed in km/h for a half/single height gradient".
    • In all cases, we will therefore call way->get_desc()->get_topspeed_gradient_2(), which will set potential_way_speed to obj_desc_transport_related_t::topspeed_gradient_2, which the comment explains as "maximum allowed speed in km/h for a single/double height gradient".
• Let's think more about that !hang.
  • If hang == 0, then hang evaluates to FALSE, which !hang reverses to TRUE, and we go down the topspeed path.
  • If hang >= 1, then hang evaluates to TRUE, which !hang reverses to FALSE, and we go down the topspeed_gradient paths.
  • If hang <= -1, then hang evaluates to TRUE, which !hang reverses to FALSE, and we go down the topspeed_gradient paths. I don't know whether this actually occurs (it's possible that slope_t::type was made a sint so that overflows show up more obviously) but I need to allow for the possibility.

• Let's think more about that hang == 1. Is there anywhere else in the Simutrans codebase using integer checks on slope_t::type? The very ambiguous name does not help here.
  • There are some potentially helpful notes in simtool.cc:L3450ff. as follows:

	// get initial height of bridge from start tile
	// flat -> height is 1 if conversion factor 1, 2 if conversion factor 2
	// single height -> height is 1
	// double height -> height is 2
	const slope_t::type slope = gr->get_weg_hang();
	uint8 max_height = slope ?  slope_t::max_diff(slope) : bridge_height;

• I think here -> is being used to mean something like == rather than as the member operator.
• Maybe get_weg_hang is returning slope_t::type in some special way so that it really does equal the height of the slope rather than the values used in the slope_t::_type (NB underscore) enum???

• See also Standard's /src/simutrans/builder/brueckenbauer.cc:L350ff. where:

	if (slope_t::type sl = gr->get_weg_hang()) {
		if (!slope_t::is_way(sl)) {
			// it's not a straight slope
			return "Bruecke muss an\neinfachem\nHang beginnen!\n";
		}

		if (slope_t::max_diff(gr->get_weg_hang()) == 2  &&  !desc->has_double_start()) {
			// cannot handle double slope
			return "Bruecke muss an\neinfachem\nHang beginnen!\n";
		}

Here we can see that Prissi allows the equivalent of if (hang = gr->get_weg_hang()) as a check to see whether this tile (not yet a way in this case, because we're building a bridge) is a slope. But interestingly, to see whether a slope is a double slope, he doesn't just compare the value returned by gr->get_weg_hang() to integer 2. Instead, he calls slope_t::max_diff() to the result. At least in Extended, this function returns a uint and the comment says it "Returns maximal height difference between the corners of this slope". So that might be another avenue that we go down to find which topspeed_gradient... variable is applicable.

We've been saying that this returns a slope_t::type, but maybe that's incorrect. It's declared in boden\grund.h:L730 as:

virtual slope_t::type get_weg_hang() const { return get_grund_hang(); }

If it's a virtual function, there must be some subclass which has its own version of get_weg_hang(). This is fact the case, and searching header files finds the following:

brueckenboden.h: slope_t::type get_weg_hang() const OVERRIDE { return weg_hang; } pier_deck.h: slope_t::type get_weg_hang() const override {return way_slope; } tunnelboden.h: slope_t::type get_weg_hang() const OVERRIDE { return ist_karten_boden() ? (slope_t::type)slope_t::flat : get_grund_hang(); }

These are for bridges, piers, and tunnels, which makes perfect sense, because these are cases where the slope of the way might (usually will) be different from the slope of the ground. If you called gr->get_weg_hang fron a codepath handling one of these then you'd at best get the wrong information (for our use case, the Way Info Window). This is a complication I hadn't considered and I need to check that the urban roads codepath handles this correctly. But one main point here is that on-the-ground ways like locks and rapids don't have subclasses and they should be using the base virtual function. [EDIT 19/4: The other main point is that the tunnel version of get_weg_hang are also returning slope_t::type bitfields, not simple integers reporting the height of the slope, as the current weg_info.h assumes. The bridge version is returning a uint8 weg_hang that might be doing its own thing. This starts to get complicated depending on which version of the function is being called. Standard DOES have a slope_t::type weg_hang for bridges, so this is starting to look like it might be a merge error in Extended.... Standard made the change in ceeac's Commit a9f9cff , which is SVN revision 10399, which is not listed in cherry-picked-commits.]

Let's follow the white rabbit to get_grund_hang() at boden\grund.h:L432:

	// slope are now maintained locally
	slope_t::type get_grund_hang() const { return slope; }

We then follow that to

	/**
	 * Slope (now saved locally), because different grounds need different slopes
	 */
	slope_t::type slope;

Let's stop and consider this. gr->get_weg_hang() is ultimately going to return us this variable slope, which of the type slope_t::type, which is really a sint8, but which always seems to be manipulated using the various enum values and is probably a bitfield or something very similar to one. I am more and more convinced that the hang == 1 method used in ~L89 of weginfo.cc is based on the assumption that get_weg_hang() returns an integer value for the slope height, when actually it's returning a bitfield value. [EDIT 19/4: It certainly returns a bitfield for the case of gr that I need here, but

While researching the above sections, I stumbled across two potentially helpful functions at dataobj/ribi.h:L90:

	/// Returns if slope prefers certain way directions (either n/s or e/w).
	static bool is_single(type x) { return (flags[x] & single) != 0; }

	static bool is_doubles(type x) { return (flags[x] & doubles) != 0; }

These would seem to be public functions that we can use to avoid comparing integers, which is the part that is worrying me. But what puzzles me is what we call them on. When I've seen is_adjective() functions in tutorials, they've been called on objects using the slope.is_single() syntax. But I guess we can try just passing get_weg_hang() as the parameter, as long as we include ribi.h in gui/weginfo.h, since slopes are variables of ground objects, rather than being a free-floating object in their own right.

We included ribi.h.

else if ( gr->get_weg_hang() ) {

Standard uses this, so we can be sure it's a safe way to check that this is a slope. Now switch gr->get_weg_hang() == 1 to slope_t::is_single(gr->get_weg_hang()) and likewise for 2, like so:

					if ( ( slope_t::is_single(gr->get_weg_hang()) && ( way->get_desc()->get_topspeed_gradient_1() == 0 ) ) || ( slope_t::is_doubles(gr->get_weg_hang()) && ( way->get_desc()->get_topspeed_gradient_2() == 0 ) ) ) {
						new_component<gui_label_t>("(way_blocked_by_slope)", SYSCOL_TEXT_STRONG);
					} else if ( ( slope_t::is_single(gr->get_weg_hang()) && ( way->get_desc()->get_topspeed_gradient_1() < way->get_desc()->get_topspeed() ) ) || ( slope_t::is_doubles(gr->get_weg_hang())  && ( way->get_desc()->get_topspeed_gradient_2() < way->get_desc()->get_topspeed() ) ) ) {
						new_component<gui_label_t>("(speed_restricted_by_slope)", SYSCOL_TEXT);
  					}
				}

All in my 2025Map save.

1. (936,730) is a Narrowboat Canal lock. The speed restriction text is correctly displayed. Yay!

2. (811,966) has Upstream Small River rapids created for testing this fix. The speed ins 0km/h. The movement blocked text is correctly displayed. Yay!

3. Clicking on roads that are limited by being in a built-up area doesn't produce any speed warning at all. So swapping the city and slope checks wasn't the cause and/or wasn't enough to fix the bug.

4. (776,729) is an urban Barge Canal lock. The speed restriction text is correctly displayed. Yay!

5. Looking at the ford at (697,943), we still get the untranslated text literal "(speed restricted by a crossing)". Still needs fixing.

6. n/a

7. Railway tunnels continue to show the correct message about speed being restricted by the tunnel, so at least I haven't broken that. However,

8. At (776,750,2), we have a slope in a railway tunnel. It shows the correct message about speed being restricted by the tunnel, but it would be good to test with canal and road tunnels too.

Let's look again at the check that I introduced:

				else if ( world()->get_city( way->get_pos().get_2d() ) ) {
					new_component<gui_label_t>("(speed_restricted_by_city)", SYSCOL_TEXT_STRONG); 
				} else {

The first things that stands out is that we are using a mix of arrow and dot operators. Maybe I'm using the wrong operator or one of the steps is returning a Boolean value that I didn't expect.

simworld.h:L144 tell us that world() returns a pointer of type karte_t* to the single instance of the world. We dereference that with the -> operator. simworld.h:L2155 and L4136 tell us that karte_t*::get_city(koord pos) returns a pointer of type stadt_t* and the comment confirms

	// Returns the city at the position given.
	// Returns NULL if there is no city there.

NULL evaluates to FALSE, and a pointer is nonzero so will evaluate to TRUE. So let's look at the parameter we are passing to get_city().

gui\way_info.h:L25 tells us that way is a pointer of type weg_t and a member of class gui_way_detail_info_t. Everything that I am touching in gui\way_info.cc (except the #include) is within the body of the if(way) check in L57, so we can be certain that a way exists. And it's also all within the function draw(), which is also a member of *gui_way_detail_info_t, so I am fairly certain we can access way at all times for this instance of the class.

We dereference way with the arrow operator. Then we call get_pos(), which is a member function of the class obj_t.

(obj/simobj.h:L29 tell us that is the "Base class of all objects on the map, obj == thing". A way is obviously a thing on the map, so it likely to be derived from that base class. This is confirmed by boden/wege/weg.h:L89 stating that weg_t is a class of obj_no_info_t, which in turn is declared in obj\simobj.h:L380 to be of type obj_t. But the comment here explains that obj_no_info_t is for "Game objects that do not have description windows (for instance zeiger_t, wolke_t)". But in Extended, ways do have description windows, because that's exactly what I'm modifying. I am not going to start changing major classes like this, but it's worth bearing in mind as a possible cause.)

obj/simobj.h:L297 tells us that get_pos() returns a koord3d. But get_city() needs a 2D koord, not a koord3D, so we use the dot operator to call get_2d() on it. dataobj\koord3d.h:L40 tells us that this function indeed returns a koord and is in the class koord3d so the koord3d must have access to it.

So we dereference the way pointer to get_pos(), which returns a koord3d, which get_2d() converts to a koord, which is passed to this world instance's get_city(), which will return a pointer evaluating to TRUE if the way is on a tile in a city, and a NULL evaluating to FALSE if not.

In addition, L74 of this same function also uses world()->get_city(way->get_pos().get_2d()), which is where I stole it from. And it works there. The difference there is that the if-check also checks for way->get_desc()->get_waytype() == road_wt. But in the absence of that check, my code path should still be taken (I might expect the code path to be taken too often, e.g. on canals, but I am not seeing it taken at all).

I have double-checked the brackets in the parameter calls and they are fine.

So far, I am stumped.

I have double-checked that the localization key exists.

Could it be a mistake in the curly brackets of the if/else blocks? Checked using VSCode's colour-coding and careful reading and I can't see a problem there.

All the code paths are within the body of the initial branch in L598:

if (restricted_speed != SINT32_MAX_VALUE) {

Perhaps restricted_speed is not affected by built-up areas?

This is initialized earlier in the same function (in L287) to SINT32_MAX_VALUE. At L460 the variable may be set to a useful value:

			if (way->get_desc()->get_topspeed() > way->get_max_speed()) {
				...
				restricted_speed = way->get_max_speed();

We explored the difference between topspeed and max_speed extensively earlier in Pseudo-code draft for code path 3 (slope restricts speed without blocking). Looking back at that section, I realize that get_max_speed() checks for electrification, but not for being in a built-up area.

I still don't fully understand Prissi's comment about the difference between the two. I suppose we need to see how the built-up areas limit is actually applied/used and whether that uses topspeed or max_speed. If it's using topspeed from desc then we have a big problem with the whole structure of the codepaths in the Way Info Window. But if it doesn't interfere with the working of get_max_speed() in other places, could we perhaps incorporate a built-up areas checks into it and then use that to set restriced_speed to a more accurate value?

(restricted_speed is checked a few times again in L477f., L486 & L496, but that's for replacement ways (i.e. railway track renewals), which we don't care about for this bug.)

The built-up areas speed limit is set in simuconf.tab by town_road_speed_limit and the comment says the mechanism is disabled by setting it to 0. The getter is the usual get_town_road_speed_limit() in dataobj/settings.h. This is called in a few places that are too abstract for our concerns and four places that look highly relevant.

At boden/wege/weg.cc:L411, in weg_t::calc_speed_limit(), we have two calls to the getter:

	if (desc->get_wtyp() == road_wt)
	{
		if (hat_gehweg())
		{
			if (welt->get_settings().get_town_road_speed_limit())
			{
				set_max_speed(min(welt->get_settings().get_town_road_speed_limit(), get_desc()->get_topspeed()));
			}
			else
			{
				set_max_speed(get_desc()->get_topspeed());
			}
		}
	}

• First, check if the Waytype is a Road.
  • If so, check if the the Way has a pavement (Gehweg = pavement/sidewalk, i.e. the road has been adopted by the city)
    • If so, check if there is a town_road_speed_limit
      • If so, set_max_speed() to the lower of (a) the town_road_speed_limit or (b) the topspeed from the .pak.
      • If not.... then it's irrelevant to us.

So that means that max_speed does take account of town_road_speed_limit, so long as calc_speed_limit has been applied to the Way before the Way Info Window accesses it.

We have two more calls to the town_road_speed_limit getter at boden/wege/strasse.cc:L37, in strasse_t::set_gehweg(). So we are in the Roads (strasse_t) subclass of Ways (weg_t) and this sets the pavement (i.e. adopts the road); the function takes a single Boolean parameter, the (un)helpfully named janein ("yesno"):

	weg_t::set_gehweg(janein);
	if(janein && get_desc())
	{
		if(welt->get_settings().get_town_road_speed_limit())
		{
			set_max_speed(min(welt->get_settings().get_town_road_speed_limit(), get_desc()->get_topspeed()));
		}
		else
		{
			set_max_speed(get_desc()->get_topspeed());
		}
	}

Having passed initial checks irrelevant to us, this function calls its base class sibling, which sets a SIDEWALK flag on the Way/Road.

• Now check whether the Boolean janein is positive and we can get the Road's description from the .pak (why couldn't we???).
  • If so, check whether there is a town_road_speed_limit
    • If so, set_max_speed() to the lower of (a) the town_road_speed_limit or (b) the topspeed from the .pak.
    • If not.... then it's irrelevant to us.

So for our purposes, strasse_t::set_gehweg() does the same thing as weg_t::calc_speed_limit(): it sets the Road/Way's max_speed to the town_road_speed_limit. It's called in various places you'd expect: when roads are built, when citybuildings are built, etc.

So the town_road_speed_limit absolutely should be applied to the max_speed variable of at least every adopted Road in a built-up area and therefore restricted_speed should pass the if-check on those Roads. Yet the Way Info Window is not taking the desired code path.

Yesterday I noticed that a few places in gui/weg_info.cc use the lookup() function. Am I missing a step here? For example, L53 has:

grund_t *gr = world()->lookup(way->get_pos())

The approach that I borrowed/stole for my code-path doesn't use lookup(); for convenience here it is:

world()->get_city( way->get_pos().get_2d() )

simworld.h:L1898 explains that lookup() returns the "grund an pos/height", i.e. a pointer to the given tile on the map. So the former code (L53) uses lookup() to get a pointer to the tile that the Way Info Window is interested in (perhaps using the co-ordinates passed by the mouse event??). My code already has a pointer to the Way; we're getting that Way's co-ordinates variable in order to pass the co-ordinates to get_city(). So I'm doing the reverse process and lookup() is no use to me.

I'm now firing arrows blindly into the dark. Let's add a message to see whether this codepath is even taken.

The code-path was not taken on ordinary urban roads. It was taken on canal locks. So something must be happening earlier in gui/weg_info.cc to stop this.

D'oh! D'oh! D'oh! The reason that the code-path isn't taken is that I'm testing on 19th century roads where the topspeed is 35km/h and so the built-up areas limit doesn't reduce them anyway.... My initial implementation works fine when tested on 21st century roads.

I tested this wayobj testing by building light rail catenary, third-rail electricification, and a one train staff cabinet on high speed (320 km/h) track. Neither display the appropriate explanation. But there might be good reasons for that: I'm not certain that the electrification wayobjs actually reduce the speed of e.g. diesel trains. It's also possible that the one-train-staff-cabinet only limits speed in one direction or has other oddities. I'm going to decide that fixing this is outside the scope of this bug, which is really about rapids.

Underground canal locks don't seem to apply any speed limits on slopes. That simplifies matters considerably.

The urban speed limit is not applied to unadopted (no Gehweg) roads.

The original code used an elif chain:

• If a bridge, that caused the restriction, but if not...
  • ...and a tunnel, that caused the restriction, but if not...
    • ...and have a wayobj, that caused the restriction, but if not...
      • ...and degraded, that caused the restriction, but if not...
        • ...and a crossing, that caused the restriction, but if not...
          • ...then guess it's the urban speed limit.

Some of these might overlap: it seems that in principle you could have a degraded tunnel. But it's likely the original coder understood this better than I do.

Where do slopes and an independent urban speed limit check fit into this?

Although pak128.Britain-Ex only uses the topspeed_gradient_n restrictions for ground canals and rivers, they could in principle be applied to bridges, which seem to be scripted simply as ways, or to road types that are used in urban areas. So we need parallel warning checks, for the original chain (minus urban speed limit), for slopes, and for the urban speed limit (checking properly for road adoption, which we have learned is the key factor). That means adding two more rows to the table or learning how to conditionally expand the table.

• SHOW_BBOX is still enabled
• A superfluous space (empty component/table?) and warning triangle are displayed when the if-chain is all false.
• Splitting the blocked slope message over two lines doesn't work (see screenshot): the second line just fails to show.

In 2025Map save:

1. (936,730) is a Narrowboat Canal lock. The correct warning text is displayed under the superfluous warning triangle.

2. (811,966) has Upstream Small River rapids. The correct warning text is displayed under the superfluous warning triangle.

3. Built-up roads can't be tested on this map.

4. (776,729) is an the urban Barge Canal lock. The correct warning text is displayed under the superfluous warning triangle. There is no built-up area warning, which is correct.

5. (697,943) is a ford. The untranslated text literal "(speed restricted by a crossing)" remains.

6. n/a

7. Railway tunnels continue to show the correct message about speed being restricted by the tunnel.

In rapid-test-hilly save:

4. On adopted urban roads, the correct warning text is displayed under the superfluous warning triangle.

5. (25,99) is an Upstream Medium River on a double-height slope. It displays the correct warning text about a blockage, but with the second half cut off due to the \n issue, under the superfluous warning triangle..

6. (22,100,7) is a Large Ship Canal lock in a Narrowboat Tunnel (which should presumably be impossible, but that's a separate bug). It displays the correct warning about the slope speed restriction, under the superfluous warning triangle. It is also says the speed is restricted by the tunnel, which is incorrect.

7. (31,161,0) is a Barge Canal lock in a Barge Canal Tunnel. It does not display any speed restriction warning because no speed restriction is in place.

I have resolved all four issues exposed by the 'Debugging Fix' tests above.

The new issues are: [ticks indicate fix written]

1. SHOW_BBOX is on. ✅

2. A superfluous space (empty component/table?) and warning triangle are displayed when the if-chain is all false. ✅

3. Splitting the blocked slope message over two lines doesn't work (see screenshot): the second line just fails to show. ✅

4. The (speed_restricted_by_crossing) text in en.tab is incomplete. ✅

5. Test 10 shows that underground locks do not always show speed restriction warnings, because the speed restrictions are not always applied. That is beyond the scope of this bug.

6. Test 9 shows that if a slope speed restriction does apply in a tunnel, then the tunnel warning message shows even though the tunnel is not the limiting factor. Part of the difficulty here is that I don't know what ways are permissible in which tunnels. I think fixing this may also be beyond the scope of this bug until that is clarified. But it's also a fundamental flaw of the approach that I have taken.

7. weg_info.cc:LL91,101 are still using hang == 1 etc. Is that a factor?

• SHOW_BBOX no longer enabled

In 2025Map save:

1. (936,730) is a Narrowboat Canal lock. The correct warning text is displayed.

2. (811,966) has Upstream Small River rapids. The correct warning text is displayed.

3. Built-up roads can't be tested on this map.

4. (776,729) is an urban Barge Canal lock. The correct warning text is displayed. There is no built-up area warning, which is correct.

5. (697,943) is a ford over a brook. The correct warning text appears in the road tab. (There are two road tabs, probably because the brook is not a navigable way, but that is outside the scope of this bug).

6. n/a

7. Railway tunnels continue to show the correct message about speed being restricted by the tunnel.

In rapid-test-hilly save:

4. On adopted urban roads, the correct warning text is displayed.

5. (25,99) is an Upstream Medium River on a double-height slope. It displays the correct warning text about a blockage.

6. (22,100,7) is a Large Ship Canal lock in a Barge Tunnel (which should presumably be impossible, but that's a separate bug), going down to the south. Initially it displayed a warning about the slope speed restriction, but at a later point that disappeared. It is also says the speed is restricted by the tunnel, which is incorrect.

9bis.(20,98,7) is a Large Ship Canal lock in a Barge Tunnel(which should presumably be impossible, but that's a separate bug), going down to the west. It does not display any slope warnings.

Surely ONE of 9 and 9bis is incorrect?

10. (31,161,0) is a Barge Canal lock in a Barge Canal Tunnel. It does not display any speed restriction warning because no speed restriction is in place.

1. SHOW_BBOX confirmed fixed.

2. Superfluous warning triangle confirmed fixed.

3. Blocked slope message overflow confirmed fixed.

4. (speed_restricted_by_crossing) message confirmed fixed.

5-7. Speed restrictions in tunnels are confirmed to have significant issues beyond the scope of this bug.