Signal Error Message

[Wasila]

This isn't really a problem so I put it here. When you try and connect a piece of track to one with signals on, you can't. Fair enough. But the error message is 'impossible track combination'. The track combination is perfectly fine... this error message bear no relevance to the actual error. How about 'Can't place track here... signals must be removed first,' which is based on the error message you get when building over a building.

[SwissFan91]

I always thought it was saying "impossible track combination" due to the combination of a junction and a signal on the same track piece.

[Wasila]

Well that's not a track combination that's impossible...

[Alberth]

iirc you get the same error also when you try to combine two different railtypes at a single tile. In that case, it seems like an impossible track combination to me

[Wasila]

Yeah... but that's a different error.

[planetmaker]

I agree that Wasila has a point there. It's a different thing if a piece of track cannot be placed as two track types cannot be combined on one tile (that might be called "track combination" but is IMO slightly inaccurate or mis-understandable, too) or if a track cannot be placed as a signal blocks you from building there.

[MrFrans]

I agree, it would be more helpful to players if the error message says it is because of the signal.

[oberhümer]

you get the same error also when you try to combine two different railtypes at a single tile.

This is sometimes pretty annoying, as it also makes it impossible to build a rail type next to a different one diagonally, even though that actually would work perfectly.

[Wasila]

I agree with neg, but with mainlines it's rarely needed. Also, devs, how hard can this be to implement? If I knew where the message was I'd change it myself...

[ChillCore]

This is sometimes pretty annoying, as it also makes it impossible to build a rail type to a different one diagonally, even though that actually would work perfectly

IMHO that is something entirely different and should be reported as being a bug or not yet programmed.

As for the original problem, when building rail it should say "impossible track combination" but maybe extended to say why.(As mentioned before)

EDIT:
Wasila:
You may want to search english.txt for the excact error mesage but It might be that the error message is used multiple times, so then in the other cases it will be incorrect.
In that case a new string needs to be added and the code itself needs to be changed.

[Wasila]

I found:

STR_ERROR_IMPOSSIBLE_TRACK_COMBINATION :{WHITE}Impossible track combination

So I would need to create a new string and change something somewhere else to refer to it?

[ChillCore]

Those files starting with rail is maybe somewhere to start?
Search for the part before ":". Try to find out if it is used once or multiple times.
If multiple times you need to make a new string for example:
STR_ERROR_IMPOSSIBLE_TRACK_SIGNAL_COMBINATION :{WHITE}Impossible track combination because a signal is in the way.
Then you use the first part of the new string in the correct place instead of the old one.
You may need to replace some more code if the part where the "check for this condition" is checks for multiple conditions at once.
Good luck and take your time.

[Wasila]

Found the string twice in rail_cmd:

Code: Select all

static bool CheckTrackCombination(TileIndex tile, TrackBits to_build, uint flags)
{
	_error_message = [b]STR_ERROR_IMPOSSIBLE_TRACK_COMBINATION;[/b]

	if (!IsPlainRail(tile)) return false;

	/* So, we have a tile with tracks on it (and possibly signals). Let's see
	 * what tracks first */
	TrackBits current = GetTrackBits(tile); // The current track layout.
	TrackBits future = current | to_build;  // The track layout we want to build.

	/* Are we really building something new? */
	if (current == future) {
		/* Nothing new is being built */
		_error_message = STR_ERROR_ALREADY_BUILT;
		return false;
	}

	/* Let's see if we may build this */
	if ((flags & DC_NO_RAIL_OVERLAP) || HasSignals(tile)) {
		/* If we are not allowed to overlap (flag is on for ai companies or we have
		 * signals on the tile), check that */
		return future == TRACK_BIT_HORZ || future == TRACK_BIT_VERT;
	} else {
		/* Normally, we may overlap and any combination is valid */
		return true;
	}
}

Code: Select all

switch (GetTileType(tile)) {
		case MP_RAILWAY: {
			if (!CheckTileOwnership(tile)) return CMD_ERROR;

			if (!IsPlainRail(tile)) return CMD_ERROR;

			if (!IsCompatibleRail(GetRailType(tile), railtype)) return_cmd_error([b]STR_ERROR_IMPOSSIBLE_TRACK_COMBINATION[/b]);

			if (!CheckTrackCombination(tile, trackbit, flags) ||
					!EnsureNoTrainOnTrack(tile, track)) {
				return CMD_ERROR;
			}

[ChillCore]

As you can see the function for checking if there is a signal on the rail is in the first block:

Code: Select all

   /* Let's see if we may build this */
   if ((flags & DC_NO_RAIL_OVERLAP) || HasSignals(tile)) {
      /* If we are not allowed to overlap (flag is on for ai companies or we have
       * signals on the tile), check that */
      return future == TRACK_BIT_HORZ || future == TRACK_BIT_VERT;
      ...

But it also checks for DC_NO_RAIL_OVERLAP sepperated from HasSignals by "or" = "||".
We are only interested in the signal part since that is what we want to change.(We as in he who wants to change it.)
The first thing to do is to seperate the checks so they can behave differently.
That means instead of

Code: Select all

if ((condition) || (condition)) {
   do something;
} else {
   do something else;
}

You want your code to look something like this:

Code: Select all

if (condition) {
   behave like before;
} else if (condition) {
   _error_message = my new string;
   behave like before;
} else {
   behave like before;
}

Now compile and see if the code behaves the way you want.
If it does, you have just written your first patch.
If it does not yet, you may have to dig a bit deeper in the code.

Also there are some comments surrounding the code. You may want to change those for them to stay correct.
Now that you have come this far you may want to download the docs, you can find them near the binaries on openttd.org.
Inside you will find all sorts of information, for example what DC_NO_RAIL_OVERLAP means and much more.(Start with index.html)

ps:
No there is no easier way and it is what Devs and patchwriters do all the time to fix stuff or to make feature requests happen.

EDIT:
Corrected sample code to not confuse he who follows. please read below.

[Wasila]

Perhaps unsurprisingly, I have encountered a problem. This is what I did:

Code: Select all

   /* Let's see if we may build this */
   if (flags & DC_NO_RAIL_OVERLAP) {
      /* If we are not allowed to overlap (flag is on for ai companies or we have
       * signals on the tile), check that */
      return future == TRACK_BIT_HORZ || future == TRACK_BIT_VERT;
   } else if (HasSignals(tile)) {
      return future == TRACK_BIT_HORZ || future == TRACK_BIT_VERT;
      _error_message = STR_ERROR_MUST_REMOVE_SIGNALS;
   } else {
      /* Normally, we may overlap and any combination is valid */
      return true;
   }

Of course, I haven't changed the comments yet. But the point is, while the game compiled successfully, nothing has changed in-game. The version number does have an 'M' on the end, but I still get the same error message. What have I done wrong?

Also I've found another problem. What do I do about the other language files? Do I just leave the strings out? Or is there something I have to put in?

Also I couldn't find the docs on the website.

[Yexo]

Perhaps unsurprisingly, I have encountered a problem. This is what I did:

Code: Select all

      return future == TRACK_BIT_HORZ || future == TRACK_BIT_VERT;
      _error_message = STR_ERROR_MUST_REMOVE_SIGNALS;

Due to the "return" on the first line the second line is never executed. Try reversing those lines.

Also I couldn't find the docs on the website.

[/quote]Try http://docs.openttd.org/, you can view them online.

[Wasila]

Thanks Yexo, it works!

Since the patch is so small, I was wondering how much I need to upload. Does it need to be the source code, the compiled folder or just the necessary files?

As said, I don't know how to deal with other translations either.

[petert]

Since the patch is so small, I was wondering how much I need to upload.

Just upload a diff created with "svn diff > file.diff".

[ChillCore]

Thanks Yexo, it works!

Cool. You also may want to ask for your thread to be moved to development as it is no longer a suggestion.
Congratulations.

[Roujin]

I guess it no longer needs to be moved, because it was implemented in r19190.