Code review P2: small verified bug fixes (zero seed impact)

args/challenges.py

@@ -115,14 +115,6 @@ def options(args):
         ("Remove Learnable Spells", args.remove_learnable_spell_ids, "remove_learnable_spell_ids"),
         ("No Saves", args.no_saves, "no_saves"),
     ]
-
-    return opts
-def _format_spells_log_entries(spell_ids):
-    from constants.spells import id_spell
-    spell_entries = []
-    for i, spell_id in enumerate(spell_ids):
-        spell_entries.append(("", id_spell[spell_id], f"rls_{i}"))
-    return spell_entries
 
 def _format_spells_log_entries(spell_ids):
     from constants.spells import id_spell

battle/scaling.py

@@ -185,7 +185,7 @@ def scale_xp_gp_mod(self):
             asm.STA(0xea, asm.DIR),
             asm.JMP(self.scale_value, asm.ABS),
         ]
-        space = Write(Bank.C2, src, "scale hp/mp")
+        space = Write(Bank.C2, src, "scale xp/gp")
         self.scale_xp_gp = space.start_address
 
         if args.xp_gp_scaling:

data/character.py

@@ -85,7 +85,7 @@ def init_run_success(self, value):
         if value < self.MIN_RUN_SUCCESS or value > self.MAX_RUN_SUCCESS:
             raise ValueError(f"Character.init_run_success setter: invalid value {value}")
 
-        self._init_run_success = value - self.MAX_RUN_SUCCESS
+        self._init_run_success = self.MAX_RUN_SUCCESS - value
 
     # initial level of characters is 3
     # when new character is recruited, their level is set to the average of all other recruited characters + init_level_factor

event/whelk.py

@@ -16,9 +16,6 @@ def init_event_bits(self, space):
         )
 
     def mod(self):
-        if self.reward.type == RewardType.NONE:
-            return
-
         self.dialog_mod()
         self.entrance_event_mod()
         self.cleanup_mod()

metadata/flag_metadata_writer.py

@@ -57,8 +57,11 @@ def get_flag_metadata(self):
                     self.metadata[key].args = action.metavar
                 if action.choices is not None and isinstance(action.choices, list) and not isinstance(action.choices, range):
                     self.metadata[key].allowed_values = list(action.choices)
-                if type(group_title):
-                    self.metadata[key].group = group_title if type(group_title) == str else None if group_title == None else group_title()
+                # group titles may be a plain string, None, or a callable returning the name
+                if isinstance(group_title, str) or group_title is None:
+                    self.metadata[key].group = group_title
+                else:
+                    self.metadata[key].group = group_title()
                 if getattr(action, 'mutually_exclusive_group_title', None) is not None:
                     self.metadata[key].mutually_exclusive_group = action.mutually_exclusive_group_title
                 if getattr(action, 'choices', None) is not None:

objectives/objective.py

@@ -128,5 +128,5 @@ def _init_suplex_train_quest_value(cls):
             if value != 'r' and quest_bit[value].name == cls.suplex_train_quest_name:
                 cls.suplex_train_quest_value = value
                 return
-        assert False, f"'{suplex_train_quest_name}' quest value not found"
+        raise RuntimeError(f"'{cls.suplex_train_quest_name}' quest value not found")
 Objective._init_suplex_train_quest_value()

tests/test_character.py

@@ -0,0 +1,43 @@
+import unittest
+
+from data.character import Character
+
+def make_character():
+    init_data = [0] * 22
+    name_data = [0xff] * 6 # padding bytes only, i.e. an empty name
+    return Character(0, init_data, name_data)
+
+class TestInitRunSuccess(unittest.TestCase):
+    # run success is stored inverted in 2 bits of init_data[21]:
+    # 0b11 = 2, 0b10 = 3, 0b01 = 4, 0b00 = 5 (run_value = 5 - bit_value)
+    def test_getter_decodes_stored_bits(self):
+        character = make_character()
+        for raw, expected in ((0b11, 2), (0b10, 3), (0b01, 4), (0b00, 5)):
+            character._init_run_success = raw
+            self.assertEqual(character.init_run_success, expected)
+
+    def test_setter_round_trip(self):
+        # regression test: the setter used to store (value - MAX) instead of
+        # (MAX - value), corrupting the bit-packed init data byte
+        character = make_character()
+        for value in range(Character.MIN_RUN_SUCCESS, Character.MAX_RUN_SUCCESS + 1):
+            character.init_run_success = value
+            self.assertEqual(character.init_run_success, value)
+            self.assertIn(character._init_run_success, (0b00, 0b01, 0b10, 0b11))
+
+    def test_setter_rejects_out_of_range(self):
+        character = make_character()
+        with self.assertRaises(ValueError):
+            character.init_run_success = Character.MIN_RUN_SUCCESS - 1
+        with self.assertRaises(ValueError):
+            character.init_run_success = Character.MAX_RUN_SUCCESS + 1
+
+class TestInitLevelFactor(unittest.TestCase):
+    def test_round_trip(self):
+        character = make_character()
+        for adjustment in (0, 2, 5, -3):
+            character.init_level_factor = adjustment
+            self.assertEqual(character.init_level_factor, adjustment)
+
+if __name__ == "__main__":
+    unittest.main()

tests/test_utils.py

@@ -50,6 +50,14 @@ def test_size_header(self):
         size = int.from_bytes(bytes(compressed[:2]), "little")
         self.assertEqual(size, len(compressed))
 
+    def test_oversized_output_raises(self):
+        # incompressible data grows ~9/8x when compressed; this used to
+        # print an error and return a truncated size header instead of raising
+        rng = random.Random(42)
+        data = [rng.randrange(256) for _ in range(60000)]
+        with self.assertRaises(ValueError):
+            compress(data)
+
 class TestIntersection(unittest.TestCase):
     def test_intersection_preserves_first_list_order(self):
         self.assertEqual(intersection([3, 1, 2, 5], [2, 3]), [3, 2])

utils/compression.py

@@ -73,8 +73,8 @@ def compress(data):
 
     size = len(result) + 2
     if size > MAX_COMPRESS_SIZE:
-        print(f"Error: compress: data too large (compressed size {size} > 65535)")
-        size = MAX_COMPRESS_SIZE
+        # a wrong size header would silently corrupt the decompressed data
+        raise ValueError(f"compress: data too large (compressed size {size} > {MAX_COMPRESS_SIZE})")
     return list(size.to_bytes(2, "little")) + result
 
 def decompress(data):