remove the false guards

* Sfall fix for > 160 maps for automap code (RPU triggers this)
* Logic fix for action menu, where actionIndex was incremented unconditionally and could go out of bounds.  This was a regression from vanilla

SFALL_COMPATIBILITY.md

@@ -82,7 +82,7 @@ See [`https://sfall-team.github.io/sfall/`](https://sfall-team.github.io/sfall/)
 | Interface / Cursor | get/set_cursor_mode | ✅ | - |
 | Locks | lock_is_jammed<br>unjam_lock<br>set_unjam_locks_time | not implemented | - |
 | INI settings | get_ini_setting<br>get_ini_string<br>get_ini_section<br>get_ini_sections<br>get_ini_config<br>get_ini_config_db<br>set_ini_setting | ✅ | `modified_ini` is intentionally omitted as deprecated. |
-| Objects and scripts | set_self<br>set_dude_obj<br>real_dude_obj<br>remove_script<br>get/set_script<br>obj_is_carrying_obj<br>loot_obj<br>dialog_obj<br>obj_under_cursor<br>get/set_object_data<br>get/set_flags<br>set_unique_id<br>set_scr_name<br>obj_is_openable<br>get/set_proto_data<br>get_object_ai_data | implemented: set_self, get/set/remove_script, obj_is_carrying_obj, loot_obj, dialog_obj, obj_under_cursor, get_object_data, get_flags, set_flags, set_unique_id, obj_is_openable, get_proto_data, set_proto_data | - |
+| Objects and scripts | set_self<br>set_dude_obj<br>real_dude_obj<br>remove_script<br>get/set_script<br>obj_is_carrying_obj<br>loot_obj<br>dialog_obj<br>obj_under_cursor<br>get/set_object_data<br>get/set_flags<br>set_unique_id<br>set_scr_name<br>obj_is_openable<br>get/set_proto_data<br>get_object_ai_data | implemented: set_self, real_dude_obj, get/set/remove_script, obj_is_carrying_obj, loot_obj, dialog_obj, obj_under_cursor, get_object_data, get_flags, set_flags, set_unique_id, obj_is_openable, get_proto_data, set_proto_data | get_object_data assumes raw memory offsets which may return different values in CE |
 | Other / Game management | set_movie_path<br>stop/resume_game<br>mark_movie_played<br>game_loaded<br>get_game_mode<br>get_uptime<br>signal_close_game | implemented: game_loaded, get_game_mode, get_uptime, signal_close_game | - |
 | Gameplay tweaks | set_pickpocket_max<br>set_hit_chance_max<br>set_xp_mod<br>set_critter_hit_chance_mod<br>set_base_hit_chance_mod<br>set_hp_per_level_mod<br>gdialog_get_barter_mod<br>get/set_unspent_ap_bonus<br>get/set_unspent_ap_perk_bonus<br>set_base_pickpocket_mod<br>set_critter_pickpocket_mod<br>get/set_inven_ap_cost<br>set_drugs_data<br>get_kill_counter<br>mod_kill_counter<br>set_pipboy_available | implemented: gdialog_get_barter_mod, get/set_unspent_ap{_perk}_bonus, get/set_inven_ap_cost | - |
 | NPCs | inc_npc_level<br>get_npc_level<br>npc_engine_level_up | not implemented | - |

sfall_testing/gl_test_ini.ssl

@@ -1,5 +1,6 @@
 #include "lib.arrays.h" // For arrays_equal, len_array, etc.
 #include "test_utils.h"   // For assertEquals, VALTYPE_STR, report_test_results
+#include "sfall.h"
 
 // To test: copy test.ini into the game folder before running
 procedure ini_test_suite begin
@@ -29,6 +30,15 @@ procedure ini_test_suite begin
     call assertEquals("TC2 get_ini_setting empty int", get_ini_setting("test.ini|ValidSection|EmptyValue"), 0);
     call assertEquals("TC2 get_ini_string empty string", get_ini_string("test.ini|ValidSection|EmptyValue"), "");
 
+    // Regression: sfall resolves leading-backslash paths relative to the game root.
+    result_array := get_ini_section("\\test.ini", "ValidSection");
+    call assertEquals("TC2B Size", len_array(result_array), 3);
+    call assertEquals("TC2B Key1", result_array["Key1"], "Value1");
+    call assertEquals("TC2B get_ini_setting int", get_ini_setting("\\test.ini|ValidSection|Key2"), 2);
+    call assertEquals("TC2B get_ini_string string", get_ini_string("\\test.ini|ValidSection|Key1"), "Value1");
+    result_array := get_ini_sections("\\test.ini");
+    call assertEquals("TC2B get_ini_sections size", len_array(result_array), 3);
+
     // INI file not found
     result_array := get_ini_section("nonexistent.ini", "AnySection");
     call assertEquals("TC3 Size", len_array(result_array), 0);

src/animation.cc

@@ -1,6 +1,8 @@
 #include "animation.h"
 
 #include <algorithm>
+#include <array>
+#include <cstddef>
 #include <stdio.h>
 #include <string.h>
 
@@ -38,9 +40,13 @@
 
 namespace fallout {
 
-#define ANIMATION_SEQUENCE_LIST_CAPACITY 32
+// sfall exposes this as [Misc] AnimationsAtOnceLimit. Keep CE aligned with
+// sfall's default 64 total / 52 non-reserved slots for now; this can become
+// configurable later.
+#define ANIMATION_SEQUENCE_LIST_CAPACITY 64
 #define ANIMATION_DESCRIPTION_LIST_CAPACITY 55
-#define ANIMATION_SAD_LIST_CAPACITY 24
+#define ANIMATION_SAD_LIST_CAPACITY 64
+#define ANIMATION_NON_RESERVED_LIST_CAPACITY 52
 
 #define ANIMATION_SEQUENCE_FORCED 0x01
 
@@ -76,6 +82,11 @@ typedef enum AnimationKind {
     ANIM_KIND_SET_LIGHT_INTENSITY,
 } AnimationKind;
 
+static bool animationKindIsCallback(int kind)
+{
+    return kind == ANIM_KIND_CALLBACK || kind == ANIM_KIND_CALLBACK3;
+}
+
 typedef enum AnimationSequenceFlags {
     // Specifies that the animation sequence has high priority, it cannot be
     // cleared.
@@ -334,7 +345,7 @@ static AnimationSad gAnimationSads[ANIMATION_SAD_LIST_CAPACITY];
 static PathNode gClosedPathNodeList[PATH_NODE_CAPACITY];
 
 // 0x54CC14 anim_set
-static AnimationSequence gAnimationSequences[32];
+static AnimationSequence gAnimationSequences[ANIMATION_SEQUENCE_LIST_CAPACITY];
 
 // 0x561814 seen_tile
 static unsigned char gPathfinderProcessedTiles[5000];
@@ -438,7 +449,7 @@ static int _anim_free_slot(int requestOptions)
         }
 
         return -1;
-    } else if ((requestOptions & ANIMATION_REQUEST_RESERVED) != 0 || v2 < 20) {
+    } else if ((requestOptions & ANIMATION_REQUEST_RESERVED) != 0 || v2 < ANIMATION_NON_RESERVED_LIST_CAPACITY) {
         return v1;
     }
 
@@ -473,7 +484,7 @@ int reg_anim_clear(Object* a1)
         int animationDescriptionIndex;
         for (animationDescriptionIndex = 0; animationDescriptionIndex < animationSequence->length; animationDescriptionIndex++) {
             AnimationDescription* animationDescription = &(animationSequence->animations[animationDescriptionIndex]);
-            if (a1 != animationDescription->owner || animationDescription->kind == 11) {
+            if (a1 != animationDescription->owner || animationKindIsCallback(animationDescription->kind)) {
                 continue;
             }
 
@@ -587,7 +598,7 @@ static int _check_registry(Object* obj)
         if (animationSequenceIndex != gAnimationSequenceCurrentIndex && animationSequence->step != ANIM_COMPLETE) {
             for (int animationDescriptionIndex = 0; animationDescriptionIndex < animationSequence->length; animationDescriptionIndex++) {
                 AnimationDescription* animationDescription = &(animationSequence->animations[animationDescriptionIndex]);
-                if (obj == animationDescription->owner && animationDescription->kind != 11) {
+                if (obj == animationDescription->owner && !animationKindIsCallback(animationDescription->kind)) {
                     if ((animationSequence->flags & ANIM_SEQ_INSIGNIFICANT) == 0) {
                         return -1;
                     }
@@ -619,7 +630,7 @@ int animationIsBusy(Object* a1)
                     continue;
                 }
 
-                if (animationDescription->kind == ANIM_KIND_CALLBACK) {
+                if (animationKindIsCallback(animationDescription->kind)) {
                     continue;
                 }
 
@@ -1609,12 +1620,25 @@ static int _anim_set_end(int animationSequenceIndex)
         }
     }
 
+    std::array<Object*, ANIMATION_DESCRIPTION_LIST_CAPACITY> destroyedOwners = {};
+    std::size_t destroyedOwnersLength = 0;
+    auto ownerWasDestroyed = [&destroyedOwners, &destroyedOwnersLength](Object* owner) {
+        return std::find(destroyedOwners.begin(), destroyedOwners.begin() + destroyedOwnersLength, owner) != destroyedOwners.begin() + destroyedOwnersLength;
+    };
+
     for (i = 0; i < animationSequence->length; i++) {
         animationDescription = &(animationSequence->animations[i]);
         if (animationDescription->kind == ANIM_KIND_HIDE && ((i < animationSequence->animationIndex) || (animationDescription->extendedFlags & ANIMATION_SEQUENCE_FORCED))) {
+            Object* owner = animationDescription->owner;
+            if (ownerWasDestroyed(owner)) {
+                continue;
+            }
+
+            destroyedOwners[destroyedOwnersLength++] = owner;
+
             Rect rect;
-            int elevation = animationDescription->owner->elevation;
-            objectDestroy(animationDescription->owner, &rect);
+            int elevation = owner->elevation;
+            objectDestroy(owner, &rect);
             tileWindowRefreshRect(&rect, elevation);
         }
     }
@@ -1623,18 +1647,23 @@ static int _anim_set_end(int animationSequenceIndex)
         animationDescription = &(animationSequence->animations[i]);
         if (animationDescription->artCacheKey) {
             artUnlock(animationDescription->artCacheKey);
+            animationDescription->artCacheKey = nullptr;
         }
 
-        if (animationDescription->kind != 11 && animationDescription->kind != 12) {
+        if (!animationKindIsCallback(animationDescription->kind)) {
             // TODO: Check.
             if (animationDescription->kind != ANIM_KIND_PING) {
                 Object* owner = animationDescription->owner;
+                if (ownerWasDestroyed(owner)) {
+                    continue;
+                }
+
                 if (FID_TYPE(owner->fid) == OBJ_TYPE_CRITTER) {
                     int j = 0;
                     for (; j < i; j++) {
                         AnimationDescription* ad = &(animationSequence->animations[j]);
                         if (owner == ad->owner) {
-                            if (ad->kind != ANIM_KIND_CALLBACK && ad->kind != ANIM_KIND_CALLBACK3) {
+                            if (!animationKindIsCallback(ad->kind)) {
                                 break;
                             }
                         }

src/art.cc

@@ -47,6 +47,7 @@ static int artReadFrameData(unsigned char* data, File* stream, int count, int* p
 static int artReadHeader(Art* art, File* stream);
 static int artGetDataSize(const Art* art);
 static int paddingForSize(int size);
+static char artGetCritterWeaponCode(int weaponType);
 
 // A frame is laid out like [ArtFrame header][pixel bytes][padding].
 // These functions return a pointer to the pixel bytes, but must be given a pointer to a frame header,
@@ -583,7 +584,7 @@ int _art_get_code(int animation, int weaponType, char* weaponCodePtr, char* anim
             return -1;
         }
 
-        *weaponCodePtr = 'd' + (weaponType - 1);
+        *weaponCodePtr = artGetCritterWeaponCode(weaponType);
         return 0;
     } else if (animation == ANIM_PRONE_TO_STANDING) {
         *animationCodePtr = 'h';
@@ -625,7 +626,7 @@ int _art_get_code(int animation, int weaponType, char* weaponCodePtr, char* anim
             *weaponCodePtr = 'a';
             *animationCodePtr = 'n';
         } else {
-            *weaponCodePtr = 'd' + (weaponType - 1);
+            *weaponCodePtr = artGetCritterWeaponCode(weaponType);
             *animationCodePtr = 'e';
         }
         return 0;
@@ -633,7 +634,7 @@ int _art_get_code(int animation, int weaponType, char* weaponCodePtr, char* anim
 
     *animationCodePtr = 'a' + animation;
     if (animation <= ANIM_WALK && weaponType > 0) {
-        *weaponCodePtr = 'd' + (weaponType - 1);
+        *weaponCodePtr = artGetCritterWeaponCode(weaponType);
         return 0;
     }
     *weaponCodePtr = 'a';
@@ -641,6 +642,24 @@ int _art_get_code(int animation, int weaponType, char* weaponCodePtr, char* anim
     return 0;
 }
 
+static char artGetCritterWeaponCode(int weaponType)
+{
+    switch (weaponType) {
+    case WEAPON_ANIMATION_SFALL_S:
+        return 's';
+    case WEAPON_ANIMATION_SFALL_O:
+        return 'o';
+    case WEAPON_ANIMATION_SFALL_P:
+        return 'p';
+    case WEAPON_ANIMATION_SFALL_Q:
+        return 'q';
+    case WEAPON_ANIMATION_SFALL_T:
+        return 't';
+    default:
+        return 'd' + (weaponType - 1);
+    }
+}
+
 // 0x419428
 char* artBuildFilePath(int fid)
 {
@@ -957,9 +976,11 @@ int artAliasFid(int fid)
     int anim = FID_ANIM_TYPE(fid);
     if (type == OBJ_TYPE_CRITTER) {
         if (anim == ANIM_ELECTRIFY
+            || anim == ANIM_CHARRED_BODY
             || anim == ANIM_BURNED_TO_NOTHING
             || anim == ANIM_ELECTRIFIED_TO_NOTHING
             || anim == ANIM_ELECTRIFY_SF
+            || anim == ANIM_CHARRED_BODY_SF
             || anim == ANIM_BURNED_TO_NOTHING_SF
             || anim == ANIM_ELECTRIFIED_TO_NOTHING_SF
             || anim == ANIM_FIRE_DANCE
@@ -1010,10 +1031,21 @@ static int artCacheGetFileSizeImpl(int fid, int* sizePtr)
             Art art;
             if (artReadHeader(&art, stream) == 0) {
                 *sizePtr = artGetDataSize(&art);
+                if (*sizePtr < 0) {
+                    debugPrint("ART ERROR: fid %d path %s returned negative data size %d\n", fid, artFilePath, *sizePtr);
+                } else if (*sizePtr > 0x10000) {
+                    debugPrint("ART INFO: fid %d path %s has large data size %d\n", fid, artFilePath, *sizePtr);
+                }
                 result = 0;
+            } else {
+                debugPrint("ART ERROR: artReadHeader failed for fid %d path %s\n", fid, artFilePath);
             }
             fileClose(stream);
+        } else {
+            debugPrint("ART ERROR: could not open file for fid %d path %s\n", fid, artFilePath);
         }
+    } else {
+        debugPrint("ART ERROR: could not build path for fid %d\n", fid);
     }
 
     return result;
@@ -1031,19 +1063,32 @@ static int artCacheReadDataImpl(int fid, int* sizePtr, unsigned char* data)
         if (artGetLocalizedPath(artFileName, &localizedPath)) {
             if (artRead(localizedPath, data) == 0) {
                 loaded = true;
+            } else {
+                debugPrint("ART INFO: artRead localized failed for fid %d path %s\n", fid, localizedPath);
             }
         }
 
         if (!loaded) {
             if (artRead(artFileName, data) == 0) {
                 loaded = true;
+            } else {
+                debugPrint("ART INFO: artRead fallback failed for fid %d path %s\n", fid, artFileName);
             }
         }
 
         if (loaded) {
             *sizePtr = artGetDataSize((Art*)data);
+            if (*sizePtr < 0) {
+                debugPrint("ART ERROR: fid %d path %s read data returned negative size %d\n", fid, artFileName, *sizePtr);
+            } else if (*sizePtr > 0x10000) {
+                debugPrint("ART INFO: fid %d path %s read large data size %d\n", fid, artFileName, *sizePtr);
+            }
             result = 0;
+        } else {
+            debugPrint("ART ERROR: failed to load ART data for fid %d path %s\n", fid, artFileName);
         }
+    } else {
+        debugPrint("ART ERROR: could not build path for fid %d\n", fid);
     }
 
     return result;
@@ -1128,6 +1173,20 @@ static int artReadHeader(Art* art, File* stream)
         art->dataSize = fileGetSize(stream);
     }
 
+    if (art->frameCount < 0) {
+        debugPrint("ART WARNING: negative frameCount %d in header\n", art->frameCount);
+    }
+
+    if (art->dataSize < 0) {
+        debugPrint("ART WARNING: negative dataSize %d in header\n", art->dataSize);
+    }
+
+    for (int rotation = 0; rotation < ROTATION_COUNT; rotation++) {
+        if (art->dataOffsets[rotation] < 0) {
+            debugPrint("ART WARNING: negative dataOffset[%d] %d in header\n", rotation, art->dataOffsets[rotation]);
+        }
+    }
+
     return 0;
 }
 
@@ -1418,6 +1477,13 @@ int artRead(const char* path, unsigned char* data)
         return -3;
     }
 
+    int totalAllocSize = artGetDataSize(art);
+    if (totalAllocSize <= 0) {
+        debugPrint("ART ERROR: artRead computed invalid totalAllocSize %d for %s\n", totalAllocSize, path);
+        fileClose(stream);
+        return -5;
+    }
+
     int currentPadding = paddingForSize(sizeof(Art));
     int previousPadding = 0;
 
@@ -1427,7 +1493,14 @@ int artRead(const char* path, unsigned char* data)
         if (index == 0 || art->dataOffsets[index - 1] != art->dataOffsets[index]) {
             art->padding[index] += previousPadding;
             currentPadding += previousPadding;
-            if (artReadFrameData(data + sizeof(Art) + art->dataOffsets[index] + art->padding[index], stream, art->frameCount, &previousPadding) != 0) {
+            long long frameDataOffset = static_cast<long long>(sizeof(Art)) + art->dataOffsets[index] + art->padding[index];
+            if (frameDataOffset < 0 || frameDataOffset >= totalAllocSize) {
+                debugPrint("ART ERROR: invalid frame data destination for %s at rotation %d: offset=%lld base=%p size=%d\n", path, index, frameDataOffset, data, totalAllocSize);
+                fileClose(stream);
+                return -5;
+            }
+            unsigned char* frameDataPtr = data + frameDataOffset;
+            if (artReadFrameData(frameDataPtr, stream, art->frameCount, &previousPadding) != 0) {
                 fileClose(stream);
                 return -5;
             }

src/art.h

@@ -100,6 +100,11 @@ typedef enum WeaponAnimation {
     WEAPON_ANIMATION_LASER_RIFLE, // k
     WEAPON_ANIMATION_MINIGUN, // l
     WEAPON_ANIMATION_LAUNCHER, // m
+    WEAPON_ANIMATION_SFALL_S, // s
+    WEAPON_ANIMATION_SFALL_O, // o
+    WEAPON_ANIMATION_SFALL_P, // p
+    WEAPON_ANIMATION_SFALL_Q, // q
+    WEAPON_ANIMATION_SFALL_T, // t
     WEAPON_ANIMATION_COUNT,
 } WeaponAnimation;
 

src/automap.cc

@@ -48,6 +48,11 @@ static int _copy_file_data(File* stream1, File* stream2, int length);
 
 static int gAutomapWindow = -1;
 
+static bool automapEntryIsValid(int map, int elevation)
+{
+    return map >= 0 && map < AUTOMAP_MAP_COUNT && elevationIsValid(elevation);
+}
+
 typedef enum AutomapFrm {
     AUTOMAP_FRM_BACKGROUND,
     AUTOMAP_FRM_BUTTON_UP,
@@ -293,6 +298,10 @@ int automapSave(File* stream)
 // 0x41B8B4 automapDisplayMap
 int _automapDisplayMap(int map)
 {
+    if (map < 0 || map >= AUTOMAP_MAP_COUNT) {
+        return -1;
+    }
+
     return _displayMapList[map];
 }
 
@@ -696,6 +705,9 @@ int automapSaveCurrent()
 {
     int map = mapGetCurrentMap();
     int elevation = gElevation;
+    if (!automapEntryIsValid(map, elevation)) {
+        return 0;
+    }
 
     int entryOffset = gAutomapHeader.offsets[map][elevation];
     if (entryOffset < 0) {
@@ -941,6 +953,9 @@ err:
 static int automapLoadEntry(int map, int elevation)
 {
     gAutomapEntry.compressedData = nullptr;
+    if (!automapEntryIsValid(map, elevation)) {
+        return -1;
+    }
 
     char path[COMPAT_MAX_PATH];
     snprintf(path, sizeof(path), "%s\\%s", "MAPS", AUTOMAP_DB);

src/cache.cc

@@ -24,6 +24,7 @@ static bool cacheEntryInit(CacheEntry* cacheEntry);
 static bool cacheEntryFree(Cache* cache, CacheEntry* cacheEntry);
 static bool cacheClean(Cache* cache);
 static bool cacheResetStatistics(Cache* cache);
+static bool cacheValidateHeap(Cache* cache, const char* operation, int key, int size, int heapHandleIndex, const void* data);
 static bool cacheEnsureSize(Cache* cache, int size);
 static bool cacheSweep(Cache* cache);
 static bool cacheSetCapacity(Cache* cache, int newCapacity);
@@ -222,10 +223,21 @@ static bool cacheFetchEntryForKey(Cache* cache, int key, int* indexPtr)
     do {
         int size;
         if (cache->sizeProc(key, &size) != 0) {
+            debugPrint("Cache ERROR: sizeProc failed for key %d\n", key);
             break;
         }
 
+        if (size < 0) {
+            debugPrint("Cache ERROR: invalid size %d for key %d\n", size, key);
+            break;
+        }
+
+        if (size > 0x10000) {
+            debugPrint("Cache INFO: key %d requested large allocation size %d\n", key, size);
+        }
+
         if (!cacheEnsureSize(cache, size)) {
+            debugPrint("Cache INFO: cacheEnsureSize failed for key %d size %d\n", key, size);
             break;
         }
 
@@ -262,16 +274,45 @@ static bool cacheFetchEntryForKey(Cache* cache, int key, int* indexPtr)
             break;
         }
 
+        bool locked = false;
         do {
             if (!heapLock(&(cache->heap), cacheEntry->heapHandleIndex, &(cacheEntry->data))) {
                 break;
             }
 
+            locked = true;
+            int requestedSize = size;
+            int allocatedSize = (requestedSize + 3) & ~3;
+
+            if (!cacheValidateHeap(cache, "after lock before read", key, size, cacheEntry->heapHandleIndex, cacheEntry->data)) {
+                break;
+            }
+
             if (cache->readProc(key, &size, cacheEntry->data) != 0) {
                 break;
             }
 
+            if (!cacheValidateHeap(cache, "after read before unlock", key, size, cacheEntry->heapHandleIndex, cacheEntry->data)) {
+                break;
+            }
+
+            if (size < 0 || size > allocatedSize) {
+                debugPrint("Cache ERROR: readProc returned invalid size for key %d: requested=%d allocated=%d returned=%d handle=%d data=%p\n",
+                    key,
+                    requestedSize,
+                    allocatedSize,
+                    size,
+                    cacheEntry->heapHandleIndex,
+                    cacheEntry->data);
+                break;
+            }
+
             heapUnlock(&(cache->heap), cacheEntry->heapHandleIndex);
+            locked = false;
+
+            if (!cacheValidateHeap(cache, "after unlock", key, size, cacheEntry->heapHandleIndex, cacheEntry->data)) {
+                break;
+            }
 
             cacheEntry->size = size;
             cacheEntry->key = key;
@@ -298,7 +339,9 @@ static bool cacheFetchEntryForKey(Cache* cache, int key, int* indexPtr)
             return true;
         } while (0);
 
-        heapUnlock(&(cache->heap), cacheEntry->heapHandleIndex);
+        if (locked) {
+            heapUnlock(&(cache->heap), cacheEntry->heapHandleIndex);
+        }
     } while (0);
 
     // NOTE: Uninline.
@@ -356,9 +399,9 @@ static int cacheFindIndexForKey(Cache* cache, int key, int* indexPtr)
         }
 
         if (cmp > 0) {
-            l = l + 1;
+            l = mid + 1;
         } else {
-            r = r - 1;
+            r = mid - 1;
         }
     } while (r >= l);
 
@@ -381,6 +424,7 @@ static bool cacheEntryInit(CacheEntry* cacheEntry)
     cacheEntry->hits = 0;
     cacheEntry->flags = 0;
     cacheEntry->mru = 0;
+    cacheEntry->heapHandleIndex = -1;
     return true;
 }
 
@@ -389,8 +433,10 @@ static bool cacheEntryInit(CacheEntry* cacheEntry)
 // 0x420740 cache_destroy_item
 static bool cacheEntryFree(Cache* cache, CacheEntry* cacheEntry)
 {
-    if (cacheEntry->data != nullptr) {
+    if (cacheEntry->heapHandleIndex != -1) {
         heapBlockDeallocate(&(cache->heap), &(cacheEntry->heapHandleIndex));
+        cacheEntry->heapHandleIndex = -1;
+        cacheEntry->data = nullptr;
     }
 
     internal_free(cacheEntry);
@@ -445,6 +491,24 @@ static bool cacheResetStatistics(Cache* cache)
     return true;
 }
 
+static bool cacheValidateHeap(Cache* cache, const char* operation, int key, int size, int heapHandleIndex, const void* data)
+{
+    if (heapValidate(&(cache->heap))) {
+        return true;
+    }
+
+    debugPrint("Cache ERROR: heap validation failed %s: key=%d size=%d handle=%d data=%p cacheSize=%d maxSize=%d entries=%d\n",
+        operation,
+        key,
+        size,
+        heapHandleIndex,
+        data,
+        cache->size,
+        cache->maxSize,
+        cache->entriesLength);
+    return false;
+}
+
 // Prepare cache for storing new entry with the specified size.
 //
 // 0x42084C cache_make_room

src/combat_ai.cc

@@ -2012,26 +2012,7 @@ static Object* _ai_best_weapon(Object* attacker, Object* weapon1, Object* weapon
 // 0x4298EC
 static bool _ai_can_use_weapon(Object* critter, Object* weapon, int hitMode)
 {
-    bool result = true;
-
-    int damageFlags = critter->data.critter.combat.results;
-    if ((damageFlags & DAM_CRIP_ARM_LEFT) != 0 && (damageFlags & DAM_CRIP_ARM_RIGHT) != 0) {
-        result = false;
-    }
-
-    if (result && (damageFlags & DAM_CRIP_ARM_ANY) != 0 && weaponIsTwoHanded(weapon)) {
-        result = false;
-    }
-
-    if (result) {
-        int rotation = critter->rotation + 1;
-        int animationCode = weaponGetAnimationCode(weapon);
-        int weaponAnimationCode = weaponGetAnimationForHitMode(weapon, hitMode);
-        int fid = buildFid(OBJ_TYPE_CRITTER, critter->fid & 0xFFF, weaponAnimationCode, animationCode, rotation);
-        if (!artExists(fid)) {
-            result = false;
-        }
-    }
+    bool result = critterCanUseWeapon(critter, weapon, hitMode);
 
     AiPacket* ai = aiGetPacket(critter);
     if (result) {

src/critter.cc

@@ -8,6 +8,7 @@
 #include "art.h"
 #include "character_editor.h"
 #include "combat.h"
+#include "combat_defs.h"
 #include "debug.h"
 #include "display_monitor.h"
 #include "endgame.h"
@@ -1023,6 +1024,30 @@ int critterGetBodyType(Object* critter)
     return proto->critter.data.bodyType;
 }
 
+bool critterCanUseWeapon(Object* critter, Object* weapon, int hitMode)
+{
+    if (critter == nullptr || weapon == nullptr || itemGetType(weapon) != ITEM_TYPE_WEAPON) {
+        return false;
+    }
+
+    int damageFlags = critter->data.critter.combat.results;
+    if ((damageFlags & DAM_CRIP_ARM_ANY) == DAM_CRIP_ARM_ANY) {
+        // both limbs are crippled
+        return false;
+    }
+
+    if ((damageFlags & DAM_CRIP_ARM_ANY) != 0 && weaponIsTwoHanded(weapon)) {
+        return false;
+    }
+
+    // verify art exists
+    int rotation = critter->rotation + 1;
+    int animationCode = weaponGetAnimationCode(weapon);
+    int weaponAnimationCode = weaponGetAnimationForHitMode(weapon, hitMode);
+    int fid = buildFid(OBJ_TYPE_CRITTER, critter->fid & 0xFFF, weaponAnimationCode, animationCode, rotation);
+    return artExists(fid);
+}
+
 int critterBuildGorisFid(Object* critter, int frmId)
 {
     assert(critter != nullptr && critter->pid == PROTO_ID_GORIS);

src/critter.h

@@ -54,6 +54,9 @@ bool critterIsDead(Object* critter);
 bool critterIsCrippled(Object* critter);
 bool critterIsProne(Object* critter);
 int critterGetBodyType(Object* critter);
+// Checks physical/art capability only. Callers that expose weapon usability
+// decisions must still call scriptHooks_CanUseWeapon with the final result.
+bool critterCanUseWeapon(Object* critter, Object* weapon, int hitMode);
 int critterBuildGorisFid(Object* critter, int frmId);
 int gcdLoad(const char* path);
 int protoCritterDataRead(File* stream, CritterProtoData* critterData);

src/datafile.cc

@@ -49,10 +49,9 @@ void datafileRemapPixelsRgb8(uint8_t* data, uint8_t* palette, int width, int hei
 
     indexedPalette[0] = 0;
     for (int index = 1; index < INDEXED_PALETTE_MAX; index++) {
-        // TODO: Check.
-        int r = palette[index * 3 + 2] >> 3;
+        int r = palette[index * 3] >> 3;
         int g = palette[index * 3 + 1] >> 3;
-        int b = palette[index * 3] >> 3;
+        int b = palette[index * 3 + 2] >> 3;
         int colorTableIndex = (r << 10) | (g << 5) | b;
         indexedPalette[index] = _colorTable[colorTableIndex];
     }
@@ -72,10 +71,9 @@ void datafileRemapPixelsRgb6(uint8_t* data, uint8_t* palette, int width, int hei
 
     indexedPalette[0] = 0;
     for (int index = 1; index < INDEXED_PALETTE_MAX; index++) {
-        // TODO: Check.
-        int r = palette[index * 3 + 2] >> 1;
+        int r = palette[index * 3] >> 1;
         int g = palette[index * 3 + 1] >> 1;
-        int b = palette[index * 3] >> 1;
+        int b = palette[index * 3 + 2] >> 1;
         int colorTableIndex = (r << 10) | (g << 5) | b;
         indexedPalette[index] = _colorTable[colorTableIndex];
     }

src/game_dialog.cc

@@ -3924,7 +3924,7 @@ void partyMemberControlWindowHandleEvents()
                 dialogMode = GAME_DIALOG_MODE_TALK;
                 return;
             } else if (keyCode == KEY_LOWERCASE_A) {
-                if (gGameDialogSpeaker->pid != 0x10000A1) {
+                if (partyMemberPidCanEquipArmor(gGameDialogSpeaker->pid)) {
                     Object* armor = _ai_search_inven_armor(gGameDialogSpeaker);
                     if (armor != nullptr) {
                         inventoryEquip(gGameDialogSpeaker, armor, 0);

src/game_mouse.cc

@@ -1,6 +1,7 @@
 #include "game_mouse.h"
 
 #include <assert.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <string.h>
 
@@ -17,6 +18,7 @@
 #include "combat.h"
 #include "content_config.h"
 #include "critter.h"
+#include "debug.h"
 #include "draw.h"
 #include "game.h"
 #include "game_sound.h"
@@ -1190,14 +1192,16 @@ void _gmouse_handle_event(int mouseX, int mouseY, int mouseState)
                         mouseGetPosition(&updatedMouseX, &updatedMouseY);
 
                         if (abs(updatedMouseY - newMouseY) > 10) {
+                            int nextActionIndex;
                             if (newMouseY >= updatedMouseY) {
-                                actionIndex -= 1;
+                                nextActionIndex = actionIndex - 1;
                             } else {
-                                actionIndex += 1;
+                                nextActionIndex = actionIndex + 1;
                             }
 
-                            if (gameMouseHighlightActionMenuItemAtIndex(actionIndex) == 0) {
+                            if (gameMouseHighlightActionMenuItemAtIndex(nextActionIndex) == 0) {
                                 tileWindowRefreshRect(&cursorRect, gElevation);
+                                actionIndex = nextActionIndex;
                             }
                             newMouseY = updatedMouseY;
                         }
@@ -1840,6 +1844,85 @@ int gameMouseRenderActionMenuItems(int x, int y, const int* menuItems, int menuI
     int menuItemWidth = artGetWidth(menuItemFrms[0], 0, 0);
     int menuItemHeight = artGetHeight(menuItemFrms[0], 0, 0);
 
+    auto unlockActionMenuArt = [&]() {
+        artUnlock(arrowFrmHandle);
+
+        for (int index = 0; index < menuItemsLength; index++) {
+            artUnlock(menuItemFrmHandles[index]);
+        }
+    };
+
+    auto blitFitsActionMenuFrame = [&](const char* label, unsigned char* dest, int blitWidth, int blitHeight) {
+        if (gGameMouseActionMenuFrmData == nullptr || gGameMouseActionMenuFrmWidth <= 0 || gGameMouseActionMenuFrmHeight <= 0 || gGameMouseActionMenuFrmDataSize <= 0) {
+            debugPrint("Action menu ERROR: invalid frame buffer before %s blit: data=%p width=%d height=%d size=%d\n",
+                label,
+                gGameMouseActionMenuFrmData,
+                gGameMouseActionMenuFrmWidth,
+                gGameMouseActionMenuFrmHeight,
+                gGameMouseActionMenuFrmDataSize);
+            return false;
+        }
+
+        if (dest == nullptr || blitWidth <= 0 || blitHeight <= 0 || blitWidth > gGameMouseActionMenuFrmWidth) {
+            debugPrint("Action menu ERROR: invalid %s blit args: dest=%p size=%dx%d frame=%dx%d\n",
+                label,
+                dest,
+                blitWidth,
+                blitHeight,
+                gGameMouseActionMenuFrmWidth,
+                gGameMouseActionMenuFrmHeight);
+            return false;
+        }
+
+        intptr_t offset = reinterpret_cast<intptr_t>(dest) - reinterpret_cast<intptr_t>(gGameMouseActionMenuFrmData);
+        if (offset < 0 || offset >= gGameMouseActionMenuFrmDataSize) {
+            debugPrint("Action menu ERROR: %s blit destination out of frame: offset=%lld frameSize=%d mouse=%d,%d menuItems=%d screen=%dx%d\n",
+                label,
+                static_cast<long long>(offset),
+                gGameMouseActionMenuFrmDataSize,
+                x,
+                y,
+                menuItemsLength,
+                width,
+                height);
+            return false;
+        }
+
+        int destX = static_cast<int>(offset % gGameMouseActionMenuFrmWidth);
+        int destY = static_cast<int>(offset / gGameMouseActionMenuFrmWidth);
+        if (destX + blitWidth > gGameMouseActionMenuFrmWidth || destY + blitHeight > gGameMouseActionMenuFrmHeight) {
+            debugPrint("Action menu ERROR: %s blit would overflow frame: dest=%d,%d size=%dx%d frame=%dx%d mouse=%d,%d menuItems=%d screen=%dx%d\n",
+                label,
+                destX,
+                destY,
+                blitWidth,
+                blitHeight,
+                gGameMouseActionMenuFrmWidth,
+                gGameMouseActionMenuFrmHeight,
+                x,
+                y,
+                menuItemsLength,
+                width,
+                height);
+            return false;
+        }
+
+        return true;
+    };
+
+    if (arrowWidth + menuItemWidth > gGameMouseActionMenuFrmWidth || std::max(arrowHeight, menuItemsLength * menuItemHeight) > gGameMouseActionMenuFrmHeight) {
+        debugPrint("Action menu ERROR: menu art does not fit frame: arrow=%dx%d item=%dx%d items=%d frame=%dx%d\n",
+            arrowWidth,
+            arrowHeight,
+            menuItemWidth,
+            menuItemHeight,
+            menuItemsLength,
+            gGameMouseActionMenuFrmWidth,
+            gGameMouseActionMenuFrmHeight);
+        unlockActionMenuArt();
+        return -1;
+    }
+
     _gmouse_3d_menu_frame_hot_x = 0;
     _gmouse_3d_menu_frame_hot_y = 0;
 
@@ -1848,6 +1931,23 @@ int gameMouseRenderActionMenuItems(int x, int y, const int* menuItems, int menuI
 
     int maxY = y + menuItemsLength * menuItemHeight - 1;
     int shiftY = maxY - height + 2;
+    int maxArrowShiftY = gGameMouseActionMenuFrmHeight - arrowHeight;
+    if (shiftY > maxArrowShiftY) {
+        debugPrint("Action menu INFO: clamping arrow shift: requested=%d max=%d mouse=%d,%d menuItems=%d screen=%dx%d frame=%dx%d arrow=%dx%d\n",
+            shiftY,
+            maxArrowShiftY,
+            x,
+            y,
+            menuItemsLength,
+            width,
+            height,
+            gGameMouseActionMenuFrmWidth,
+            gGameMouseActionMenuFrmHeight,
+            arrowWidth,
+            arrowHeight);
+        shiftY = maxArrowShiftY;
+    }
+
     unsigned char* arrowFrmDest = gGameMouseActionMenuFrmData;
     unsigned char* menuItemFrmDest = arrowFrmDest;
 
@@ -1865,6 +1965,13 @@ int gameMouseRenderActionMenuItems(int x, int y, const int* menuItems, int menuI
         artUnlock(arrowFrmHandle);
         fid = buildFid(OBJ_TYPE_INTERFACE, 285, 0, 0, 0);
         arrowFrm = artLock(fid, &arrowFrmHandle);
+        if (arrowFrm == nullptr) {
+            for (int index = 0; index < menuItemsLength; index++) {
+                artUnlock(menuItemFrmHandles[index]);
+            }
+            return -1;
+        }
+
         arrowData = artGetFrameData(arrowFrm, 0, 0);
         arrowFrmDest += menuItemWidth;
 
@@ -1878,20 +1985,26 @@ int gameMouseRenderActionMenuItems(int x, int y, const int* menuItems, int menuI
     }
 
     memset(gGameMouseActionMenuFrmData, 0, gGameMouseActionMenuFrmDataSize);
-    blitBufferToBuffer(arrowData, arrowWidth, arrowHeight, arrowWidth, arrowFrmDest, gGameMouseActionPickFrmWidth);
+    if (!blitFitsActionMenuFrame("arrow", arrowFrmDest, arrowWidth, arrowHeight)) {
+        unlockActionMenuArt();
+        return -1;
+    }
+
+    blitBufferToBuffer(arrowData, arrowWidth, arrowHeight, arrowWidth, arrowFrmDest, gGameMouseActionMenuFrmWidth);
 
     unsigned char* dest = menuItemFrmDest;
     for (int index = 0; index < menuItemsLength; index++) {
         unsigned char* data = artGetFrameData(menuItemFrms[index], 0, 0);
-        blitBufferToBuffer(data, menuItemWidth, menuItemHeight, menuItemWidth, dest, gGameMouseActionPickFrmWidth);
+        if (!blitFitsActionMenuFrame("item", dest, menuItemWidth, menuItemHeight)) {
+            unlockActionMenuArt();
+            return -1;
+        }
+
+        blitBufferToBuffer(data, menuItemWidth, menuItemHeight, menuItemWidth, dest, gGameMouseActionMenuFrmWidth);
         dest += gGameMouseActionMenuFrmWidth * menuItemHeight;
     }
 
-    artUnlock(arrowFrmHandle);
-
-    for (int index = 0; index < menuItemsLength; index++) {
-        artUnlock(menuItemFrmHandles[index]);
-    }
+    unlockActionMenuArt();
 
     memcpy(gGameMouseActionMenuItems, menuItems, sizeof(*gGameMouseActionMenuItems) * menuItemsLength);
     gGameMouseActionMenuItemsLength = menuItemsLength;

src/heap.cc

@@ -1,5 +1,7 @@
 #include "heap.h"
 
+#include <limits.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -307,12 +309,13 @@ bool heapBlockAllocate(Heap* heap, int* handleIndexPtr, int size, int disallowSy
     int blockSize;
     HeapHandle* handle;
 
-    size += 4 - size % 4;
-
     if (heap == nullptr || handleIndexPtr == nullptr || size == 0) {
         goto err;
     }
 
+    // Keep subsequent block headers aligned when callers request odd-sized payloads.
+    size = (size + 3) & ~3;
+
     if (disallowSystemAllocation != 0 && disallowSystemAllocation != 1) {
         disallowSystemAllocation = 0;
     }
@@ -429,7 +432,17 @@ bool heapBlockDeallocate(Heap* heap, int* handleIndexPtr)
 
     int handleIndex = *handleIndexPtr;
 
+    if (heap->handles == nullptr || handleIndex < 0 || handleIndex >= heap->handlesLength) {
+        debugPrint("Heap Error: Invalid handle during deallocate.\n");
+        debugPrint("Heap Error: Could not deallocate block.\n");
+        return false;
+    }
+
     HeapHandle* handle = &(heap->handles[handleIndex]);
+    if (handle->data == nullptr) {
+        debugPrint("Heap Error: Null handle data detected during deallocate.\n");
+        return false;
+    }
 
     HeapBlockHeader* blockHeader = (HeapBlockHeader*)handle->data;
     if (blockHeader->guard != HEAP_BLOCK_HEADER_GUARD) {
@@ -493,12 +506,22 @@ bool heapBlockDeallocate(Heap* heap, int* handleIndexPtr)
 // 0x452DE0 heap_lock
 bool heapLock(Heap* heap, int handleIndex, unsigned char** bufferPtr)
 {
-    if (heap == nullptr) {
+    if (heap == nullptr || bufferPtr == nullptr) {
         debugPrint("Heap Error: Could not lock block");
         return false;
     }
 
+    if (heap->handles == nullptr || handleIndex < 0 || handleIndex >= heap->handlesLength) {
+        debugPrint("Heap Error: Invalid handle during lock.\n");
+        debugPrint("Heap Error: Could not lock block.\n");
+        return false;
+    }
+
     HeapHandle* handle = &(heap->handles[handleIndex]);
+    if (handle->data == nullptr) {
+        debugPrint("Heap Error: Null handle data detected during lock.\n");
+        return false;
+    }
 
     HeapBlockHeader* blockHeader = (HeapBlockHeader*)handle->data;
     if (blockHeader->guard != HEAP_BLOCK_HEADER_GUARD) {
@@ -559,7 +582,17 @@ bool heapUnlock(Heap* heap, int handleIndex)
         return false;
     }
 
+    if (heap->handles == nullptr || handleIndex < 0 || handleIndex >= heap->handlesLength) {
+        debugPrint("Heap Error: Invalid handle during unlock.\n");
+        debugPrint("Heap Error: Could not unlock block.\n");
+        return false;
+    }
+
     HeapHandle* handle = &(heap->handles[handleIndex]);
+    if (handle->data == nullptr) {
+        debugPrint("Heap Error: Null handle data detected during unlock.\n");
+        return false;
+    }
 
     HeapBlockHeader* blockHeader = (HeapBlockHeader*)handle->data;
     if (blockHeader->guard != HEAP_BLOCK_HEADER_GUARD) {
@@ -984,6 +1017,11 @@ static int heapMoveableExtentsCompareBySize(const void* leftPtr, const void* rig
 // 0x453BC4 heap_build_free_list
 static bool heapBuildFreeBlocksList(Heap* heap)
 {
+    if (heap == nullptr || heap->data == nullptr || heap->size <= 0) {
+        debugPrint("Heap Error: Invalid heap during build free list.\n");
+        return false;
+    }
+
     if (heap->freeBlocks == 0) {
         return false;
     }
@@ -1001,6 +1039,11 @@ static bool heapBuildFreeBlocksList(Heap* heap)
     int blocksLength = heap->moveableBlocks + heap->freeBlocks + heap->lockedBlocks;
 
     unsigned char* ptr = heap->data;
+    uintptr_t heapStartAddress = reinterpret_cast<uintptr_t>(heap->data);
+    uintptr_t heapEndAddress = heapStartAddress + static_cast<uintptr_t>(heap->size);
+    if (heapEndAddress < heapStartAddress) {
+        debugPrint("Heap ERROR: heap address range overflow during free list build: data=%p size=%d\n", heap->data, heap->size);
+    }
 
     int freeBlockIndex = 0;
     while (blocksLength != 0) {
@@ -1008,19 +1051,49 @@ static bool heapBuildFreeBlocksList(Heap* heap)
             break;
         }
 
+        uintptr_t ptrAddress = reinterpret_cast<uintptr_t>(ptr);
+        if (ptrAddress < heapStartAddress || ptrAddress > heapEndAddress || heapEndAddress - ptrAddress < HEAP_BLOCK_HEADER_SIZE) {
+            debugPrint("Heap ERROR: invalid heap block pointer during free list build: ptr=%p heapEnd=%p blocksLength=%d freeBlocks=%d moveableBlocks=%d lockedBlocks=%d\n", ptr, reinterpret_cast<void*>(heapEndAddress), blocksLength, heap->freeBlocks, heap->moveableBlocks, heap->lockedBlocks);
+        }
+
         HeapBlockHeader* blockHeader = (HeapBlockHeader*)ptr;
+        if (blockHeader->guard != HEAP_BLOCK_HEADER_GUARD) {
+            debugPrint("Heap ERROR: bad block header guard during free list build: ptr=%p guard=%08X\n", ptr, blockHeader->guard);
+        }
+        if (blockHeader->size < 0 || heapEndAddress - ptrAddress < HEAP_BLOCK_OVERHEAD_SIZE || static_cast<uintptr_t>(blockHeader->size) > heapEndAddress - ptrAddress - HEAP_BLOCK_OVERHEAD_SIZE) {
+            debugPrint("Heap ERROR: invalid block size during free list build: ptr=%p size=%d heapEnd=%p\n", ptr, blockHeader->size, reinterpret_cast<void*>(heapEndAddress));
+        }
+
+        uintptr_t blockExtentSize = static_cast<uintptr_t>(blockHeader->size) + HEAP_BLOCK_OVERHEAD_SIZE;
         if (blockHeader->state == HEAP_BLOCK_STATE_FREE) {
             // Join consecutive free blocks if any.
             while (blocksLength > 1) {
                 // Grab next block and check if's a free block.
-                HeapBlockHeader* nextBlockHeader = (HeapBlockHeader*)(ptr + blockHeader->size + HEAP_BLOCK_OVERHEAD_SIZE);
+                uintptr_t nextBlockAddress = ptrAddress + blockExtentSize;
+                if (nextBlockAddress < heapStartAddress || nextBlockAddress > heapEndAddress || heapEndAddress - nextBlockAddress < HEAP_BLOCK_HEADER_SIZE) {
+                    debugPrint("Heap ERROR: next block pointer out of range during free list join: nextPtr=%p heapEnd=%p\n", reinterpret_cast<void*>(nextBlockAddress), reinterpret_cast<void*>(heapEndAddress));
+                }
+                unsigned char* nextBlockPtr = reinterpret_cast<unsigned char*>(nextBlockAddress);
+                HeapBlockHeader* nextBlockHeader = (HeapBlockHeader*)nextBlockPtr;
+                if (nextBlockHeader->guard != HEAP_BLOCK_HEADER_GUARD) {
+                    debugPrint("Heap ERROR: bad next block header guard during free list join: nextPtr=%p guard=%08X\n", nextBlockPtr, nextBlockHeader->guard);
+                }
+                if (nextBlockHeader->size < 0 || heapEndAddress - nextBlockAddress < HEAP_BLOCK_OVERHEAD_SIZE || static_cast<uintptr_t>(nextBlockHeader->size) > heapEndAddress - nextBlockAddress - HEAP_BLOCK_OVERHEAD_SIZE) {
+                    debugPrint("Heap ERROR: invalid next block size during free list join: nextPtr=%p size=%d heapEnd=%p\n", nextBlockPtr, nextBlockHeader->size, reinterpret_cast<void*>(heapEndAddress));
+                }
                 if (nextBlockHeader->state != HEAP_BLOCK_STATE_FREE) {
                     break;
                 }
 
+                long long joinedSize = static_cast<long long>(blockHeader->size) + nextBlockHeader->size + HEAP_BLOCK_OVERHEAD_SIZE;
+                if (joinedSize > INT_MAX) {
+                    debugPrint("Heap ERROR: joined free block size overflow during free list join: ptr=%p size=%d nextSize=%d\n", ptr, blockHeader->size, nextBlockHeader->size);
+                }
+
                 // Accumulate it's size plus size of the overhead in the main
                 // block.
-                blockHeader->size += nextBlockHeader->size + HEAP_BLOCK_OVERHEAD_SIZE;
+                blockHeader->size = static_cast<int>(joinedSize);
+                blockExtentSize = static_cast<uintptr_t>(blockHeader->size) + HEAP_BLOCK_OVERHEAD_SIZE;
 
                 // Update heap stats, the free size increased because we've just
                 // remove overhead for one block.
@@ -1034,7 +1107,8 @@ static bool heapBuildFreeBlocksList(Heap* heap)
         }
 
         // Move pointer to the header of the next block.
-        ptr += blockHeader->size + HEAP_BLOCK_OVERHEAD_SIZE;
+        ptrAddress += blockExtentSize;
+        ptr = reinterpret_cast<unsigned char*>(ptrAddress);
 
         blocksLength--;
     }
@@ -1053,6 +1127,11 @@ static int heapBlockCompareBySize(const void* leftPtr, const void* rightPtr)
 // 0x453CD0 heap_build_moveable_list
 static bool heapBuildMoveableExtentsList(Heap* heap, int* moveableExtentsLengthPtr, int* maxBlocksLengthPtr)
 {
+    if (heap == nullptr || heap->data == nullptr || moveableExtentsLengthPtr == nullptr || maxBlocksLengthPtr == nullptr) {
+        debugPrint("Heap Error: Invalid heap state during build moveable list.\n");
+        return false;
+    }
+
     // Calculate max number of extents. It's only possible when every
     // free or moveable block is followed by locked block.
     int maxExtentsCount = heap->moveableBlocks + heap->freeBlocks;
@@ -1137,10 +1216,22 @@ static bool heapBuildMoveableExtentsList(Heap* heap, int* moveableExtentsLengthP
 // 0x452FC4 heap_validate
 bool heapValidate(Heap* heap)
 {
-    debugPrint("Validating heap...\n");
+    if (heap == nullptr || heap->data == nullptr || heap->size <= 0) {
+        debugPrint("Heap ERROR: invalid heap during validate: heap=%p data=%p size=%d\n",
+            heap,
+            heap != nullptr ? heap->data : nullptr,
+            heap != nullptr ? heap->size : 0);
+        return false;
+    }
 
     int blocksCount = heap->freeBlocks + heap->moveableBlocks + heap->lockedBlocks;
     unsigned char* ptr = heap->data;
+    uintptr_t heapStartAddress = reinterpret_cast<uintptr_t>(heap->data);
+    uintptr_t heapEndAddress = heapStartAddress + static_cast<uintptr_t>(heap->size);
+    if (heapEndAddress < heapStartAddress) {
+        debugPrint("Heap ERROR: heap address range overflow during validate: data=%p size=%d\n", heap->data, heap->size);
+        return false;
+    }
 
     int freeBlocks = 0;
     int freeSize = 0;
@@ -1150,15 +1241,52 @@ bool heapValidate(Heap* heap)
     int lockedSize = 0;
 
     for (int index = 0; index < blocksCount; index++) {
+        uintptr_t ptrAddress = reinterpret_cast<uintptr_t>(ptr);
+        if (ptrAddress < heapStartAddress || ptrAddress > heapEndAddress || heapEndAddress - ptrAddress < HEAP_BLOCK_HEADER_SIZE) {
+            debugPrint("Heap ERROR: invalid heap block pointer during validate: index=%d ptr=%p heapEnd=%p blocks=%d freeBlocks=%d moveableBlocks=%d lockedBlocks=%d\n",
+                index,
+                ptr,
+                reinterpret_cast<void*>(heapEndAddress),
+                blocksCount,
+                heap->freeBlocks,
+                heap->moveableBlocks,
+                heap->lockedBlocks);
+            return false;
+        }
+
         HeapBlockHeader* blockHeader = (HeapBlockHeader*)ptr;
         if (blockHeader->guard != HEAP_BLOCK_HEADER_GUARD) {
-            debugPrint("Bad guard begin detected during validate.\n");
+            debugPrint("Heap ERROR: bad block header guard during validate: index=%d ptr=%p guard=%08X size=%d state=%u handle=%d\n",
+                index,
+                ptr,
+                blockHeader->guard,
+                blockHeader->size,
+                blockHeader->state,
+                blockHeader->handle_index);
+            return false;
+        }
+
+        if (blockHeader->size < 0 || heapEndAddress - ptrAddress < HEAP_BLOCK_OVERHEAD_SIZE || static_cast<uintptr_t>(blockHeader->size) > heapEndAddress - ptrAddress - HEAP_BLOCK_OVERHEAD_SIZE) {
+            debugPrint("Heap ERROR: invalid block size during validate: index=%d ptr=%p size=%d state=%u handle=%d heapEnd=%p\n",
+                index,
+                ptr,
+                blockHeader->size,
+                blockHeader->state,
+                blockHeader->handle_index,
+                reinterpret_cast<void*>(heapEndAddress));
             return false;
         }
 
         HeapBlockFooter* blockFooter = (HeapBlockFooter*)(ptr + blockHeader->size + HEAP_BLOCK_HEADER_SIZE);
         if (blockFooter->guard != HEAP_BLOCK_FOOTER_GUARD) {
-            debugPrint("Bad guard end detected during validate.\n");
+            debugPrint("Heap ERROR: bad block footer guard during validate: index=%d ptr=%p footer=%p guard=%08X size=%d state=%u handle=%d\n",
+                index,
+                ptr,
+                blockFooter,
+                blockFooter->guard,
+                blockHeader->size,
+                blockHeader->state,
+                blockHeader->handle_index);
             return false;
         }
 
@@ -1171,17 +1299,34 @@ bool heapValidate(Heap* heap)
         } else if (blockHeader->state == HEAP_BLOCK_STATE_LOCKED) {
             lockedBlocks++;
             lockedSize += blockHeader->size;
+        } else {
+            debugPrint("Heap ERROR: invalid block state during validate: index=%d ptr=%p size=%d state=%u handle=%d\n",
+                index,
+                ptr,
+                blockHeader->size,
+                blockHeader->state,
+                blockHeader->handle_index);
+            return false;
         }
 
-        if (index != blocksCount - 1) {
-            ptr += blockHeader->size + HEAP_BLOCK_OVERHEAD_SIZE;
-            if (ptr > (heap->data + heap->size)) {
-                debugPrint("Ran off end of heap during validate!\n");
-                return false;
-            }
+        ptr += blockHeader->size + HEAP_BLOCK_OVERHEAD_SIZE;
+        if (reinterpret_cast<uintptr_t>(ptr) > heapEndAddress) {
+            debugPrint("Heap ERROR: ran off end of heap during validate: index=%d nextPtr=%p heapEnd=%p\n",
+                index,
+                ptr,
+                reinterpret_cast<void*>(heapEndAddress));
+            return false;
         }
     }
 
+    if (reinterpret_cast<uintptr_t>(ptr) != heapEndAddress) {
+        debugPrint("Heap ERROR: block traversal did not end at heap boundary: ptr=%p heapEnd=%p blocks=%d\n",
+            ptr,
+            reinterpret_cast<void*>(heapEndAddress),
+            blocksCount);
+        return false;
+    }
+
     if (freeBlocks != heap->freeBlocks) {
         debugPrint("Invalid number of free blocks.\n");
         return false;
@@ -1212,23 +1357,42 @@ bool heapValidate(Heap* heap)
         return false;
     }
 
-    debugPrint("Heap is O.K.\n");
-
     int systemBlocks = 0;
     int systemSize = 0;
 
     for (int handleIndex = 0; handleIndex < heap->handlesLength; handleIndex++) {
         HeapHandle* handle = &(heap->handles[handleIndex]);
         if (handle->state != HEAP_HANDLE_STATE_INVALID && (handle->state & HEAP_BLOCK_STATE_SYSTEM) != 0) {
+            if (handle->data == nullptr) {
+                debugPrint("Heap ERROR: null system handle data during validate: handle=%d state=%u\n", handleIndex, handle->state);
+                return false;
+            }
+
             HeapBlockHeader* blockHeader = (HeapBlockHeader*)handle->data;
             if (blockHeader->guard != HEAP_BLOCK_HEADER_GUARD) {
-                debugPrint("Bad guard begin detected in system block during validate.\n");
+                debugPrint("Heap ERROR: bad system block header guard during validate: handle=%d ptr=%p guard=%08X\n",
+                    handleIndex,
+                    handle->data,
+                    blockHeader->guard);
+                return false;
+            }
+
+            if (blockHeader->size < 0) {
+                debugPrint("Heap ERROR: invalid system block size during validate: handle=%d ptr=%p size=%d\n",
+                    handleIndex,
+                    handle->data,
+                    blockHeader->size);
                 return false;
             }
 
             HeapBlockFooter* blockFooter = (HeapBlockFooter*)(handle->data + blockHeader->size + HEAP_BLOCK_HEADER_SIZE);
             if (blockFooter->guard != HEAP_BLOCK_FOOTER_GUARD) {
-                debugPrint("Bad guard end detected in system block during validate.\n");
+                debugPrint("Heap ERROR: bad system block footer guard during validate: handle=%d ptr=%p footer=%p guard=%08X size=%d\n",
+                    handleIndex,
+                    handle->data,
+                    blockFooter,
+                    blockFooter->guard,
+                    blockHeader->size);
                 return false;
             }
 

src/interpreter.cc

@@ -227,8 +227,11 @@ int _interpretOutput(const char* format, ...)
     va_start(args, format);
     const int rc = vsnprintf(string, sizeof(string), format, args);
     va_end(args);
+    if (rc < 0) {
+        string[0] = '\0';
+    }
 
-    debugPrint(string);
+    debugPrint("%s", string);
 
     return rc;
 }
@@ -258,7 +261,10 @@ static char* programGetCurrentProcedureName(Program* program)
 static void programPrintError(const char* format, va_list args)
 {
     char string[260];
-    vsnprintf(string, sizeof(string), format, args);
+    if (vsnprintf(string, sizeof(string), format, args) < 0) {
+        string[0] = '\0';
+    }
+
     debugPrint("\nError during execution: %s\n", string);
 
     if (gInterpreterCurrentProgram == nullptr) {
@@ -490,9 +496,7 @@ Program* programCreateByPath(const char* path)
 {
     File* stream = fileOpen(path, "rb");
     if (stream == nullptr) {
-        char err[260];
-        snprintf(err, sizeof(err), "Couldn't open %s for read\n", path);
-        programFatalError(err);
+        programFatalError("Couldn't open %s for read\n", path);
         return nullptr;
     }
 
@@ -2307,9 +2311,7 @@ static void opStoreExternalVariable(Program* program)
     const char* identifier = programGetIdentifier(program, addr.integerValue);
 
     if (externalVariableSetValue(program, identifier, value)) {
-        char err[256];
-        snprintf(err, sizeof(err), "External variable %s does not exist\n", identifier);
-        programFatalError(err);
+        programFatalError("External variable %s does not exist\n", identifier);
     }
 }
 
@@ -2322,9 +2324,7 @@ static void opFetchExternalVariable(Program* program)
 
     ProgramValue value;
     if (externalVariableGetValue(program, identifier, value) != 0) {
-        char err[256];
-        snprintf(err, sizeof(err), "External variable %s does not exist\n", identifier);
-        programFatalError(err);
+        programFatalError("External variable %s does not exist\n", identifier);
     }
 
     programStackPushValue(program, value);
@@ -2342,9 +2342,7 @@ static void opExportProcedure(Program* program)
     const int procedureAddress = stackReadInt32(proc_ptr, offsetof(Procedure, bodyOffset));
 
     if (externalProcedureCreate(program, procedureName, procedureAddress, argumentCount) != 0) {
-        char err[256];
-        snprintf(err, sizeof(err), "Error exporting procedure %s", procedureName);
-        programFatalError(err);
+        programFatalError("Error exporting procedure %s", procedureName);
     }
 }
 
@@ -2356,9 +2354,7 @@ static void opExportVariable(Program* program)
     const char* identifier = programGetIdentifier(program, addr.integerValue);
 
     if (externalVariableCreate(program, identifier)) {
-        char err[256];
-        snprintf(err, sizeof(err), "External variable %s already exists", identifier);
-        programFatalError(err);
+        programFatalError("External variable %s already exists", identifier);
     }
 }
 
@@ -2411,9 +2407,7 @@ static void opCallStart(Program* program)
     // NOTE: Uninline.
     program->child = runScript(name);
     if (program->child == nullptr) {
-        char err[260];
-        snprintf(err, sizeof(err), "Error spawning child %s", name);
-        programFatalError(err);
+        programFatalError("Error spawning child %s", name);
     }
 
     program->child->parent = program;
@@ -2435,9 +2429,7 @@ static void opSpawn(Program* program)
     // NOTE: Uninline.
     program->child = runScript(name);
     if (program->child == nullptr) {
-        char err[260];
-        snprintf(err, sizeof(err), "Error spawning child %s", name);
-        programFatalError(err);
+        programFatalError("Error spawning child %s", name);
     }
 
     program->child->parent = program;
@@ -2457,9 +2449,7 @@ static Program* forkProgram(Program* program)
     Program* forked = runScript(name);
 
     if (forked == nullptr) {
-        char err[256];
-        snprintf(err, sizeof(err), "couldn't fork script '%s'", name);
-        programFatalError(err);
+        programFatalError("couldn't fork script '%s'", name);
     }
 
     forked->windowId = program->windowId;
@@ -2511,9 +2501,7 @@ static void opCheckProcedureArgumentCount(Program* program)
     const int actualArgumentCount = stackReadInt32(program->procedures + 4 + 24 * procedureIndex, offsetof(Procedure, argCount));
     if (actualArgumentCount != expectedArgumentCount) {
         const char* identifier = programGetIdentifier(program, stackReadInt32(program->procedures + 4 + 24 * procedureIndex, offsetof(Procedure, nameOffset)));
-        char err[260];
-        snprintf(err, sizeof(err), "Wrong number of args to procedure %s\n", identifier);
-        programFatalError(err);
+        programFatalError("Wrong number of args to procedure %s\n", identifier);
     }
 }
 
@@ -2541,9 +2529,7 @@ static void opLookupStringProc(Program* program)
         procedurePtr += sizeof(Procedure);
     }
 
-    char err[260];
-    snprintf(err, sizeof(err), "Couldn't find string procedure %s\n", procedureNameToLookup);
-    programFatalError(err);
+    programFatalError("Couldn't find string procedure %s\n", procedureNameToLookup);
 }
 
 // 0x46C7DC
@@ -2643,8 +2629,6 @@ void _interpretClose()
 // 0x46CCA4
 void programInterpret(Program* program, int numInstructions)
 {
-    char err[260];
-
     Program* const oldCurrentProgram = gInterpreterCurrentProgram;
 
     if (!interpreterEnabled) {
@@ -2708,15 +2692,13 @@ void programInterpret(Program* program, int numInstructions)
         program->flags |= (opcode << 16);
 
         if (!((opcode >> 8) & 0x80)) {
-            snprintf(err, sizeof(err), "Bad opcode %x %c %d.", opcode, opcode, opcode);
-            programFatalError(err);
+            programFatalError("Bad opcode %x %c %d.", opcode, opcode, opcode);
         }
 
         const unsigned int opcodeIndex = opcode & 0x3FF;
         OpcodeHandler* handler = gInterpreterOpcodeHandlers[opcodeIndex];
         if (handler == nullptr) {
-            snprintf(err, sizeof(err), "Undefined opcode %x.", opcode);
-            programFatalError(err);
+            programFatalError("Undefined opcode %x.", opcode);
         }
 
         handler(program);
@@ -2814,7 +2796,6 @@ void programExecuteProcedureAsync(Program* program, int procedureIndex)
     int externalProcedureAddress;
     int externalProcedureArgumentCount;
     int procedureFlags;
-    char err[256];
 
     procedurePtr = program->procedures + 4 + sizeof(Procedure) * procedureIndex;
     procedureFlags = stackReadInt32(procedurePtr, offsetof(Procedure, flags));
@@ -2824,12 +2805,12 @@ void programExecuteProcedureAsync(Program* program, int procedureIndex)
         if (externalProgram != nullptr) {
             if (externalProcedureArgumentCount == 0) {
             } else {
-                snprintf(err, sizeof(err), "External procedure cannot take arguments in interrupt context");
-                _interpretOutput(err);
+                _interpretOutput("External procedure cannot take arguments in interrupt context");
+                return;
             }
         } else {
-            snprintf(err, sizeof(err), "External procedure %s not found\n", procedureIdentifier);
-            _interpretOutput(err);
+            _interpretOutput("External procedure %s not found\n", procedureIdentifier);
+            return;
         }
 
         // NOTE: Uninline.
@@ -2888,7 +2869,6 @@ void programExecuteProcedure(Program* program, int procedureIndex)
     int externalProcedureAddress;
     int externalProcedureArgumentCount;
     int procedureFlags;
-    char err[256];
     jmp_buf env;
 
     procedurePtr = program->procedures + 4 + sizeof(Procedure) * procedureIndex;
@@ -2905,12 +2885,10 @@ void programExecuteProcedure(Program* program, int procedureIndex)
                 programInterpret(externalProgram, -1);
                 memcpy(externalProgram->env, env, sizeof(env));
             } else {
-                snprintf(err, sizeof(err), "External procedure cannot take arguments in interrupt context");
-                _interpretOutput(err);
+                _interpretOutput("External procedure cannot take arguments in interrupt context");
             }
         } else {
-            snprintf(err, sizeof(err), "External procedure %s not found\n", procedureIdentifier);
-            _interpretOutput(err);
+            _interpretOutput("External procedure %s not found\n", procedureIdentifier);
         }
     } else {
         procedureAddress = stackReadInt32(procedurePtr, offsetof(Procedure, bodyOffset));

src/interpreter_extra.cc

@@ -372,11 +372,7 @@ static int gGameDialogReactionOrFidget;
 // 0x453FD0 dbg_error
 static void scriptPredefinedError(Program* program, const char* name, int error)
 {
-    char string[260];
-
-    snprintf(string, sizeof(string), "Script Error: %s: op_%s: %s", program->name, name, _dbg_error_strs[error]);
-
-    debugPrint(string);
+    debugPrint("Script Error: %s: op_%s: %s", program->name, name, _dbg_error_strs[error]);
 }
 
 // 0x45400C int_debug
@@ -386,10 +382,12 @@ static void scriptError(const char* format, ...)
 
     va_list argptr;
     va_start(argptr, format);
-    vsnprintf(string, sizeof(string), format, argptr);
+    if (vsnprintf(string, sizeof(string), format, argptr) < 0) {
+        string[0] = '\0';
+    }
     va_end(argptr);
 
-    debugPrint(string);
+    debugPrint("%s", string);
 }
 
 // 0x45404C scripts_tile_is_visible
@@ -532,9 +530,7 @@ static void opOverrideMapStart(Program* program)
     int y = programStackPopInteger(program);
     int x = programStackPopInteger(program);
 
-    char text[60];
-    snprintf(text, sizeof(text), "OVERRIDE_MAP_START: x: %d, y: %d", x, y);
-    debugPrint(text);
+    debugPrint("OVERRIDE_MAP_START: x: %d, y: %d", x, y);
 
     int tile = 200 * y + x;
     int previousTile = gCenterTile;
@@ -1020,8 +1016,7 @@ static void opDisplayMsg(Program* program)
     displayMonitorAddMessage(string);
 
     if (settings.debug.show_script_messages) {
-        debugPrint("\n");
-        debugPrint(string);
+        debugPrint("\n%s", string);
     }
 }
 
@@ -4820,11 +4815,8 @@ static void opDebugMessage(Program* program)
 {
     char* string = programStackPopString(program);
 
-    if (string != nullptr) {
-        if (settings.debug.show_script_messages) {
-            debugPrint("\n");
-            debugPrint(string);
-        }
+    if (string != nullptr && settings.debug.show_script_messages) {
+        debugPrint("\n%s", string);
     }
 }
 

src/interpreter_lib.cc

@@ -313,7 +313,7 @@ void opTokenize(Program* program)
             }
 
             temp = (char*)internal_calloc_safe(1, length + 1, __FILE__, __LINE__); // "..\\int\\INTLIB.C, 230
-            strncpy(temp, start, length);
+            strncpy(temp, start + 1, length);
             programStackPushString(program, temp);
         } else {
             programStackPushInteger(program, 0);

src/item.cc

@@ -509,19 +509,19 @@ static int _item_move_func(Object* source, Object* target, Object* item, int qua
     return 0;
 }
 
-// 0x47769C
+// 0x47769C item_move
 int itemMove(Object* from, Object* to, Object* item, int quantity)
 {
     return _item_move_func(from, to, item, quantity, false);
 }
 
-// 0x4776A4
+// 0x4776A4 item_move_force
 int itemMoveForce(Object* from, Object* to, Object* item, int quantity)
 {
     return _item_move_func(from, to, item, quantity, true);
 }
 
-// 0x4776AC
+// 0x4776AC item_move_all
 void itemMoveAll(Object* from, Object* to)
 {
     Inventory* inventory = &(from->data.inventory);
@@ -532,7 +532,7 @@ void itemMoveAll(Object* from, Object* to)
     }
 }
 
-// 0x4776E0
+// 0x4776E0 item_move_all_hidden
 int itemMoveAllHidden(Object* from, Object* to)
 {
     Inventory* inventory = &(from->data.inventory);
@@ -549,7 +549,7 @@ int itemMoveAllHidden(Object* from, Object* to)
     return 0;
 }
 
-// 0x477770
+// 0x477770 item_destroy_all_hidden
 int itemDestroyAllHidden(Object* owner)
 {
     Inventory* inventory = &(owner->data.inventory);