Cataclysm-dda: Bug when trying to remove car part

[rm-code]

Activity ACT_VEHICLE: vehicle not found
FUNCTION : static void veh_interact::complete_vehicle()
FILE : src/veh_interact.cpp
LINE : 2290

INTO

process_activity ACT_VEHICLE: vehicle not found
FUNCTION : void activity_handlers::vehicle_finish( player_activity, player* )
FILE : src/activity_handlers.cpp
LINE : 1357

Happend when I tried to remove a swappable storage battery from this position:

(Sorry for taking a photo instead of a screenshot ... the PC I'm playing on isn't connected to the net atm :P)

[kolsurma]

Just a few questions:

• On what OS did you experience the problem (Windows, Linux, OS X etc.)

• What version were you playing:

  • Version string (preferably full version e.g. "0.C-4547-g3f1c109", or Jenkins build number e.g. 3245)
    (look at the title bar of the window for this info)

• Description of the problem you've found written in a way that enables anyone to try recreate it.
  I see you removed a solar panel, was that all you did?

Thanks!

[codemime]

I can't explain this, but the photo looks somewhat cool (I mean, more interesting than a screenshot would).
I wish we could use shaders to achieve certain visual effects.

[rm-code]

• OS: _Windows 10_
• Version: _6250_
• Description: _I tried to remove a swappable storage battery which was the first thing I did when I got to the car. Before that I just dragged my shopping cart (you can see it next to the car two tiles right of my character)._

I wish we could use shaders to achieve certain visual effects.

One day, I'm sure! :)

[ZhilkinSerg]

Something like this will be cool

[kolsurma]

@rm-code do you have a copy of the save where (and maybe when) the error occurred?
Is it repeatable for you?

[ZhilkinSerg]

There is post-processing injector (https://reshade.me/).

[rm-code]

@kolsurma I'll check tomorrow if maybe one of the backups is close to this point in the game and if I can reproduce the issue.

When I the issue occurred I immediately tried to remove the battery again and it worked without a problem.

[bdgackle]

@rm-code: Did you have the experimental Z-level support enabled?

Taking an initial look:

The only place in the whole codebase that veh_interact::complete_vehicle() is called from is activity_handlers::vehicle_finish() (not coincidentally, these are the two sources of the errors above).

In both cases, the specific error message is being generated by a nullptr return value from:
g->m.veh_at( tripoint( g->u.activity.values[0], g->u.activity.values[1], g->u.posz()));

g in this case is the game instance. I'm still tracing back to see how the activity values might be set wrong to give an x/y that contains no vehicle. It also seems like a bad g->u.posz() would cause this, but I don't _think_ this could have been the cause unless z levels are on.

Edit: clarification

[bdgackle]

One other thing, well we're in there:

activity_handlers::vehicle_finish has two arguments, a player_activity* and a player*. These get passed in, used to find the vehicle (among other things), and then the very next line calls veh_interact::complete_vehicle(), which then gets those same pointers from the global game instance.

This is kind of a glaring inconsistancy. I feel like we should do one of the following:

1) Eliminate the function call parameters to activity_handlers::vehicle_finish(), and get the values from g

OR

2) Add function call parameters to veh_interact::complete_vehicle() and refrain from using the global values.

I assume that this inconsistancy is the result of slowly eliminating one convention or the other from the code base, so I'm not sure which one to change to move us in the right direction.

Edits: typos.

[bdgackle]

One more question for @rm-code, did you do the part remove by opening the vehicle interaction menu, and then going to the 'remove' menu within that?

I think that is the only way you could have done it, but I'm not 100% sure, and want to make sure there is no other path you might have followed to trigger these function calls.

[rm-code]

@kolsurma I haven't been able to reproduce it with the same car on one of the older backups unfortunately.

@bdgackle Z-Levels are off. I used the vehicle menu and selected the part to remove. Not sure if it helps, but after those two messages above I was able to select remove for the same part again, but this time it worked fine.

Regarding the coding style: I have no clue about the cdda codebase, but generally I think it's always good to avoid direct access to the global namespace whenever it's possible.

[bdgackle]

The fact that it worked the second time is interesting.

It looks like map::veh_at() either returns false, or calls map::veh_at_internal(). There are three possilbe ways to get a null pointer back from these functions (without seeing different error messages than you saw):

1. level_cache::veh_in_active_range set to false for your map level
2. level_cache::veh_exists_at[x][y] set to false for the particular x/y coordinates passed in.
3. Coordinate passed in is complete garbage (not on map at all).

It looks like you've stumbled on an edge case where the cache isn't getting updated properly. On the one hand, this is going to be super hard to reproduce. On the other, it is a good find because this could cause other subtle intermittent bugs with vehicle handling code.

I'll see if I can track down where this is, but the caches are complex and I'm brand-new to this codebase. We may need someone smarter than me to look at it. :-)

Edit to add: This is just a swag, but it probably didn't occur again following the save game because the cache is getting setup correctly during the game loading process. It probably took wandering to that point on the map without an intervening load to follow the (hypothetical) code path that fails to set one of the vehicle existance flags.

[Coolthulhu]

Most likely it's the coords.
If vehicle cache was wrong, there would be visible oddities, such as being able to phase through vehicles, vehicle parts not drawing etc.

[bdgackle]

@rm-code: Were you by chance tracking that vehicle (ie, did you have the "remember vehicle position" thing turned on for it)?

Edit, nevermind, I hadn't noticed Coolthulu's comment. If the vehicle cache is being used to actually draw the vehicles, then I'm probably barking up the wrong tree here.

[keyspace]

Most likely it's the coords.

I have had OP happen to me when resuming an activity (removing a part) on a vehicle that has moved.

[rm-code]

The vehicle didn't move in my case. It was generated broken down.

[rm-code]

I'll close this since I never reproduced it and with the lack of info it's impossible to track down the issue.