Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Close adjacent level crossings as if they were one large crossing #8444

Draft
wants to merge 2 commits into
base: master
from

Conversation

@Eddi-z
Copy link
Contributor

@Eddi-z Eddi-z commented Dec 27, 2020

Motivation / Problem

Level crossings on parallel tracks can be dangerous, especially for articulated road vehicles, or if multiple road vehicles follow each other. Often, a vehicle is waiting for a level crossing to open, when it suddenly gets struck in its "tail" by another train.

Description

Updated patch from https://www.tt-forums.net/viewtopic.php?f=33&t=46091

This patch tries to avoid the situation that a vehicle gets stuck inbetween two tracks by closing all level crossings simultaneously, and instead allowing road vehicles to leave a crossing they already entered. Best combined with a path signal some tiles ahead of a level crossing, to give vehicles time to leave if a train approaches.

Limitations

on loading an old savegame, road vehicles that were previously waiting for a crossing to open, may now immediately start up, causing them to run straight into a train. some savegame conversion update is needed.

also, a call to SndPlayTileFx(SND_0E_LEVEL_CROSSING, tile); fell off and i don't remember where it went, or why i removed it.
Update: level crossing sound is now changed to only appear when a train approaches the crossing, instead of sometimes when it is being closed.

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')
@DorpsGek DorpsGek temporarily deployed to preview-pr-8444 Dec 27, 2020 Inactive
@michicc
Copy link
Member

@michicc michicc commented Dec 27, 2020

Your missing SndPlayTileFx seems to be done in UpdateLevelCrossingTile. I haven't tried it yet, but I think it will now play a sound for each crossing tile that is closed, which should probably be reduced to only the triggering tile.

@Eddi-z
Copy link
Contributor Author

@Eddi-z Eddi-z commented Dec 27, 2020

I vaguely remember that there was a second source of that sound, when a train was approaching a crossing tile

@TrueBrain TrueBrain added this to the 1.11.0 milestone Jan 5, 2021
@TrueBrain TrueBrain changed the title Feature: Close adjacent level crossings as if they were one large cro… Feature: Close adjacent level crossings as if they were one large crossing Jan 6, 2021
Copy link
Member

@TrueBrain TrueBrain left a comment

I kinda stopped my review half-way through. Please spend some more time on code quality here. So many things that appear to be in a half-state, that it made me check if this was a draft PR (it was not).

src/road_cmd.cpp Outdated Show resolved Hide resolved
src/road_cmd.cpp Outdated Show resolved Hide resolved
src/train_cmd.cpp Outdated Show resolved Hide resolved
src/train_cmd.cpp Outdated Show resolved Hide resolved
src/train_cmd.cpp Outdated Show resolved Hide resolved
src/train_cmd.cpp Outdated Show resolved Hide resolved
src/train_cmd.cpp Outdated Show resolved Hide resolved
@Eddi-z
Copy link
Contributor Author

@Eddi-z Eddi-z commented Jan 6, 2021

it made me check if this was a draft PR (it was not).

it probably should be.

@Eddi-z Eddi-z marked this pull request as draft Jan 6, 2021
@TrueBrain TrueBrain removed this from the 1.11.0 milestone Jan 6, 2021
@Eddi-z Eddi-z force-pushed the Eddi-z:level branch from 3b0d6bc to 009e960 Jan 9, 2021
@DorpsGek DorpsGek temporarily deployed to preview-pr-8444 Jan 9, 2021 Inactive
@Eddi-z Eddi-z force-pushed the Eddi-z:level branch from 009e960 to 80e02db Jan 9, 2021
@DorpsGek DorpsGek temporarily deployed to preview-pr-8444 Jan 9, 2021 Inactive
@Eddi-z Eddi-z force-pushed the Eddi-z:level branch from 80e02db to c3e28c5 Jan 10, 2021
@DorpsGek DorpsGek temporarily deployed to preview-pr-8444 Jan 10, 2021 Inactive
@Eddi-z Eddi-z force-pushed the Eddi-z:level branch from c3e28c5 to d16257a Jan 10, 2021
@DorpsGek DorpsGek temporarily deployed to preview-pr-8444 Jan 10, 2021 Inactive
@Eddi-z Eddi-z force-pushed the Eddi-z:level branch from d16257a to f6e26d6 Jan 10, 2021
@DorpsGek DorpsGek temporarily deployed to preview-pr-8444 Jan 10, 2021 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-8444 Jan 10, 2021 Inactive
@Eddi-z
Copy link
Contributor Author

@Eddi-z Eddi-z commented Jan 10, 2021

i revisited the crossing sound effect. before this PR, crossing sound was played in two different ways

a) when a crossing without a PBS nearby gets closed
b) when a crossing with PBS is approached by a train

case a) was a bit awkward in my patch, since that might result in multiple crossing sounds being played at once, so i opted instead to drop case a) and change case b) to be in all cases

i opted to do this in a separate commit, in case it was somehow controversial

…nstead of sometimes on closing
@Eddi-z Eddi-z force-pushed the Eddi-z:level branch from d61c24a to ab5806b Jan 15, 2021
@DorpsGek DorpsGek temporarily deployed to preview-pr-8444 Jan 15, 2021 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants