Info window for 'rapids' has incorrect error message

[Matthew]

Steps to reproduce

1. Outside of city limits, find or create a Small River that passes over a slope, which creates 'rapids' (or at least, I think that is the intention from the graphics).
2. Click on the 'rapids' to open the Info Window.
3. Scroll to the bottom.

Expected results

An error message that explains water vehicles cannot pass over the 'rapids' and tells players to build locks.

Actual results

An error message that says: "The speed is limited on account of being in a built-up area".

Example



Note also the words "Open countryside" at the top, confirming that this tile is not within city limits. I double-checked this with the minimap.

Observation

This could also be a pakset bug.

[Matthew]

I know that some Extended developers prefer just to receive bug reports without speculation about the cause, so if that is you, please ignore this post.

For the benefit of any developers who don't mind speculation, I have put my guess about the cause on GitHub. That hides it from people who don't want to see it, and also means the code is automatically formatted in glorious Technicolor.

[Matthew]

PR#675 fixes this bug and tries to improve the Way Info window's presentation of speed restrictions.

Current code
At the moment, the Way Info Window detects a speed restriction and checks whether it is due to a tunnel, bridge, way object (e.g. OLE), way degradation, or crossing, in that order, and displays a warning only for whichever it checks first. If it's none of those things, you get a fallback warning stating that the speed restriction is due to a built-up area (the town speed limit for roads), but the code doesn't actually check whether that's true. It doesn't display any specific warning or explanation about slopes.

Slope speed restriction warning added
It would almost have been possible to fit speed restrictions on slopes into that chain if we only considered pak128.Britain-Ex. In that pakset, only canals and rivers have slope speed restrictions, so canal locks and rapids don't overlap with most of the other categories of restriction.

However, there are two problems with that. Firstly, there's nothing in the .dat file reference for Ways suggesting that this pattern is a requirement. For example, there might be other paksets that do have both slope restrictions and way object restrictions on the same tile, either now or in the future. So I made the slope speed restriction warning additional to the existing ones.

Town speed limit warning and fallback
Having going down that road, I then added a second additional warning for roads in built-up areas that have been adopted by towns. That means the fallback case should never appear, and I have altered it so it now tells players they have found a bug. If there are other speed restrictions that I have never thought of, then they should be bug reported so that the Way Info Window can give a proper warning message for those cases too.

Tunnels
The second reason that slopes have to be handled separately from the Way Info Window's existing checks is canal tunnels. pak128.Britain-Ex's .dat files say that it does have both tunnel speed restrictions and slope restrictions for canals (locks/rapids). That seems to be in accordance with the .dat file reference for Ways.

I expected that canal locks in tunnels would have the lower of the Way speed limit, the tunnel's speed restriction, or the Way's slope speed restrictions. But the current version of the Way Info Window mostly reports that the only speed limit that applies is the tunnel speed limit, which would mean that many canal types are faster underground than above ground, if you could find a suitable vehicle. For example, any player can build a Large Ship Canal (speed limit 11 km/h) in a Narrowboat Canal Tunnel (50 km/h) and the Way Info Window predicts that vehicles will travel at up to 50km/h, even through locks. This surprises me. Is this the intended behaviour? Until I know which speed limit is supposed to apply, it is difficult to target the correct behaviour in the GUI, and there's no point rewriting the Window if the current behaviour is a bug.

I said in the previous paragraph that the current version of the Way Info Window mostly reports that the only speed limit that applies is the tunnel speed limit. When you click on underground canal locks, it very occasionally reports that the speed restriction is 2km/h, the slope speed limit (the current version wrongly warns that this speed limit is due to the tunnel because it doesn't have any warning for slopes). But it is inconsistent even in successive clicks on the same tile, so that definitely is a bug, and a different one from the one that is the topic of this thread.

I will report this new bug separately, but I have left the Way Info Window's handling of underground canals (and underground rivers, which can be built by the Public Player) untouched. If there is an underground canal lock, the Way Info Window mostly can't detect it, but when it does detect it, this patch will display the appropriate speed restriction warning. This patch doesn't fix that newly-discovered bug with canal tunnels, but it doesn't make it worse either.

[jamespetts]

Excellent, thank you for that. Are you able to add the .dat files to Simutranslator to match the additional strings that you have created?

In any event, I have now merged this.

To answer your question, the intended behaviour is that the speed limit is indeed the lowest of the various restrictions. If this is not in fact the limit being reported and applied, this is a bug.

[Matthew]

Excellent, thank you for that. Are you able to add the .dat files to Simutranslator to match the additional strings that you have created?

Thank you for the reminder! Yes, I have done this now.

In any event, I have now merged this.

Thank you for the swift response!

To answer your question, the intended behaviour is that the speed limit is indeed the lowest of the various restrictions. If this is not in fact the limit being reported and applied, this is a bug.

I will start a new thread then.

[jamespetts]

Unfortunately, this topic had attracted some unacceptable behaviour, which has had to be dealt with (see here for details). The posts dealing with that have been split so as not to detract from discussion of the substantive issue.

The relevant part of the discussion in the moved messages is as follows.

A road closure and a speed limit are different. Therefore, the conditional branch created by Ves is not a bad thing.
What's important is that a notice indicating that the road is not passable should have been displayed before the conditional branch.
That is to say, in my country we don't put up signs saying "0km/h speed limit" to indicate a road closure.
This means that the speed limit will not be displayed when the road is closed.

I agree that a road that is impassible should state as such and not give a "0km/h speed limit". Does the code as now incorporated from Matthew's patch show a "0km/h speed limit" on impassible ways? If so, this will need to be changed.

[Matthew]

A road closure and a speed limit are different. Therefore, the conditional branch created by Ves is not a bad thing.
What's important is that a notice indicating that the road is not passable should have been displayed before the conditional branch.
That is to say, in my country we don't put up signs saying "0km/h speed limit" to indicate a road closure.
This means that the speed limit will not be displayed when the road is closed.

I agree that a road that is impassible should state as such and not give a "0km/h speed limit". Does the code as now incorporated from Matthew's patch show a "0km/h speed limit" on impassible ways? If so, this will need to be changed.

Thank you for the constructive criticism. If you look at the screenshot in the original post, at the top of this thread, you will see that the previous code already used the "0 km/h" format (in three places). I have not changed that code. If you look at the Expected Results of the original post, then you can see that the aim was only to create a more accurate warning at the bottom of the Way Info Window. These issues are obviously closely related though, so this is a great place to discuss all of them.

You can also see that the Way Info Window has two relevant parts. At the top is the container "cont", which shows a summary of information for multiple ways. At the bottom is the tabbed area, which shows detailed information for individual ways in separate tabs. I think they need to be considered separately.

Ranran, you make a good point about road signs. However, the "cont" container does not have the format of a road sign. It has the format of a summary table. We can see this more clearly in the example below:


If we look at cont's column for the stream, we can see that the existing code displays "Max. speed: 0km/h" and "Max. weight: 0t". This might not be the most natural language, but in the context of a table, it makes clear to the player that they can't use vehicles on it. It would be possible to replace the "0km/h" with a new internal text (no_speed_limit?) if the maximum speed is 0. In English, that might be localized as "Max. speed: n/a", where "n/a" is the short form of "not applicable". But not displaying the speed limit row at all would be very complicated, because we still need to display a max speed for other ways (in this case, the road). If we treat one of the ways completely differently, then we lose the benefit of having a summary table. I do not think "Max. speed: n/a" is significantly better than "Max. speed: 0km/h", but it's not a big problem.

What about the tabbed area? This is a much more promising place. And it does already have a code path for impassible ways. Ranran, I guess that is what you are referring to? We can see an example here:


You can see that it displays inserts a warning, "This way cannot be used by any vehicle", for the mothballed tramway track. At the moment, this code path is only taken by mothballed (?and degraded) ways and the same variable is used to control the display of the information in the right-hand column ("Degraded: Jan. 2000"). I don't think we should use this warning, because it does not inform the player that the situation can be changed. If you go back to the Expected Results of the original post, I think that we need to tell players how to solve the problem of rapids.

However, it would be possible to move the blocked slope (rapids) warning here, rather than at the very bottom of the window. I think would also be straightforward to change the "0km/h" to the same "n/a", both in the "Max speed" row of the tab (for both the current and future/post-renewal values) and in the second line of the tab.

Although it's not shown in these screenshots, there's also another part of the window that can show the maximum speed on bridges and tunnels. I would avoid changing this at the moment, because of the other bug that means speed limits are not properly applied in underground water tunnels. That makes it difficult to carry out reliable tests of any changes here.

[RESTRICTED ACCOUNT]

At the very least, it is clear that, as always, my intentions have not been conveyed at all. What I wanted was a better solution.

I didn't read your any extremely long and unnecessary excuse (because nothing has been changed), I did have a quick look at the issue.



Answer in no particular order, bullet points:

Please write code concisely too.

I wonder if it really took so many changes to bring about such a small change -
Keep branching conditions simple as well.
Splitting and multiplying speed_restricted was a foolish change that only added lines of code and made things less readable.
In other words, why change the table structure for such a minor change? It doesn't make sense.

I believe you don't understand what speed_restricted is.

As A. Carlotti once said: Mind the consistency of your code formatting.

I said don't put disparate elements together.

What you're missing is thinking about what expression will be easy to understand from the player's perspective.

Didn't you understand that one of the things I asked for was for the closure to be displayed as a closure?



(Unfortunately, I can see that the code I wrote previously, in my inexperience, is also very bad. I probably shouldn't have written them in the draw(), but this is my fault. However, this is an old design UI, so I was planning to update it someday, but it will never be completed.)

EDIT:
One of the existing bugs that I recently noticed is that the tables in the tabs should display a description.

EDIT2:
I've looked into this code a bit and found your algorithm to be very very stupid...
To determine whether or not gradient speed restrictions exist, simply compare get_topspeed_gradient_2(), get_topspeed_gradient_1(), and get_topspeed(). So the code should be simpler and clearer.
Whoever originally wrote the algorithm just forgot about that condition. You just made a big fuss over such a small thing. That's why I say all you need to do is file a bug report, which is just a few lines.

My advice to you is also to check what variables and functions in the surrounding code are and what they do before making any changes.