WEEK 7

Hello, everyone! With both the European and US Amiga releases of Kult / Chamber of the Sci-Mutant Priestess playable from start to finish, this week I shifted focus from adding features to hardening what was already there. Last week I mentioned that ScummVM runs the engine through the Coverity and PVS-Studio static analysers, and that their reports had turned up a handful of issues worth looking at. This week I went through those reports properly, and on top of that I chased down a save/load bug that had been quietly breaking a couple of rooms after loading a game.

Working through the Coverity report

Static analysers are good at spotting the kind of code that is technically wrong but rarely bites in day-to-day play: unreachable branches, resources that are allocated but never freed, and fields that are read before they are written. Chamber is a reimplementation of a 1989 DOS game, so a lot of the code mirrors the original binary very closely — which means some of these findings are actually faithful reproductions of dead code that was already dead in the original.

That distinction mattered while reviewing the reports, because I did not want to “fix” something that was deliberately mirroring the disassembly. So for each finding I went back to the original code to confirm what was really going on before touching anything.

A few of the more interesting ones:

• In CMD_21_VortTalk, Coverity flagged a branch guarded by rand_value >= 170 as unreachable. Checking the disassembly, the num == 7 case it belonged to is dead in the original too, so the branch could go.

• In drawRoomStatics, there was a door check for index == 91, but by the time that code runs the index is already constrained to the 50..60 range, so the check can never be true.

• In the CGA blitter, blitToScreen had an endY > 200 clamp that could never fire, because dy and h are hardcoded to 0 and 200 just above it.

Alongside the dead code, the report caught two genuinely useful things: a memory leak and an uninitialised field. Several call sites were calling loadFond / ega_loadFond / loadSplash, which return a Graphics::Surface, and then throwing that surface away without freeing it — a small leak every time a room background or splash was loaded. And the engine’s _speaker member was not being initialised in the constructor. Both are the sort of thing that is easy to miss by eye and exactly what these tools are for.

Working through the PVS-Studio report

PVS-Studio leans towards a different class of finding — dead stores and redundant expressions — and it caught a similar mix:

• Another impossible clamp in the CGA blitter, this time endX_bytes > 80 on the horizontal axis, again dead because the width is hardcoded to a full 320 pixels.

• A dead ofs++ in cga_ZoomInplace, immediately followed by ofs being reassigned to oofs, so the increment never had any effect.

• In SCR_23_HidePortrait, a left/right button branch where both arms were identical — this handler simply has no special behaviour to skip, so the branch collapsed to a single path.

• In SCR_46_DeProfundisLowerHook, the return value of getPuzzlSprite() was being stored into sprofs and then overwritten on the very next line, so the first store was pointless.

There were also a couple of intentional busy-wait loops that PVS-Studio flagged for having an empty ; body. These are genuinely meant to be empty — they are timing delays — so instead of silencing the warning I gave them an explicit {} body to make the intent obvious to both the analyser and the next person reading the code.

None of these are dramatic bugs, but clearing them keeps the engine’s future reports clean, so real regressions do not get lost in a wall of pre-existing noise.

Fixing room state that vanished after loading a save

The most interesting problem this week was not a static-analysis finding at all — it was a save/load bug I ran into while testing.

Chamber builds each room from a base decor, but scripts frequently draw persistent changes on top of that: alternate decor sets, extra objects that appear once triggered, and the final frames of animations. The catch is that they draw these directly into the backbuffer. On a fresh visit that is fine, but when you saved and reloaded, the engine rebuilt the room from the base decor only — so anything a script had painted into the backbuffer was simply gone.

In practice this meant that things like the De Profundis monster, or Deilos, would fail to reappear after loading a game, even though the game state said they should be there. The state was correct; the pixels that represented it had never been saved.

The fix was to bump the save version to 2 and store what was actually missing: the zone palette, the current video mode, and the backbuffer contents themselves. On load, if the video mode matches, the backbuffer is restored verbatim; otherwise the engine falls back to the old rebuild-from-decor behaviour, so older saves and cross-mode loads still work. While I was in there I also made load reapply the room palette and honour the launcher’s save-slot option, so booting straight into a save from the launcher lands you in a correctly coloured room.

Looking ahead

That wraps up the cleanup pass I wanted to do before moving on. The engine is now noticeably tidier and more robust, which is a perfect state to leave it in. Starting next week, I will be shifting my focus to a completely different engine. I wonder if anyone can guess which one it will be?(Hint: It will be a great adventure!) Stay tuned, and thanks for reading.