search

pioneerspacesim / pioneer

A game of lonely space adventure
https://pioneerspacesim.net
1.64k stars 380 forks source link

Crash when selecting delivery mission on bulletin board #123

Closed robn closed 13 years ago

robn commented 13 years ago

Mentioned by @SeanN in #117:

Using dev build fabea7a-win32: I just failed a delivery mission to Alpha Centauri and tried to pick up another delivery to Sol. Upon clicking on the square I got the following:

error:data/modules/DeliverPackage.lua.82:attempt to index local 'ad' (a nil value)

Brianetta commented 13 years ago

Meh. Got a different crash whilst trying to replicate this one. Logging a new issue...

Brianetta commented 13 years ago

This issue is cursed, or something. Every time I try to reproduce it, I find an unrelated game crash.

Can somebody else have a go at this?

Brianetta commented 13 years ago

OK, I have managed to actually test this. It doesn't crash; I have not seen any Lua errors with the courier mission, whether or not they involved Sol or Alpha Centauri.

One thing I have noticed is that courier missions are never marked as failed in the mission list. They are "Active" even after the deadline. Once docked, they vanish from the list. Assassination missions become marked as failed when the due moment passes.

Could it be that the mission was cleaned from the BBS system whilst the player was viewing it?

robn commented 13 years ago

The mission state is set by the script as it pleases. DeliverPackage only updates the mission status when you docked, so they don't change from to failed immediately. This should perhaps be changed. I don't know what the Assassination module does - I assume you're talking about @richardpl's new module? I haven't looked at it yet as its been a bit of a moving target and I've been waiting for him to let us know when he's finished.

Its possible that the mission could disappear while the player is viewing it. The new menu code should make that less dangerous, but I haven't tested it properly. I will do so.

richardpl commented 13 years ago

My code just check for mission due time on entering new system and sets mission state according to that. Similar can be done for DeliverPackage.

DeliverPackage missions are checked when docked and when failed are immediately removed. They will be always displayed as failed until you dock right station...

Ziusudra commented 13 years ago

I just got this in Beta Hydri when choosing a new mission after successfully completing one.

Ziusudra commented 13 years ago

Ideally, line 189 would be the problem. The script should wait to remove the ad from its own table until the game calls onDelete, which should happen once the game processes the RemoveAdvert call on line 190.

The problem is that onDelete is rarely getting called. Here is some debug output I got in Alpha Centauri.

onUpdateBB:ref: 27 removed onDelete:ref: 27 removed ... onUpdateBB:ref: 29 removed ... onUpdateBB:ref: 38 removed onDelete:ref: 38 removed ... 3 clouds brought over <-----player left the system onDelete:ref: 29 removed (other ads in system also deleted)

After that I went to Sol and sat in Gates Spaceport and set maximum time acceleration. While in Sol onUpdateBB called RemoveAdvert 107 times; onDelete was only called 9 times.

So, why is SpaceStation::RemoveBBAdvert not matching the ref sent to it?

Oh, there it is. Line 190, that is the station passed to onUpdateBB which may not be the station that ad is on. It should be ad.station:RemoveAdvert.

See #379 for pull request.

robn commented 13 years ago

Incredibly subtle. I need to test it but it makes sense. Congratulations, and I believe @Brianetta owes you a beer! :)

Brianetta commented 13 years ago

Great, now how am I going to find all the other bugs? (-: