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

Jump to bottom

1.9.3 Crash report #7766

Closed
SamuXarick opened this issue Oct 9, 2019 · 2 comments
Closed

1.9.3 Crash report #7766

SamuXarick opened this issue Oct 9, 2019 · 2 comments

Comments

@SamuXarick
Copy link
Contributor

SamuXarick commented Oct 9, 2019 •
edited

Version of OpenTTD

1.9.3

Expected result

Actual result

Steps to reproduce

I set up some NewGRFs downloaded via online content, and ran some AIs. Game started in 1950, but crashed quite early, in 1955.

The Minimal GS is edited.
Minimal GS.zip
crash.zip
@nielsmh
Copy link
Contributor

nielsmh commented Oct 9, 2019

Offending function:

OpenTTD/src/script/api/script_order.cpp

Lines 667 to 677 in f664395

/* static */ uint ScriptOrder::GetOrderDistance(ScriptVehicle::VehicleType vehicle_type, TileIndex origin_tile, TileIndex dest_tile)
{
if (vehicle_type == ScriptVehicle::VT_AIR) {
if (ScriptTile::IsStationTile(origin_tile) && ::Station::GetByTile(origin_tile)->airport.tile != INVALID_TILE) origin_tile = ::Station::GetByTile(origin_tile)->airport.tile;
if (ScriptTile::IsStationTile(dest_tile) && ::Station::GetByTile(dest_tile)->airport.tile != INVALID_TILE) dest_tile = ::Station::GetByTile(dest_tile)->airport.tile;
return ScriptMap::DistanceSquare(origin_tile, dest_tile);
} else {
return ScriptMap::DistanceManhattan(origin_tile, dest_tile);
}
}

Crash happens on line 670. origin_tile is 39971 (0x9C23) and that tile is a buoy.

image

This causes a crash because a buoy is a station tile, but it's not a station (it's a waypoint), thus IsStationTile() returns true, but Station::GetByTile() returns nullptr. So you get a null pointer dereference looking up the airport station part location.

Fix would be replacing by something like this, no need to check for IsStationTile() since Station::GetByTile() already checks that:

Station * origin_station = ::Station::GetByTile(origin_tile);
if (origin_station != nullptr) origin_tile = origin_st->airport.tile;

There is also a bug in the AI doing this, since it's asking for aircraft distance from a buoy to somewhere else, and aircraft don't use buoys!

@glx22
Copy link
Contributor

glx22 commented Oct 9, 2019

Duplicate of #7593

@glx22 glx22 marked this as a duplicate of #7593 Oct 9, 2019
@glx22 glx22 closed this as completed Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nielsmh @glx22 @SamuXarick