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.

Sign up for GitHub

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

Jump to bottom

Feature: Extended depots #8480

Draft
wants to merge 78 commits into
base: master
Choose a base branch
from
Draft

Feature: Extended depots #8480

wants to merge 78 commits into from
+5,951 −1,370

Conversation

J0anJosep
Copy link
Contributor

@J0anJosep J0anJosep commented Jan 2, 2021 •
edited
Loading

(Depends on #9577)

Motivation / Problem

It is strange a train with a length of 6 tiles can get out from a 1-tile depot.
Or that you can have 300 road vehicles in a single tile depot.
Or that 300 ships can be stopped in the same depot tile.

Description

This PR introduces a new depot tile for rail, road and water transport: extended depots.

Standard depots are the legacy depots of OpenTTD. They should work the same as before.

Extended depots have limitations related to the amount of vehicles they can handle. It is possible to join extended depots with other extended or standard depots.

How to use it:

  • In settings, you can choose which depot types the human players can use (search "depot types").

Limitations

This is for testing purpose.
There are several things to be improved and I need feedback: wording, functionality, implementation,...

It needs testing and feedback.
Please, expect some crashes. Report them, please.
Try also to crash it in:

  • Multiplayer using NewGRFs
  • Replacing/autorenewing/autoreplacing

To consider

  • Consider which new graphics are needed and possible NewGRF support.
  • Platform length for stations and depots can be stored in the map array, so pathfinders can benefit from it. (I consider it out of scope by now but it could be done.)
  • Related with previous item: Station::GetPlatformLength is changed to a generic GetPlatformLength. I am not really sure if this is the way to go. Help will be appreciated in this one.
  • Hangars: airports with multiple hangars currently have different depot windows for different hangar tiles and you can build aircraft in each of those tiles; with the current implementation of multi-tile depots, the hangars of an airport share the same window and newly built aircraft is placed only in the first hangar. This works, but it is not the current vanilla behavior. Check with the community/developers whether this is acceptable or not. (Having in mind tileable airports, the new behavior may be a better approach than the current one.)
  • How these changes affect AI API? Right now AI are able to build small depots, independently from the settings. Keeping this behavior is the easiest way to deal with AIs and big depots. Exposing big depots to AI is not an easy task, as they should deal with cross-replacements and max train length in platform depot.

Checklist for review

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

  • 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')

Binaries of this branch are available at https://www.openttd.org/downloads/openttd-branches/pr8480/latest.html . They might not be up-to-date with latest version of this PR, so check the date!

@TrueBrain TrueBrain added preview This PR is receiving preview builds size: extra large This Pull Request is very large; it properly needs to be split up before it can be properly reviewed work: still in progress (draft) This Pull Request is a draft labels Jan 2, 2021
@DorpsGek DorpsGek temporarily deployed to preview-pr-8480 January 2, 2021 19:30 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-8480 January 2, 2021 19:36 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-8480 January 2, 2021 20:40 Inactive
@TrueBrain TrueBrain added the candidate: needs considering This Pull Request needs more opinions label Jan 5, 2021
@DorpsGek DorpsGek temporarily deployed to preview-pr-8480 January 7, 2021 22:10 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-8480 January 7, 2021 22:34 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-8480 January 8, 2021 10:56 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-8480 January 9, 2021 16:48 Inactive
@Wolfolo
Copy link

Wolfolo commented Jan 9, 2021

Really easy to use, the ro-ro capability like station tiles is something I really needed in some games 😁

The only downside: after trying it I found that it's missing a sort of warning when you purchase too many wagons in a depot (you get it when trying to start the train), since we have the measurement lines in depot view consider to highlight the current depot max size.

This patch will be amazing with a future grf support.

@DorpsGek DorpsGek temporarily deployed to preview-pr-8480 January 17, 2021 15:42 Inactive
@J0anJosep
Copy link
Contributor Author

The only downside: after trying it I found that it's missing a sort of warning when you purchase too many wagons in a depot (you get it when trying to start the train), since we have the measurement lines in depot view consider to highlight the current depot max size.

Internally, it is difficult to know when adding a new wagon will make the train too long. There may be more than one platform available, and they may have different lengths and rail types. So, the only help you have now is to press X and see what happens in the depot when buying or moving vehicles.

@embeddedt
Copy link
Contributor

embeddedt commented Jan 25, 2021 •
edited
Loading

This feature would be excellent to have in OpenTTD.

I gave this a try in the web version. I get a crash with the following depot layout and trains:

screenshot_1

This is the crash log in DevTools. Unfortunately the backtrace looks a bit incomplete; I can compile a debug version on Linux and see if that gives a more coherent log if that would be helpful.

(index):130 �[1;31mError: Assertion failed at line 745 of /__w/OpenTTD/OpenTTD/src/command.cpp: res.GetCost() == res2.GetCost() && res.Failed() == res2.Failed()�[0;39m
printErr @ (index):130
put_char @ openttd.js:2599
write @ openttd.js:2532
write @ openttd.js:4151
doWritev @ openttd.js:4922
_fd_write @ openttd.js:10470
__stdio_write @ openttd.wasm:0x6a4245
__vfprintf_internal @ openttd.wasm:0x6a1c01
vfiprintf @ openttd.wasm:0x6a3417
fiprintf @ openttd.wasm:0x6a3691
error(char const*, ...) @ openttd.wasm:0x4331e5
DoCommandPInternal(unsigned int, unsigned int, unsigned int, unsigned int, void (*)(CommandCost const&, unsigned int, unsigned int, unsigned int, unsigned int), char const*, bool, bool) @ openttd.wasm:0x2b7ba0
DoCommandP(unsigned int, unsigned int, unsigned int, unsigned int, void (*)(CommandCost const&, unsigned int, unsigned int, unsigned int, unsigned int), char const*, bool) @ openttd.wasm:0x2b6ff1
VehicleViewWindow::OnClick(Point, int, int) @ openttd.wasm:0x5ce98d
HandleMouseEvents() @ openttd.wasm:0x5fdb3b
VideoDriver_SDL::EmscriptenLoop(void*) @ openttd.wasm:0x27f345
browserIterationFunc @ openttd.js:9986
runIter @ openttd.js:6627
Browser_mainLoop_runner @ openttd.js:6566
requestAnimationFrame (async)
requestAnimationFrame @ openttd.js:6948
(rAF repeated)
Browser_mainLoop_scheduler_rAF @ openttd.js:6480
Browser_mainLoop_runner @ openttd.js:6577
(index):130 undefined
printErr @ (index):130
abort @ openttd.js:1506
_abort @ openttd.js:6430
error(char const*, ...) @ openttd.wasm:0x4331f6
DoCommandPInternal(unsigned int, unsigned int, unsigned int, unsigned int, void (*)(CommandCost const&, unsigned int, unsigned int, unsigned int, unsigned int), char const*, bool, bool) @ openttd.wasm:0x2b7ba0
DoCommandP(unsigned int, unsigned int, unsigned int, unsigned int, void (*)(CommandCost const&, unsigned int, unsigned int, unsigned int, unsigned int), char const*, bool) @ openttd.wasm:0x2b6ff1
VehicleViewWindow::OnClick(Point, int, int) @ openttd.wasm:0x5ce98d
HandleMouseEvents() @ openttd.wasm:0x5fdb3b
VideoDriver_SDL::EmscriptenLoop(void*) @ openttd.wasm:0x27f345
browserIterationFunc @ openttd.js:9986
runIter @ openttd.js:6627
Browser_mainLoop_runner @ openttd.js:6566
requestAnimationFrame (async)
requestAnimationFrame @ openttd.js:6948
(rAF repeated)
Browser_mainLoop_scheduler_rAF @ openttd.js:6480
Browser_mainLoop_runner @ openttd.js:6577
(index):130 exception thrown: RuntimeError: abort(undefined). Build with -s ASSERTIONS=1 for more info.,RuntimeError: abort(undefined). Build with -s ASSERTIONS=1 for more info.
    at abort (https://preview.openttd.org/pr8480/openttd.js:1516:11)
    at _abort (https://preview.openttd.org/pr8480/openttd.js:6430:7)
    at error(char const*, ...) (https://preview.openttd.org/pr8480/openttd.wasm:wasm-function[5898]:0x4331f6)
    at DoCommandPInternal(unsigned int, unsigned int, unsigned int, unsigned int, void (*)(CommandCost const&, unsigned int, unsigned int, unsigned int, unsigned int), char const*, bool, bool) (https://preview.openttd.org/pr8480/openttd.wasm:wasm-function[3779]:0x2b7ba0)
    at DoCommandP(unsigned int, unsigned int, unsigned int, unsigned int, void (*)(CommandCost const&, unsigned int, unsigned int, unsigned int, unsigned int), char const*, bool) (https://preview.openttd.org/pr8480/openttd.wasm:wasm-function[3778]:0x2b6ff1)
    at VehicleViewWindow::OnClick(Point, int, int) (https://preview.openttd.org/pr8480/openttd.wasm:wasm-function[7626]:0x5ce98d)
    at HandleMouseEvents() (https://preview.openttd.org/pr8480/openttd.wasm:wasm-function[7862]:0x5fdb3b)
    at VideoDriver_SDL::EmscriptenLoop(void*) (https://preview.openttd.org/pr8480/openttd.wasm:wasm-function[3440]:0x27f345)
    at browserIterationFunc (https://preview.openttd.org/pr8480/openttd.js:9986:66)
    at Object.runIter (https://preview.openttd.org/pr8480/openttd.js:6627:13)
printErr @ (index):130
runIter @ openttd.js:6634
Browser_mainLoop_runner @ openttd.js:6566
requestAnimationFrame (async)
requestAnimationFrame @ openttd.js:6948
(rAF repeated)
Browser_mainLoop_scheduler_rAF @ openttd.js:6480
Browser_mainLoop_runner @ openttd.js:6577
openttd.js:1516 Uncaught RuntimeError: abort(undefined). Build with -s ASSERTIONS=1 for more info.
    at abort (https://preview.openttd.org/pr8480/openttd.js:1516:11)
    at _abort (https://preview.openttd.org/pr8480/openttd.js:6430:7)
    at error(char const*, ...) (https://preview.openttd.org/pr8480/openttd.wasm:wasm-function[5898]:0x4331f6)
    at DoCommandPInternal(unsigned int, unsigned int, unsigned int, unsigned int, void (*)(CommandCost const&, unsigned int, unsigned int, unsigned int, unsigned int), char const*, bool, bool) (https://preview.openttd.org/pr8480/openttd.wasm:wasm-function[3779]:0x2b7ba0)
    at DoCommandP(unsigned int, unsigned int, unsigned int, unsigned int, void (*)(CommandCost const&, unsigned int, unsigned int, unsigned int, unsigned int), char const*, bool) (https://preview.openttd.org/pr8480/openttd.wasm:wasm-function[3778]:0x2b6ff1)
    at VehicleViewWindow::OnClick(Point, int, int) (https://preview.openttd.org/pr8480/openttd.wasm:wasm-function[7626]:0x5ce98d)
    at HandleMouseEvents() (https://preview.openttd.org/pr8480/openttd.wasm:wasm-function[7862]:0x5fdb3b)
    at VideoDriver_SDL::EmscriptenLoop(void*) (https://preview.openttd.org/pr8480/openttd.wasm:wasm-function[3440]:0x27f345)
    at browserIterationFunc (https://preview.openttd.org/pr8480/openttd.js:9986:66)
    at Object.runIter (https://preview.openttd.org/pr8480/openttd.js:6627:13)

@J0anJosep
Copy link
Contributor Author

This feature would be excellent to have in OpenTTD.

I gave this a try in the web version. I get a crash with the following depot layout and trains...

Thanks for the report. That is enough to find the bug. I am still working to fix it and some other things.

@DorpsGek DorpsGek temporarily deployed to preview-pr-8480 February 14, 2021 21:38 Inactive
@DorpsGek DorpsGek temporarily deployed to preview-pr-8480 February 14, 2021 22:13 Inactive
@reldred
Copy link

reldred commented Feb 18, 2021 •
edited
Loading

Hi @J0anJosep, immediate crash in main menu clicking the settings menu in https://preview.openttd.org/pr8480/ or in-game.

Edit: Nevermind, I've been informed this was an upstream bug. Good luck with the PR I'm looking forward to it!

@DorpsGek DorpsGek temporarily deployed to preview-pr-8480 February 19, 2021 18:54 Inactive
@J0anJosep
Copy link
Contributor Author

I get a crash every time I try to open the "Build Roads" or "Build Ship Docks" toolbars.

That was fixed some weeks ago. If you are using the binaries, they are outdated. I will ask next weekend Truebrain whether he can update them, because first I have some fixes for the PR that need to be pushed.

J0anJosep added 28 commits July 9, 2024 21:07
@J0anJosep
Add: Code dealing with extended rail depot platforms.
7ab28f6
@J0anJosep
Change: Rename end segment reason for general platforms.
fe1b0f1
@J0anJosep
Add: Add IsAnyDepotTileReserved.
30a62aa
@J0anJosep
Add: Reservation and penalties for extended depots.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
419ca91
@J0anJosep
Add: Add vehicle entered depot platform flag.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
ac55c26
@J0anJosep
Add: Use track depot for detecting trains in depots.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
d489e22
@J0anJosep
Change: Modify IsChainInDepot for ground vehicles.

Unverified

This user has not yet uploaded their public signing key.
GPG key ID: A048098B57B02178
Learn about vigilant mode
421a533
@J0anJosep
Add: Add a ShouldStopAtDepot function.
2b25944
@J0anJosep
Change: Adapt VehicleEnter_Track and handle trains entering extended …
b6946fb 
…depots.

# Conflicts:
#	src/vehicle.cpp
@J0anJosep
Add: Check length of platforms for servicing trains.
fc5445d
@J0anJosep
Change: Update signals for extended rail depots.
12862b5
@J0anJosep
Change: Changes related with crashed trains in extended depots.
d4e89ad
@J0anJosep
Add: Allow trains to reverse when leaving an extended depot.
0e34a5a
@J0anJosep
Add: Base files for placing trains in extended depots.
0619cb6
@J0anJosep
Change: Check whether to stay in an extended rail depot or place the …
dfe6be5 
…train and reverse if appropriate.
@J0anJosep
Change: Add the DC_AUTOREPLACE flag in missing places.
a0f0ece
@J0anJosep
Change: Deal with autoreplacements in extended rail depots.
c8dfc18 
As the autoreplace flag is set, only lift and tryplacing
in the original command for autoreplacing and not in any
recursive calls to move, buy, refit and sell commands.

# Conflicts:
#	src/autoreplace_cmd.cpp
@J0anJosep
Change: Flood trains inside a train depot platform.
e26d14c
@J0anJosep
Add: Set reservation and penalties to improve pathfinding in extended…
328f0ab 
… depots.
@J0anJosep
Add: Set a darker background in depot window for trains that cannot l…
a64aa22 
…eave a depot.
@J0anJosep
Add: Add road depot platforms.
ec5f239
@J0anJosep
Feature: Add construction of extended road depots.
718b925
@J0anJosep
Change: Adapt pathfinders for extended road depots.
c270ceb
@J0anJosep
Codechange: Always return the first vehicle when looking for close ro…
d9b182d 
…ad vehicles.
@J0anJosep
Add: Hide stopped road vehicles in extended depots that block the pat…
861de83 
…h of another vehicle.
@J0anJosep
Change: Buying and controlling road vehicles in extended road depots.
158c46f
@J0anJosep
Change: This is a testing version, so make it difficult to load saveg…
56af97c 
…ames with other versions of OpenTTD.
@J0anJosep
Doc: Changes in the landscape grid due to extended depots.
cc5fcd2
@github-actions github-actions bot added the work: needs rebase This Pull Request needs a rebase label Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@James103 James103

@2TallTyler 2TallTyler

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
candidate: needs considering This Pull Request needs more opinions size: extra large This Pull Request is very large; it properly needs to be split up before it can be properly reviewed work: needs rebase This Pull Request needs a rebase work: still in progress (draft) This Pull Request is a draft
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
Contributor Author J0anJosep commented I get a crash every time I try to open the "Build Roads" or "Build Ship Docks" toolbars. That was fixed some weeks ago. If you are using the binaries, they are outdated. I will ask next weekend Truebrain whether he can update them, because first I have some fixes for the PR that need to be pushed.