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

[Bug]: CTD when all cargos are disabled by NewGRF, and cargo payment graph opened #9545

Closed
2TallTyler opened this issue Sep 9, 2021 · 5 comments
Labels
bug Something isn't working priority: low This issue should be fixed but has either a low impact or is en edge-case

Comments

@2TallTyler
Copy link
Member

Version of OpenTTD

Master (7469f00)

Expected result

When all cargos are disabled by a NewGRF like Feature Disabler GRF, starting a new game without any cargos prompts the following error message on map generation:

No vehicles will be available at all
Change your NewGRF configuration

I would expect opening the "Cargo payment rates" graph to show an empty graph with no cargos.

Actual result

Opening the "Cargo payment rates" graph causes a crash to desktop.

Steps to reproduce

  1. Enable the NewGRF Feature Disabler GRF with the parameter enabled to disable all cargos
  2. Start a game
  3. Open the "Cargo payment rates" graph
  4. CTD
@nielsmh
Copy link
Contributor

nielsmh commented Sep 9, 2021

I would expect the game to abort world generation if there are no cargo types available. If there aren't any cargo types there isn't any game to play at all, since you have nothing to transport.

@ldpl
Copy link
Contributor

ldpl commented Sep 9, 2021

It's perfect! You can build eye-candy objects all day without the distraction of pointless transportation! :p

@TrueBrain TrueBrain added bug Something isn't working priority: low This issue should be fixed but has either a low impact or is en edge-case labels Sep 14, 2021
@James103
Copy link
Contributor

James103 commented Feb 14, 2022

The game crashes in this case due to a division by 0 inside the following function:

OpenTTD/src/widget.cpp

Lines 2172 to 2180 in 41c40f1

void Scrollbar::SetCapacityFromWidget(Window *w, int widget, int padding)
{
NWidgetBase *nwid = w->GetWidget<NWidgetBase>(widget);
if (this->IsVertical()) {
this->SetCapacity(((int)nwid->current_y - padding) / (int)nwid->resize_y);
} else {
this->SetCapacity(((int)nwid->current_x - padding) / (int)nwid->resize_x);
}
}

I was able to reproduce the crash on master (2022-02-10) and in jgrpp (0.46.0). The stack trace of the latter crash was used to find the crashing function.

Crash files from the former crash: crash20220214021730.zip

@nielsmh
Copy link
Contributor

nielsmh commented Feb 14, 2022

I'm sure there's more places around the code that assumes that at least one cargo type is available too. That function is just the first that happens to be hit here.

@2TallTyler
Copy link
Member Author

I think the proper fix is to abort map generation if no cargos are available, not to find and fix the crashes.

PeterN added a commit to PeterN/OpenTTD that referenced this issue Nov 4, 2023
This is not a very useful state, but it's nice to not crash.

Some parts of the game don't (yet) check for cargo types being redefined, that is out-of-scope here.
PeterN added a commit to PeterN/OpenTTD that referenced this issue Nov 4, 2023
This is not a very useful state, but it's nice to not crash.

Some parts of the game don't (yet) check for cargo types being redefined, that is out-of-scope here.
@PeterN PeterN closed this as completed in bbd64bb Nov 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: low This issue should be fixed but has either a low impact or is en edge-case
Projects
None yet
Development

No branches or pull requests

5 participants