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

Assertion failure loading savegame with missing grf #7618

Closed
andythenorth opened this issue Jun 6, 2019 · 4 comments
Closed

Assertion failure loading savegame with missing grf #7618

andythenorth opened this issue Jun 6, 2019 · 4 comments

Comments

@andythenorth
Copy link
Contributor

andythenorth commented Jun 6, 2019 •
edited

Preamble

This is a very specific case, involving changing grfs on a running game. This would not be valid for normal gameplay, but is absolutely inevitable for grf development

I have no expectation of support for the case described below. I added the ticket because peter1138 suggested it, with no promise of a fix.

Version of OpenTTD

a83b80bacfa4aa5da2a6043136749b164ac2b191 from #7380

Expected result

Game doesn't crash with failed assertion on load of a savegame where grfs are missing.

Actual result

Game crashes with "Assertion failed at line 229 of src/road.h: roadtype < ROADTYPE_END"

Steps to reproduce

For this specific case:

  • Road Hog grf has been recompiled multiple times and reloaded in a running game
  • the reloading has worked fine during gameplay
  • after restarting the game, OpenTTD no longer recognises the currently installed version of Road Hog as compatible with the version in the savegame
  • I have tried compiling and installing all the other recent revs of Road Hog, but the save appears to have a version with changes made in between commits
  • during grf dev, it is common for OpenTTD to not detect recompiled grfs as compatible with the version in a savegame, but usually OpenTTD does not crash, and newgrf settings can be used to swap out the grf (I am well aware of what can go wrong with this)

For additional bonuses, the save is from a game testing PR #7380, and also requires multiple unreleased grfs (in the attached zip, along with the .sav).

Additionally there is no crashlog, because OpenTTD has a guard and won't generate crashlogs when grfs are missing.

I'm not going to be shocked if this issue is rejected.
@andythenorth
Copy link
Contributor Author

7618.zip

@andythenorth
Copy link
Contributor Author

andythenorth commented Jun 23, 2019 •
edited

(lldb) up
frame #21: 0x000000010012468c openttdRoadVehicle::GetMaxTrackSpeed() const + 284 openttdRoadVehicle::GetMaxTrackSpeed:
-> 0x10012468c <+284>: leaq 0x308d3b(%rip), %rdi ; "Assertion failed at line %i of %s: %s"
0x100124693 <+291>: leaq 0x310a31(%rip), %rdx ; "OpenTTD.andythenorth/src/road_map.h"
0x10012469a <+298>: leaq 0x310a65(%rip), %rcx ; "MayHaveRoad(t)"
0x1001246a1 <+305>: movl $0xa6, %esi

@LordAro
Copy link
Member

LordAro commented Jan 3, 2020 

#3  0x00007ffff555e526 in __assert_fail () from /usr/lib/libc.so.6
#4  0x0000555555a589da in GetRoadTypeInfo (roadtype=<optimized out>) at /home/lordaro/dev/openttd/src/road.h:228
#5  0x0000555555a58bf0 in RoadVehicle::GetMaxTrackSpeed (this=<optimized out>, this=<optimized out>) at /home/lordaro/dev/openttd/src/roadveh.h:276
#6  0x0000555555a59e63 in GroundVehicle<RoadVehicle, (VehicleType)1>::PowerChanged (this=this@entry=0x55555835ef50) at /home/lordaro/dev/openttd/src/ground_vehicle.cpp:31
#7  0x0000555555a5a094 in GroundVehicle<RoadVehicle, (VehicleType)1>::CargoChanged (this=this@entry=0x55555835ef50) at /home/lordaro/dev/openttd/src/ground_vehicle.cpp:84
#8  0x0000555555bf0e07 in AfterLoadVehicles (part_of_load=part_of_load@entry=true) at /home/lordaro/dev/openttd/src/saveload/vehicle_sl.cpp:409
#9  0x0000555555bc5cbe in AfterLoadGame () at /home/lordaro/dev/openttd/src/saveload/afterload.cpp:793
#10 0x0000555555be2c9e in DoLoad (reader=reader@entry=0x55555820d7a0, load_check=load_check@entry=false) at /home/lordaro/dev/openttd/src/saveload/saveload.cpp:2681
#11 0x0000555555be4c2c in SaveOrLoad (filename=<optimized out>, fop=fop@entry=SLO_LOAD, dft=<optimized out>, sb=sb@entry=NO_DIRECTORY, threaded=threaded@entry=true)
    at /home/lordaro/dev/openttd/src/saveload/saveload.cpp:2788
#12 0x0000555555b4de81 in SafeLoad (filename=<optimized out>, fop=<optimized out>, dft=<optimized out>, newgm=newgm@entry=GM_NORMAL, subdir=subdir@entry=NO_DIRECTORY, lf=lf@entry=0x0)
    at /home/lordaro/dev/openttd/src/openttd.cpp:1009
#13 0x0000555555b4e095 in SwitchToMode (new_mode=SM_LOAD_GAME) at /home/lordaro/dev/openttd/src/openttd.cpp:1093
#14 0x0000555555b50c4d in GameLoop () at /home/lordaro/dev/openttd/src/openttd.cpp:1448
#15 0x0000555555cf11ef in VideoDriver_SDL::MainLoop (this=0x555557289ca0) at /home/lordaro/dev/openttd/src/video/sdl2_v.cpp:760
#16 0x0000555555b4ed0d in openttd_main (argc=<optimized out>, argv=<optimized out>) at /home/lordaro/dev/openttd/src/openttd.cpp:858
#17 0x00007ffff5551153 in __libc_start_main () from /usr/lib/libc.so.6

So after some (somewhat manual) debugging, it appears the the issue boils down to GetRoadType(v->tile, RTT_ROAD) returning ROADTYPE_END, presumably due to some roadtype having been removed/renumbered. This then breaks when passed to functions expecting an actual road type.

So yeah, gonna chalk this one up to savegame being invalid and not recoverable due to GRF changes. As much as I'd like to harden OTTD against any sort of crashes, I don't think this one is feasible

@andythenorth
Copy link
Contributor Author

Thanks for this. There's been no activity on this for some time, and as it stands, it doesn't look likely that it will go any further. I'm closing it as we try to keep the issue count low for OpenTTD, it helps us focus on things that are important and fun. Feel free to discuss in irc or request re-opening if you disagree. Thanks for contributing!

[Lol]

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
2 participants
@andythenorth @LordAro