Access violation while checking for forbidden magic

[Elias Erkamo]

Description of issue: While debugging the game via Visual Studio, I've been using wizard mode to rapidly switch dungeon levels until a certain kind of encounter is generated, and as a side effect of this I've been frequently running into an access violation at line 269 of Monster.cpp, in function denySpell which is appears to be responsible for checking what spells NPC casters shouldn't cast. This error occurs pretty commonly during level generation, apparently as a result of NPC casters doing their pre-buff routine, but I've had it occur around casters during play as well.

If the issue occurs while generating a level, the game will auto-save and the resulting save will bug out (at least when switching dungeon level with wizard mode). When loading up the save post-crash, the game will display the previous level, but the player character will be on the level that just got generated, and can move, get attacked by NPCs, etc, generating messages in the in-game log. Switching dungeon level using the wizard mode command will cause the game to switch back to displaying the level the character ends up being moved to.

Steps to reproduce: The issue is most easily reproduced by enabling wizard mode, then using w->p to switch through new dungeon levels rapidly. The game is likely to generate a level with the right conditions to generate the crash pretty quickly.

[Elias Erkamo]

Wasn't able to get the reproduction case for this I promised via email today due to the autosave thing and time restrictions, but I can try to get a savegame for that later by copying one each time I try to reproduce the issue by switching level, depending on whether the level generation is deterministic.

Still need to check how this affects normal play - if you descend via stairs like a normal player you might be able to simply ascend back to fix the issue with the viewpoint after loading the save, though I don't know what else interrupting the game before the post-level-generation tasks are complete might break.

[Elias Erkamo]

Apparently new dungeon levels are randomized at generation time, so that eliminates that method of getting a reproduction savegame for this.

The good news is that descending via stairs and encountering this issue does not appear to cause the same dysfunction as doing it via wizard mode command - the newly generated level isn't saved, the player character will be on the stairs down on the previous level after loading the savegame, and descending again will cause a completely new level to be generated. As such, the issue is not as gamebreaking as I suspected before fully investigating it, though it still has the potential to cause some problems since it can also occur when NPCs cast spells during play.

[Elias Erkamo]

I did some playtesting, as opposed to the previous brief bursts of debugging, and it turns out that this issue occurring when NPC casters are trying to cast spells during play is not rare at all - in fact, the source version of the game is crashing due to this issue all over the place, and as a result, is not really all that playable.

As such, I feel it's safe to say that this is a critical issue and has to be addressed in order for it to make any sense to release a new build of the game to the general public.

I was not able to reproduce the issue in the 2016-09-16 release build of the game. I will continue investigating.

[Elias Erkamo]

The pointer assignment on line 266 in the relevant function (const char* name = NAME(eID);) appears to be assigning garbage (ends up pointing to an address containing some variation of Ý spam and occasional random other characters according to the debugger). Not yet clear to me why, and it's always pointing to garbage even when the function call does not result in an access violation. Not certain regarding its relationship to the access violation issue, but it does look awfully conspicuous.

Other than that, my testing today confirms that denySpell gets called a lot when there's casters pre-buffing during level generation and only results in an access violation in a tiny minority of cases.