Auto-remove signal, good idea or not?

[Kuhnovic]

Hi all! I recently got completely hooked on OpenTTD (something to do with the times we're in I guess). I love this game, but there's one thing in particular that bothers me. When I have a section of straight rail track, and I want another section of track to (diagonally) merge into it, I hit a signal, and I get the "Must remove signal first" message. More often than not it seems, Murphy's Law certainly seems to apply.

Wouldn't it be great to have a setting that allows you to automatically remove the signal in such a case?

I am a software developer by trade, so I couldn't resist and have actually hacked this together already. I've already made it work in such a way that it will only delete signals when you're branching off or crossing another piece of track. Extending a piece of track by "dragging a line through it" leaves the signals in place. IMO it's a great feature, but before I tidy it up and make it bulletproof (and so that it can be turned on or off), I want to know if it's worth proceeding. If the idea gets show down right away, there's not much of a point really

P.S. I wasn't sure if I should post it here or in the development subforum. Feel free to move it mods

[MagicBuzz]

Murphy loves my rail network too
https://youtu.be/zGyThu7EAHQ

I like your idea.

May it could be upgraded to supply signals before and after the junction automatically, but the way they are placed could vary from one person to another, so it might be a not so good idea finaly...

[jfs]

Nah, removing signals is inherently dangerous. I don't think having the option to auto-remove them by building over is a good idea, there's too many things that can go wrong. (Typically trains crashing or getting stuck.) Requiring the player to switch tool and actively remove signals, and/or make a conscious choice of whether to merge/cross the tracks somewhere else, is a good kind of active confirmation of a potentially dangerous action here.

[_dp_]

Thing is, 99.99% of time removing signal in such way is safe and it's small things like that which make building very time-consuming and annoying. IMO it should definitely be added as either an option or two-click process (first signal second rail) or smth. If anything I plan to add it to CityMania client eventually now that I got into changing building tools anyway.

[SciFurz]

Thing is, 99.99% of time removing signal in such way is safe and it's small things like that which make building very time-consuming and annoying. IMO it should definitely be added as either an option or two-click process (first signal second rail) or smth. If anything I plan to add it to CityMania client eventually now that I got into changing building tools anyway.

I agree with popping up a warning and giving a choice to remove the signal or not.
This thing usually happens when I expand a rail station or yard, and often removing a signal before adding a replacement causes trouble with trains suddenly thinking a path is free.

[Kuhnovic]

Thanks for the feedback! This is indeed intended as a solution to the 99.99% times this is annoying. I agree this can be potentially dangerous, therefore it should be something that's off by default, so people turn it on very deliberately.

I will also think of ways to minimize the risk involved. Some ideas:

1. Don't remove signals if you're creating track that's already present (Already implemented). This is necessary to avoid signals being removed when you quickly extend a piece of track, and you accidentally drag partly through the existing track.
2. Only remove a maximum of one signal
3. Only remove a signal if you start or end dragging on it
4. Adding a popup to confirm the removal (thanks SciFurz)

Of course one has to pay attention to create a function that doesn't involve too much "magic". It should be intuitive to work with and require little to no explanation.

[MagicBuzz]

I think this should be :
- shortcut + click : but currently SHIFT is for cost and CTRL is for remove...
- another construction button : like the polyform button, a new button should be "build over signals"

But please, don't restrict it to "one signal" or "ending track on the signal", this will make the feature useless.

[Kuhnovic]

Here's a patch of the initial version. This is still the "unsafe" version that doesn't use hotkeys or warning windows, it just deletes the signal when it's in the way. I added a setting to the Company section, it's off by default so you have to enable it first:

setting.png (27.78 KiB) Viewed 4287 times

Here's a GIF of me happily mowing down signals. Notice how they are only removed if they are in the way. When dragging to extend existing track, the signals stay where they are.



Learning about / fighting with the intricacies for the rail and signal system, that was a fun challenge

[LaChupacabra]

In my opinion it's a very good idea and... not just an idea. It will be very useful. Situations when removing signaling by building a new track will cause an accident would be rather rare. This possibility could be limited so that only clicking on this track with signaling (or near tile) could remove it, and dragging the track from another point would not. Or better: to remove the signal is not possible if there is a train on the adjacent section - in this case adding the option enabling / disabling this function would be unnecessary, because removing the signal would not pose a threat. However, showing a warning and having to acknowledge this would not be the best.

Btw. Generally, after the changes that were introduced probably in version 1.9, which concerned the pathfinder for trains, today it is much easier to cause an accident than before. The trains reserve roads too much in advance, which means that even after covering a very long distance, they can pass another pathsignal and collide with another train. In addition, they don't respond to a change in orders. It was hardly the best change.

[Eddi]

looking at the video, i'll see myself getting annoyed at accidentally overbuilding the wrong signal more often than intending to overbuild a signal, let alone the possibility of crashing trains.

that means i'd rather keep this feature off. going to signal+remove mode is only 2 key presses, and going back to build track mode is 1 more.

that is, of course, only my personal opinion.

[mak]

A good idea for those that need, require it but not for me.

When building tracks I try to ensure that the joining track connects between existing signals, I do have five to ten squares between signals and if I do need to remove a signal then doing it manually ensures I check a collsion between trains will not happen.

[Kuhnovic]

Keep the good feedback coming guys, I love it. I totally understand that this is not for everybody. I always work with a signal interval of 2, which makes the signal-in-the-way-problem very frequent and annoying. If you work with the default 4 or larger, it's not as big of an issue. If you're into logic circuits then you probably want this feature off. That's also why I made it off by default, I don't want to disrupt the default gameplay in any way.

I'm going to field test the feature tonight with a couple of friends. That might give some additional insights or unearth any bugs. Exciting!

[Alberth]

You may want to keep an eye to how often you have trains running after each other at a 2 tile distance.
My guess is, not very often.

[MagicBuzz]

Even with the standard distance of 4 (well, 3 free spaces between signals) this happend very often.

This feature is definately usefull from my point of view.

[2TallTyler]

Is it possible to check if a train is approaching or waiting at the signal, to decide whether to auto-remove the signal?

[Kuhnovic]

Is it possible to check if a train is approaching or waiting at the signal, to decide whether to auto-remove the signal?

Its probably possible, but this would add some degree of magic behaviour to the function. I'm a bit hesitant to these kind of things, because they make the function complex (in terms of code), and also hard to understand what it really does. It is a fun idea though, it's going on the list

[Kuhnovic]

Ok, I've been testing this with some friends. The feature itself works really well in single player, but in multiplayer it causes desyncs. The horror! The good news is, with the setting off, everything works fine and there are no desyncs. So at least I haven't completely ruined the game

I have been reading up on desyncs a bit. Unfortunately I can't really see how my code could cause any nondeterministic behaviour. But since there are people far more experienced than I am on this forum, somebody might be able to spot it right away. This is essentially the code that does the work, it's part of the CmdBuildSingleRail command in rail_cmd.cpp:

Code: Select all

if (HasSignals(tile)) {
	if (TracksOverlap(GetTrackBits(tile) | TrackToTrackBits(track))) {
		/* If adding the new track causes any overlap, all signals must be removed first */
		if (_settings_game.construction.auto_remove_signals) {
			for (Track track_it = TRACK_BEGIN; track_it < TRACK_END; track_it++) {
				if (HasTrack(tile, track_it) && HasSignalOnTrack(tile, track_it)) {
					CommandCost ret_remove_signals = DoCommand(tile, track_it, 0, flags, CMD_REMOVE_SIGNALS);
					if (ret_remove_signals.Failed()) return ret_remove_signals;
					cost.AddCost(ret_remove_signals);
				}
			}
		}
		else
		{
			return_cmd_error(STR_ERROR_MUST_REMOVE_SIGNALS_FIRST);
		}
	}
}

The general idea of the code: the moment adding new track causes an overlap within the tile, all signals on that tile have to removed. There can be up to 2 signals per tile, hence the for loop.

[JGR]

Ok, I've been testing this with some friends. The feature itself works really well in single player, but in multiplayer it causes desyncs. The horror! The good news is, with the setting off, everything works fine and there are no desyncs. So at least I haven't completely ruined the game

I have been reading up on desyncs a bit. Unfortunately I can't really see how my code could cause any nondeterministic behaviour. But since there are people far more experienced than I am on this forum, somebody might be able to spot it right away. This is essentially the code that does the work, it's part of the CmdBuildSingleRail command in rail_cmd.cpp:

code.png

The general idea of the code: the moment adding new track causes an overlap within the tile, all signals on that tile have to removed. There can be up to 2 signals per tile, hence the for loop.

Code: Select all

+[SDT_BOOL]
+base     = GameSettings
+var      = construction.auto_remove_signals
+flags    = SLF_NOT_IN_SAVE | SLF_NO_NETWORK_SYNC
+def      = false
+str      = STR_CONFIG_SETTING_AUTO_REMOVE_SIGNALS
+strhelp  = STR_CONFIG_SETTING_AUTO_REMOVE_SIGNALS_HELPTEXT

This (from the patch you posted earlier) is definitely wrong.

If the setting has different values between the server and any clients, use of the feature will inevitably result in desyncs.

Also, posting code as screenshots is not very helpful. In general it is advisable to use text.

[Kuhnovic]

Also, posting code as screenshots is not very helpful. In general it is advisable to use text.

Edited my previous post

Code: Select all

+[SDT_BOOL]
+base     = GameSettings
+var      = construction.auto_remove_signals
+flags    = SLF_NOT_IN_SAVE | SLF_NO_NETWORK_SYNC
+def      = false
+str      = STR_CONFIG_SETTING_AUTO_REMOVE_SIGNALS
+strhelp  = STR_CONFIG_SETTING_AUTO_REMOVE_SIGNALS_HELPTEXT

This (from the patch you posted earlier) is definitely wrong.

If the setting has different values between the server and any clients, use of the feature will inevitably result in desyncs.

The reason for adding behind SLF_NO_NETWORK_SYNC was that one player could have this off and the other on, according to preference. But I guess that just means that the command is executed differently depending on the local setting, and that in turn causes the desync?

[JGR]

The reason for adding behind SLF_NO_NETWORK_SYNC was that one player could have this off and the other on, according to preference. But I guess that just means that the command is executed differently depending on the local setting, and that in turn causes the desync?

Yes, you're testing the value of _settings_game.construction.auto_remove_signals in the command execution, and this is run on the server and all clients.

Making it a local setting is a bit more fiddly. You would need to move it to the .gui section (struct GUISettings), and adjust the parameters of the DoCommandP call from the client depending on the value of the setting.
e.g. take a look at _settings_client.gui.drag_signals_density and _settings_client.gui.drag_signals_fixed_distance.