0003117: Game crashes during AI turn

Notes
(0007891) moog (reporter) 2020-04-26 13:46	gdb.txt is the GDB backtrace that I got when successfully reproducing the issue. It was made on the binaries from 55b54024a82aaad3b4572d674d424c2247a47930 commit.

(0007893) Shalthrea (developer) 2020-04-27 00:51	Can confirm that it reproduces. Looks like we're getting a call to the `doChannelProbing` lambda inside `VCAI::moveHeroToTile(int3, HeroPtr)`, which expects the hero's location to contain some object other than the hero itself. When it doesn't, we get a `nullptr` dereference.

(0007896) Shalthrea (developer) 2020-04-30 23:37	Alright, to be a bit more specific about what is going on here: there is a Cyra in a whirlpool, and the AI decides to have her revisit the whirlpool a number of times until she is down to a single gremlin. Once this occurs, the hero is considered "protected" from the ill effects of the whirlpool, and is allowed to choose freely from the whirlpool exits when revisiting (`CGWhirlpool::onHeroVisit`). It just so happens that once Cyra is down to only a single gremlin, the AI no longer is actively pushing her to revisit the whirlpool. Instead, the AI is just finding no actions for her to take overall, and is nearing the end of `VCAI::wander`. However, the fix for https://bugs.vcmi.eu/view.php?id=1126 [^] steps in and forces her to do one last revisit of the whirlpool. When `VCAI::showTeleportDialog` is triggered with this last revisit, and the message is thus filled with all the exit alternative, a routine begins to store these exits for later exploration, keeping them in `teleportChannelProbingList`. In most cases, this list would be filled by the main logic in `VCAI::moveHeroToTile`, and then immediately each entry would be visited and the list cleared. Unfortunately, since this is a case of revisiting, there is special case logic in `VCAI::moveHeroToTile` that kicks in. This special case logic never performs the probing, and never clears out the list. Cyra's actions end, and Piquedram attempts to visit a campfire. Unfortunately, since `teleportChannelProbingList` is persisted between heroes, he then attempts to perform probing for the whirlpools of interest that remain in the list, but he is not standing on anything visitable. This leads to the scenario that I described in my previous comment, and we get a `nullptr` dereference. While I'm not 100% comfortable with the design, I believe that the most appropriate action here is to clear `teleportChannelProbingList` at the end of the revisiting special case logic of `VCAI::moveHeroToTile`, and will prepare a pull request doing exactly this.

Issue History
Date Modified	Username	Field	Change
2020-04-26 13:43	moog	New Issue
2020-04-26 13:43	moog	File Added: bad_save.tar.xz
2020-04-26 13:45	moog	File Added: gdb.txt
2020-04-26 13:46	moog	Note Added: 0007891
2020-04-27 00:51	Shalthrea	Note Added: 0007893
2020-04-30 23:37	Shalthrea	Note Added: 0007896
2020-05-01 03:35	SXX	Status	new => resolved
2020-05-01 03:35	SXX	Fixed in Version	=> 1.next
2020-05-01 03:35	SXX	Resolution	open => fixed
2020-05-01 03:35	SXX	Assigned To	=> Shalthrea

Relationships