Traceback Activity ACT_VEHICLE: vehicle not found

[PatrikLundell]

Returning to camp while 3 companions are dismantling vehicles, a traceback is issued.

Probably caused by multiple companions trying to remove the same vehicle part (I've definitely seen it earlier, with two companions working in the last remaining component of a vehicle. The first one finished the job, while the second one resulted in a traceback later. Probably this traceback). There's a need for the game to keep track of which part someone (including the PC) is currently working on and block others (including the PC, in which case there should be a useful feedback, e.g. "can't remove X, Y is interacting with it"). It would be even better if that it was carried one step further, with interaction locking also locking parts dependent on it (as companions currently seem not to care about dependencies, and thus can split vehicles and, I think, remove structural parts while leaving hub assemblies and wheels).

DEBUG : Activity ACT_VEHICLE: vehicle not found

FUNCTION : static void veh_interact::complete_vehicle(player&) FILE : src/veh_interact.cpp LINE : 2861

Steps To Reproduce

My guess is that you set as many companions as possible to dismantle a single vehicle, the smaller the better.

Expected behavior

No traceback, obviously, and no time wasted on duplicate efforts.

Screenshots

Versions and configuration

• OS: Windows
  • OS Version: 10.0 1909
• Game Version: 0.E-2 [64-bit]
• Graphics Version: Tiles
• Game Language: English [en]
• Mods loaded: [ Dark Days Ahead [dda], Disable NPC Needs [no_npc_food] ]

Additional context

debug.log

The save file (saved immediately after the traceback was dismissed) is too large to be accepted (32 MB), even when zipped with Ultra compression. It will remain on my system for some time, if of interest. If you can provide info on which parts of the save that can be deleted (in a way I can understand: I just see a lot of folders with names that probably reflect some coordinates) while still providing a useful save I can try to do that that.

Given that a traceback (or a very similar one) happened the previous time I returned to the camp (an hour or so earlier: I'm fetching vehicles for dismantling), it's probably not too hard to reproduce. I tried to work around the problem then by asking two of the three companions to stop, led them to another vehicle also in a dismantling zone, and asked them to dismantle vehicles. Both of them seemed to start to work on that vehicle, but as I return (moving after the save so I can actually see the companions) the morons are back on the same vehicle they worked on the first time, so it doesn't seem to be possible to manually distribute them between vehicles to avoid them clashing.

[PatrikLundell]

After trying to debug vehicle dismantling I think I understand why it can get screwed up, but I don't understand enough of the code to say what to do about it beyond replacing the current part identification method with a robust one.

activity_handling.cpp operation activity_reason_info allocates a vpindex to the character dismantling the vehicle, and before that the characters are scanned to find the vpindices they work on to stop them from working on the same part as somebody else is already working on.

However, the vpindex is not static, but changes over the time the characters are working on the vehicle, presumably as parts are removed, which can lead to all sorts of trouble if the vpindex is the only reference used to identify the part to be worked on, worked on, and removed. The removed part can be a different one from the one supposedly worked on, which can be a different one from the one intended to work on.

I added a line to generate debug log output to the 0.E2 stable code just after "p.activity_vehicle_part_index = vpindex;" (unfortunately it also generated a traceback message), and set 4 companions to deconstruct a vehicle, which generated the following entries in debug.log: 00:13:30.706 ERROR : C:\Cataclysm-DDA-0.E-2\src\activity_item_handling.cpp:1406 [can_do_activity_there] Vehicle work : vpindex: 54, vpinfo name : board, NPC : Gonzalo Wisniewski 00:14:35.266 ERROR : C:\Cataclysm-DDA-0.E-2\src\activity_item_handling.cpp:1406 [can_do_activity_there] Vehicle work : vpindex: 86, vpinfo name : quarterpanel, NPC : Otha 'Mayday' Vance, Cyborg 00:15:18.142 ERROR : C:\Cataclysm-DDA-0.E-2\src\activity_item_handling.cpp:1406 [can_do_activity_there] Vehicle work : vpindex: 25, vpinfo name : windshield, NPC : Aurea 'Brat' Carrasco, Cyborg 00:17:08.086 ERROR : C:\Cataclysm-DDA-0.E-2\src\activity_item_handling.cpp:1406 [can_do_activity_there] Vehicle work : vpindex: 54, vpinfo name : board, NPC : Gonzalo Wisniewski 00:17:17.555 ERROR : C:\Cataclysm-DDA-0.E-2\src\activity_item_handling.cpp:1406 [can_do_activity_there] Vehicle work : vpindex: 38, vpinfo name : quarterpanel, NPC : Aurea 'Brat' Carrasco, Cyborg 00:17:21.824 ERROR : C:\Cataclysm-DDA-0.E-2\src\activity_item_handling.cpp:1406 [can_do_activity_there] Vehicle work : vpindex: 86, vpinfo name : quarterpanel, NPC : Otha 'Mayday' Vance, Cyborg 00:17:38.588 ERROR : C:\Cataclysm-DDA-0.E-2\src\activity_item_handling.cpp:1406 [can_do_activity_there] Vehicle work : vpindex: 36, vpinfo name : quarterpanel, NPC : Cecil Saldivar 00:19:06.557 ERROR : C:\Cataclysm-DDA-0.E-2\src\activity_item_handling.cpp:1406 [can_do_activity_there] Vehicle work : vpindex: 36, vpinfo name : quarterpanel, NPC : Cecil Saldivar 00:20:28.828 ERROR : C:\Cataclysm-DDA-0.E-2\src\activity_item_handling.cpp:1406 [can_do_activity_there] Vehicle work : vpindex: 54, vpinfo name : vehicle tank (60L), NPC : Gonzalo Wisniewski 00:32:27.170 ERROR : C:\Cataclysm-DDA-0.E-2\src\activity_item_handling.cpp:1406 [can_do_activity_there] Vehicle work : vpindex: 38, vpinfo name : headlight, NPC : Aurea 'Brat' Carrasco, Cyborg Initially, vpindex 54 refers to a board (two entries, as the companion had to move to the vehicle to actually start working on it), but after it was removed it referred to a vehicle tank instead. Similarly, vpindex 38 started to refer to a quarterpanel, but referred to a headlight once that had been removed. Presumably indices just drop down one step as parts are removed, so companions can keep "reusing" the same index for multiple parts in the same location if there is nothing blocking removal.

Edit: Rather than storing the unsafe vpindex the characters and the temporary vector ought to store the values of part->mount and part->id, which is the check currently used by vehicle.cpp operation vehicle::index_of_part to locate the part. This relies on the assumption that you can never mount two of the same part at the same vehicle location (a real world case would be modern motorcycles with two headlights rather a single one, presumably more for redundancy than a wider/shaped beam). If that assumption ceases to be valid the logic will have to be adjusted.

Assuming that the condition that there cannot be more than a single vehicle part of any one kind in a location is going to be upheld, the vehicle::index_of_part operation should be removed and replaced by e.g. bool vehicle::part_exists() to perform the "check_removed" check, plus either manual assignment of the element to the struct {point mount; vpart_id id}, called e.g. part_locator, or an operator that takes a vehicle_part and part_locator reference as parameters to copy the relevant fields from the vehicle_part to the part_locator, plus an equality operator for the part_locator.

The vehicle::index_of_part() operation is called from activity_item_handling.cpp, turret.cpp, veh_interact.cpp, veh_utils.cpp, and vehicle.cpp itself.

Edit 2: It can also be noted that the bug can cause the game to effectively crash. I don't remember if it's an outright crash or if it's an uncurable infinite repeated error message, but the only way to handle it is to stop dismantling before this state is reached. The fatal condition happens when two companions end up removing the last part of the vehicle. When the second companion finishes the job the referenced vehicle doesn't exist anymore.

[Ramza13]

So one way to share a save file is via google drive or something similar.

[kevingranade]

Well this is a bit of a problem, I've implemented the suggestion of stashing mount point and part id for identity, but then when I went to test this it's being so flaky (both with and without my changes) that I can't meaningfully test anything about it.

I have a start at a unit test for the scenario, but frankly at this point I've sunk way too much time into this and I just need to proceed to the release.

FWIW I also can't reproduce the issue, what I run into is NPCs simply refusing to continue working on dismantling the vehicle.

A much, much simpler change that would be worth considering is simply not allowing NPCs to work on the same tile of the vehicle at the same time.

[Fosheze]

A much, much simpler change that would be worth considering is simply not allowing NPCs to work on the same tile of the vehicle at the same time.

That would make the most real world sense too. In reality you wouldn't want multiple people working on different things on the car in the same area. You would just be getting in each other's way.

[actual-nh]

That would make the most real world sense too. In reality you wouldn't want multiple people working on different things on the car in the same area. You would just be getting in each other's way.

About the only exception I can see is lifting something heavy out (or putting it in).

[PatrikLundell]

Sorry, I somehow missed Ramza13's comment (but found it now in my mail history). https://www.dropbox.com/s/x3o1je78m5k3wye/Old%20Saybrook.zip?dl=0 was zipped up at the date this report was posted originally, and so ought to be the right copy.

Set 3+ companions to dismantle the same vehicle (using a zone: I've never used the garage), and the traceback will happen eventually, although it's not common. Keep at it and a crash will happen, but I don't think I've had more than 2-3 crashes over quite a few dozens of vehicles.

Just setting companions to dismantle vehicles and monitoring the ids, as I did in my original post, should show the issue fairly easily, and you see it in the game when wheels are removed (for some reason unknown to me the code explicitly disallows removal of those), and you also see it in vehicles that have frames removed while items that were mounted on the frame remain (and were invisible with the tile set I used, although they could be targeted).

I don't think blocking characters (including the PC) working on the same vehicle tile alone will work, as the indices will continue to sometimes refer to something else than they were when the task was begun if indices are still used as references, and this can result in the same tile and item on the tile being targeted anyway.

While it's a pity to have this issue remain, I understand if the need to get the release out the door first takes priority. However, things being flaky when tested regardless of whether the changes are there indicates there is a need to tackle the issue eventually.

When multiple people are assisting in lifting, they only produce a single job, so that's fine, and I don't think it would be a problem blocking multiple tasks on the same tile: it just won't fix the issue without additional measures to ensure the item targeted for removal is the one that's actually removed.

[kevingranade]

I don't think it would be a problem blocking multiple tasks on the same tile: it just won't fix the issue without additional measures to ensure the item targeted for removal is the one that's actually removed.

Whoops, you are absolutely correct, the indices are global across the vehicle. I agree this needs fixing, but I'm afraid it's going to be long and drawn out and probably super disruptive. The underlying problem is that the whole generic activity thing has grown into quite a monster, with lots of special casing scattered all over the place.

And now that I look at my previous comment again, I realize the problem was probably that I didn't have lifting resources nearby, which was what was causing my test runs to be flaky.

[PatrikLundell]

Using indices probably worked fine when only the PC was able to work on vehicles, but it fell apart when the same logic was applied with multiple agents (although in the special case of something smashing up the vehicle so it actually loses parts, not just get them into the destroyed state, such as a skeleton juggernaut, you can probably get issues even with only one worker, but that's definitely a VERY special case).

If nothing else, your problems definitely confirms my suspicion that trying to address the problem myself would not have been a good idea...

Also, it's good to find the source of the flakiness: that should help when the task is resumed.

[actual-nh]

BTW - Perhaps create a 0.G milestone and shift this issue to it?

[Salty-Panda]

This error can be also thrown by mounting part on barely floating vehicle ( possibly even when mounting an item from car's storage ). You start mounting part > vehicle changes, weight check > vehicle sinks > error appears upon completing activity.

[PatrikLundell]

Does the vehicle sink completely in that case, or is it "just" a case of the targeted part of it getting submerged, rendering the target unreachable? The reason for the question is that I would have expected a crash of the game if the vehicle was destroyed by sinking.

Regardless, it's really a separate process resulting in the same message, requiring separate measures to deal with the situations.

In the OP case it's sufficient to ensure the part targeted is the part removed and that a part under removal can't be targeted anew, while in this case it's a matter of dealing with the results of actions on the vehicle having additional effects (apart from actions causing it to sink, I can see the case of parts of the vehicle being destroyed completely by a ramming vehicle or other "attack" from e.g. a skeleton juggernaut, explosives, or a collapsing roof).

[Salty-Panda]

46399

Although it seems it was deemed unimportant back then.

[PatrikLundell]

It's odd that moving a part from cargo space to become a part of a vehicle would make it heavier, and -0.00 might indicating rounding was involved (or those welds are very heavy...).

Sinking as a result of making a vehicle heavier than its floating capacity should result in the vehicle sinking in general, of course, but the borderline case is fishy. The other issue is that vehicles sinking causes them to vanish, which I don't think will be addressed until the ability to do stuff under water gets expanded.

[actual-nh]

It's odd that moving a part from cargo space to become a part of a vehicle would make it heavier, and -0.00 might indicating rounding was involved (or those welds are very heavy...).

I seem to recall a weight cache? Perhaps that isn't getting cleared properly so the weight of the part is getting counted twice.