Feature: Allow converting track under trains when compatible with the new rail type #8505

LordAro · 2021-01-05T21:57:14Z

Motivation / Problem

Probably fixes #8314

Cherry-picked from JGRennison/OpenTTD-patches@a18ebba with some modifications due to no per-vehicle-type HasVehicleOnPos function

At the request of @markspolakovs ;)

10:51:06 <marks> > Allow converting track type under trains when compatible with the new rail type. fucken POG
10:51:19 <marks> LordAro: why is this not in core :P

Description

Allows converting track under trains when compatible with the new rail type

Limitations

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

The bug fix is important enough to be backported? (label: 'backport requested')
This PR affects the save game format? (label 'savegame upgrade')
This PR affects the GS/AI API? (label 'needs review: Script API')
- ai_changelog.hpp, gs_changelog.hpp need updating.
- The compatibility wrappers (compat_*.nut) need updating.
This PR affects the NewGRF API? (label 'needs review: NewGRF')
- PR must be added to API tracker

frosch123 · 2021-01-05T22:17:24Z

This allows converting elrail to rail, while an elrail train is on the tile.
I think it should check for "poweredness" instead.

How do road/tramtypes behave?

JGRennison · 2021-01-05T22:44:23Z

src/rail_cmd.cpp

+
+	Train *t = Train::From(v);
+	if (HasBit(t->First()->compatible_railtypes, procdata->type) && HasPowerOnRail(t->First()->railtype, procdata->type)) return nullptr;
+	if (rail_bits & TRACK_BIT_WORMHOLE) {


The TRACK_BIT_WORMHOLE masking/etc is specific to the custom bridge heads implementation in my branch, so you can probably simplify this section a bit.

JGRennison · 2021-01-05T23:16:02Z

src/rail_cmd.cpp

+
+	if (v->z_pos > procdata->z) return nullptr;
+	Train *t = Train::From(v)->First();
+	if (HasBit(t->compatible_railtypes, procdata->type) && HasPowerOnRail(t->railtype, procdata->type)) return nullptr;


HasPowerOnRail(t->railtype, procdata->type) is not correct as this only tests the rail type of the first engine.
It's perfectly legal to create mixed power consists of compatible but not necessarily mutually powered rail types.

See the calculation of Train::compatible_railtypes in Train::ConsistChanged.

For testing you also need to note the current value of VRF_EL_ENGINE_ALLOWED_NORMAL_RAIL

Yes, I did wonder. Not as simple as I thought! I'll get there... probably.

TrueBrain · 2023-09-13T08:42:17Z

Any chance of you continuing this patch? It sounds like a lovely idea :)

PeterN · 2023-09-13T08:46:23Z

This already seems possible so I don't know if this is just outdated, or does something different than the existing behaviour?

… new rail type

LordAro · 2024-11-06T21:06:57Z

Guessing a bit. As peter says, this may not be necessary at all

rubidium42 · 2024-12-10T18:23:39Z

src/rail_cmd.cpp

+	};
+
+	if (HasVehicleOnPos(tile, &data, &EnsureNoIncompatibleRailtypeTrainProc)) {
+		return_cmd_error(STR_ERROR_TRAIN_IN_THE_WAY);


return_cmd_error got removed recently.

rubidium42 · 2024-12-10T18:24:03Z

src/rail_cmd.cpp

+		return v; // Non-trains are always in the way
+	}
+
+	const EnsureNoIncompatibleRailtypeTrainOnGroundData *procdata = (EnsureNoIncompatibleRailtypeTrainOnGroundData *)data;


Suggested change

const EnsureNoIncompatibleRailtypeTrainOnGroundData *procdata = (EnsureNoIncompatibleRailtypeTrainOnGroundData *)data;

const EnsureNoIncompatibleRailtypeTrainOnGroundData *procdata = static_cast<EnsureNoIncompatibleRailtypeTrainOnGroundData *>(data);

rubidium42 · 2024-12-10T18:26:57Z

src/rail_cmd.cpp

+		if (v->type == VEH_DISASTER || v->type == VEH_AIRCRAFT) return nullptr; // Don't care about flying things
+		return v; // Non-trains are always in the way


Why would a truck/bus/tram be in the way, and a landed UFO not?
Also, if this returns a vehicle then later on an error about a train being in the way will be given when this could be a truck/bus/tram.

So, might it make sense to just ignore all non-trains? After all, won't a bus/tram be compatible with the new rail type in any case?

rubidium42 · 2024-12-10T18:29:36Z

src/rail_cmd.cpp

+		return v; // Non-trains are always in the way
+	}
+
+	const EnsureNoIncompatibleRailtypeTrainOnTrackBitsData *procdata = (EnsureNoIncompatibleRailtypeTrainOnTrackBitsData *)data;


Suggested change

const EnsureNoIncompatibleRailtypeTrainOnTrackBitsData *procdata = (EnsureNoIncompatibleRailtypeTrainOnTrackBitsData *)data;

const EnsureNoIncompatibleRailtypeTrainOnTrackBitsData *procdata = static_cast<EnsureNoIncompatibleRailtypeTrainOnTrackBitsData *>(data);

rubidium42 · 2024-12-10T18:30:19Z

src/rail_cmd.cpp

+		if (v->type == VEH_DISASTER || v->type == VEH_AIRCRAFT) return nullptr; // Don't care about flying things
+		return v; // Non-trains are always in the way


Why would a truck/bus/tram be in the way, and a landed UFO not?
Also, if this returns a vehicle then later on an error about a train being in the way will be given when this could be a truck/bus/tram.

So, might it make sense to just ignore all non-trains? After all, won't a bus/tram be compatible with the new rail type in any case?

LordAro force-pushed the allow-railtype-convert branch from f2e0c3a to 2d890c6 Compare January 5, 2021 21:58

frosch123 added the preview label Jan 5, 2021

DorpsGek temporarily deployed to preview-pr-8505 January 5, 2021 22:02 Inactive

LordAro force-pushed the allow-railtype-convert branch from 2d890c6 to 25f992b Compare January 5, 2021 22:37

DorpsGek temporarily deployed to preview-pr-8505 January 5, 2021 22:37 Inactive

JGRennison reviewed Jan 5, 2021

View reviewed changes

TrueBrain force-pushed the allow-railtype-convert branch from 25f992b to e184ffc Compare February 3, 2022 18:24

DorpsGek temporarily deployed to preview-pr-8505 February 3, 2022 18:24 Inactive

2TallTyler added the work: needs more work label Oct 27, 2022

TrueBrain removed the preview label Jul 8, 2023

Feature: Allow converting track under trains when compatible with the…

Loading
Loading status checks…

2f66088

… new rail type

LordAro force-pushed the allow-railtype-convert branch from e184ffc to 2f66088 Compare November 6, 2024 21:06

rubidium42 reviewed Dec 10, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Allow converting track under trains when compatible with the new rail type #8505

Feature: Allow converting track under trains when compatible with the new rail type #8505

LordAro commented Jan 5, 2021

frosch123 commented Jan 5, 2021

JGRennison Jan 5, 2021 •

edited

Loading

JGRennison Jan 5, 2021

LordAro Jan 6, 2021

TrueBrain commented Sep 13, 2023

PeterN commented Sep 13, 2023

LordAro commented Nov 6, 2024

rubidium42 Dec 10, 2024

rubidium42 Dec 10, 2024

rubidium42 Dec 10, 2024

rubidium42 Dec 10, 2024

rubidium42 Dec 10, 2024

	const EnsureNoIncompatibleRailtypeTrainOnGroundData procdata = (EnsureNoIncompatibleRailtypeTrainOnGroundData )data;
	const EnsureNoIncompatibleRailtypeTrainOnGroundData procdata = static_cast<EnsureNoIncompatibleRailtypeTrainOnGroundData >(data);

		if (v->type == VEH_DISASTER \|\| v->type == VEH_AIRCRAFT) return nullptr; // Don't care about flying things
		return v; // Non-trains are always in the way

Feature: Allow converting track under trains when compatible with the new rail type #8505

Are you sure you want to change the base?

Feature: Allow converting track under trains when compatible with the new rail type #8505

Conversation

LordAro commented Jan 5, 2021

Motivation / Problem

Description

Limitations

Checklist for review

frosch123 commented Jan 5, 2021

JGRennison Jan 5, 2021 • edited Loading

Choose a reason for hiding this comment

JGRennison Jan 5, 2021

Choose a reason for hiding this comment

LordAro Jan 6, 2021

Choose a reason for hiding this comment

TrueBrain commented Sep 13, 2023

PeterN commented Sep 13, 2023

LordAro commented Nov 6, 2024

rubidium42 Dec 10, 2024

Choose a reason for hiding this comment

rubidium42 Dec 10, 2024

Choose a reason for hiding this comment

rubidium42 Dec 10, 2024

Choose a reason for hiding this comment

rubidium42 Dec 10, 2024

Choose a reason for hiding this comment

rubidium42 Dec 10, 2024

Choose a reason for hiding this comment

JGRennison Jan 5, 2021 •

edited

Loading