Port 10 object tree opcodes to hybrid z-machine interpreter

Implemented 10 opcodes for object tree manipulation: get_sibling,
test_attr, set_attr, clear_attr, jin, remove_obj, print_obj,
get_prop_addr, get_next_prop, and get_prop_len.

Added 6 supporting methods to ZObjectParser: set_attribute,
clear_attribute, remove_object, get_property_data_address,
get_next_property, and get_property_length.

Fixed bug in insert_object where sibling chain walk never advanced
the prev pointer, potentially causing infinite loops.

Added 16 unit tests with MockObjectParser to verify CPU opcode
behavior. All tests passing.

src/mudlib/zmachine/zcpu.py

@@ -243,9 +243,10 @@ class ZCpu:
         self._write_result(val, store_addr=variable)
         self._branch(val > test_value)
 
-    def op_jin(self, *args):
+    def op_jin(self, obj1, obj2):
-        """TODO: Write docstring here."""
+        """Branch if obj1's parent equals obj2."""
-        raise ZCpuNotImplemented
+        parent = self._objects.get_parent(obj1)
+        self._branch(parent == obj2)
 
     def op_test(self, *args):
         """TODO: Write docstring here."""
@@ -259,17 +260,18 @@ class ZCpu:
         """Bitwise AND between the two arguments."""
         self._write_result(a & b)
 
-    def op_test_attr(self, *args):
+    def op_test_attr(self, obj, attr):
-        """TODO: Write docstring here."""
+        """Test if object has attribute, branch if true."""
-        raise ZCpuNotImplemented
+        has_attr = self._objects.get_attribute(obj, attr)
+        self._branch(has_attr != 0)
 
-    def op_set_attr(self, *args):
+    def op_set_attr(self, obj, attr):
-        """TODO: Write docstring here."""
+        """Set attribute on object."""
-        raise ZCpuNotImplemented
+        self._objects.set_attribute(obj, attr)
 
-    def op_clear_attr(self, *args):
+    def op_clear_attr(self, obj, attr):
-        """TODO: Write docstring here."""
+        """Clear attribute on object."""
-        raise ZCpuNotImplemented
+        self._objects.clear_attribute(obj, attr)
 
     def op_store(self, variable, value):
         """Store the given value to the given variable."""
@@ -299,13 +301,15 @@ class ZCpu:
         val = self._objects.get_prop(objectnum, propnum)
         self._write_result(val)
 
-    def op_get_prop_addr(self, *args):
+    def op_get_prop_addr(self, obj, prop):
-        """TODO: Write docstring here."""
+        """Get property data address, store 0 if not found."""
-        raise ZCpuNotImplemented
+        addr = self._objects.get_property_data_address(obj, prop)
+        self._write_result(addr)
 
-    def op_get_next_prop(self, *args):
+    def op_get_next_prop(self, obj, prop):
-        """TODO: Write docstring here."""
+        """Get next property number, store result."""
-        raise ZCpuNotImplemented
+        next_prop = self._objects.get_next_property(obj, prop)
+        self._write_result(next_prop)
 
     def op_add(self, a, b):
         """Signed 16-bit addition."""
@@ -367,9 +371,11 @@ class ZCpu:
         """Branch if the val is zero."""
         self._branch(val == 0)
 
-    def op_get_sibling(self, *args):
+    def op_get_sibling(self, obj):
-        """TODO: Write docstring here."""
+        """Get sibling of object, store it, branch if nonzero."""
-        raise ZCpuNotImplemented
+        sibling = self._objects.get_sibling(obj)
+        self._write_result(sibling)
+        self._branch(sibling != 0)
 
     def op_get_child(self, object_num):
         """Get and store the first child of the given object."""
@@ -379,9 +385,10 @@ class ZCpu:
         """Get and store the parent of the given object."""
         self._write_result(self._objects.get_parent(object_num))
 
-    def op_get_prop_len(self, *args):
+    def op_get_prop_len(self, data_addr):
-        """TODO: Write docstring here."""
+        """Get property data length from data address, store result."""
-        raise ZCpuNotImplemented
+        length = self._objects.get_property_length(data_addr)
+        self._write_result(length)
 
     def op_inc(self, variable):
         """Increment the given value."""
@@ -405,13 +412,14 @@ class ZCpu:
         """Call the given routine and store the return value."""
         self._call(routine_address, [], True)
 
-    def op_remove_obj(self, *args):
+    def op_remove_obj(self, obj):
-        """TODO: Write docstring here."""
+        """Remove object from its parent."""
-        raise ZCpuNotImplemented
+        self._objects.remove_object(obj)
 
-    def op_print_obj(self, *args):
+    def op_print_obj(self, obj):
-        """TODO: Write docstring here."""
+        """Print object's short name."""
-        raise ZCpuNotImplemented
+        shortname = self._objects.get_shortname(obj)
+        self._ui.screen.write(shortname)
 
     def op_ret(self, *args):
         """TODO: Write docstring here."""

src/mudlib/zmachine/zobjectparser.py

@@ -181,6 +181,52 @@ class ZObjectParser:
 
         return bf[7 - (attrnum % 8)]
 
+    def set_attribute(self, objectnum, attrnum):
+        """Set attribute number ATTRNUM of object number OBJECTNUM to 1."""
+
+        object_addr = self._get_object_addr(objectnum)
+
+        if 1 <= self._memory.version <= 3:
+            if not (0 <= attrnum <= 31):
+                raise ZObjectIllegalAttributeNumber
+            byte_offset = attrnum // 8
+            bf = BitField(self._memory[object_addr + byte_offset])
+
+        elif 4 <= self._memory.version <= 5:
+            if not (0 <= attrnum <= 47):
+                raise ZObjectIllegalAttributeNumber
+            byte_offset = attrnum // 8
+            bf = BitField(self._memory[object_addr + byte_offset])
+
+        else:
+            raise ZObjectIllegalVersion
+
+        bf[7 - (attrnum % 8)] = 1
+        self._memory[object_addr + byte_offset] = int(bf)
+
+    def clear_attribute(self, objectnum, attrnum):
+        """Clear attribute number ATTRNUM of object number OBJECTNUM to 0."""
+
+        object_addr = self._get_object_addr(objectnum)
+
+        if 1 <= self._memory.version <= 3:
+            if not (0 <= attrnum <= 31):
+                raise ZObjectIllegalAttributeNumber
+            byte_offset = attrnum // 8
+            bf = BitField(self._memory[object_addr + byte_offset])
+
+        elif 4 <= self._memory.version <= 5:
+            if not (0 <= attrnum <= 47):
+                raise ZObjectIllegalAttributeNumber
+            byte_offset = attrnum // 8
+            bf = BitField(self._memory[object_addr + byte_offset])
+
+        else:
+            raise ZObjectIllegalVersion
+
+        bf[7 - (attrnum % 8)] = 0
+        self._memory[object_addr + byte_offset] = int(bf)
+
     def get_all_attributes(self, objectnum):
         """Return a list of all attribute numbers that are set on object
         OBJECTNUM"""
@@ -250,6 +296,39 @@ class ZObjectParser:
         else:
             raise ZObjectIllegalVersion
 
+    def remove_object(self, objectnum):
+        """Detach object OBJECTNUM from its parent (unlink from sibling chain)."""
+
+        parent = self.get_parent(objectnum)
+        if parent == 0:
+            # Object has no parent, nothing to remove
+            return
+
+        sibling = self.get_sibling(objectnum)
+
+        # Check if this object is the first child
+        if self.get_child(parent) == objectnum:
+            # Make sibling the new first child
+            self.set_child(parent, sibling)
+        else:
+            # Walk the sibling chain to find the object before this one
+            prev = self.get_child(parent)
+            current = self.get_sibling(prev)
+            while current != 0:
+                if current == objectnum:
+                    # Link prev to our sibling, removing us from chain
+                    self.set_sibling(prev, sibling)
+                    break
+                prev = current
+                current = self.get_sibling(current)
+            else:
+                # Shouldn't happen - object claimed parent but not in chain
+                raise ZObjectMalformedTree
+
+        # Clear this object's parent
+        self.set_parent(objectnum, 0)
+        self.set_sibling(objectnum, 0)
+
     def insert_object(self, parent_object, new_child):
         """Prepend object NEW_CHILD to the list of PARENT_OBJECT's children."""
 
@@ -279,6 +358,8 @@ class ZObjectParser:
                 if current == new_child:
                     self.set_sibling(prev, s)  # s might be 0, that's fine.
                     break
+                prev = current
+                current = self.get_sibling(current)
             else:
                 # we reached the end of the list, never got a match
                 raise ZObjectMalformedTree
@@ -402,6 +483,94 @@ class ZObjectParser:
         else:
             raise ZObjectIllegalPropertySet
 
+    def get_property_data_address(self, objectnum, propnum):
+        """Return the address of property PROPNUM's data bytes for object
+        OBJECTNUM. Return 0 if the object doesn't have that property."""
+
+        try:
+            addr, size = self.get_prop_addr_len(objectnum, propnum)
+            # get_prop_addr_len returns default property addr if not found
+            # We need to check if this is the actual property or default
+            proplist = self.get_all_properties(objectnum)
+            if propnum in proplist:
+                return addr
+            else:
+                return 0
+        except ZObjectIllegalPropLength:
+            return 0
+
+    def get_next_property(self, objectnum, propnum):
+        """If PROPNUM is 0, return the first property number of object OBJECTNUM.
+        Otherwise, return the property number after PROPNUM in the property list.
+        Return 0 if there are no more properties."""
+
+        if propnum == 0:
+            # Return first property number
+            addr = self._get_proptable_addr(objectnum)
+            # Skip past the shortname
+            addr += 2 * self._memory[addr]
+            # Read first property number
+            if self._memory[addr] == 0:
+                return 0
+
+            if 1 <= self._memory.version <= 3:
+                bf = BitField(self._memory[addr])
+                return bf[4:0]
+            elif 4 <= self._memory.version <= 5:
+                bf = BitField(self._memory[addr])
+                return bf[5:0]
+            else:
+                raise ZObjectIllegalVersion
+
+        else:
+            # Find the property after propnum
+            proplist = self.get_all_properties(objectnum)
+            if propnum not in proplist:
+                raise ZObjectIllegalPropertyNumber
+
+            # Properties are stored in descending order
+            # Find the next lower property number
+            sorted_props = sorted(proplist.keys(), reverse=True)
+            try:
+                idx = sorted_props.index(propnum)
+                if idx + 1 < len(sorted_props):
+                    return sorted_props[idx + 1]
+                else:
+                    return 0
+            except ValueError:
+                return 0
+
+    def get_property_length(self, data_address):
+        """Given a property DATA address, return the length of that property's data.
+        Return 0 if data_address is 0."""
+
+        if data_address == 0:
+            return 0
+
+        # The size byte is just before the data address
+        size_addr = data_address - 1
+
+        if 1 <= self._memory.version <= 3:
+            bf = BitField(self._memory[size_addr])
+            size = bf[7:5] + 1
+            return size
+
+        elif 4 <= self._memory.version <= 5:
+            bf = BitField(self._memory[size_addr])
+            if bf[7]:
+                # Two size bytes, look at second byte
+                bf2 = BitField(self._memory[data_address - 2])
+                size = bf2[5:0]
+                if size == 0:
+                    size = 64
+                return size
+            else:
+                # One size byte
+                return 2 if bf[6] else 1
+
+        else:
+            raise ZObjectIllegalVersion
+
     def describe_object(self, objectnum):
         """For debugging purposes, pretty-print everything known about
         object OBJECTNUM."""

tests/test_zmachine_opcodes.py

@@ -1,5 +1,5 @@
 """
-Unit tests for the 12 newly implemented Z-machine opcodes.
+Unit tests for the Z-machine opcodes and object parser.
 
 These tests verify the basic behavior of each opcode by mocking the
 required dependencies (memory, stack, decoder, etc).
@@ -249,3 +249,235 @@ class ZMachineOpcodeTests(TestCase):
         self.assertEqual(self.stack.get_local_variable(0), 123)
         # Stack should be empty
         self.assertEqual(len(self.stack.stack), 0)
+
+
+class MockObjectParser:
+    """Mock object parser for testing CPU opcodes."""
+
+    def __init__(self):
+        self.attributes = {}
+        self.parents = {}
+        self.siblings = {}
+        self.children = {}
+        self.shortnames = {}
+        self.property_data_addresses = {}
+        self.next_properties = {}
+
+    def get_attribute(self, objnum, attrnum):
+        return self.attributes.get((objnum, attrnum), 0)
+
+    def set_attribute(self, objnum, attrnum):
+        self.attributes[(objnum, attrnum)] = 1
+
+    def clear_attribute(self, objnum, attrnum):
+        self.attributes[(objnum, attrnum)] = 0
+
+    def get_parent(self, objnum):
+        return self.parents.get(objnum, 0)
+
+    def get_sibling(self, objnum):
+        return self.siblings.get(objnum, 0)
+
+    def remove_object(self, objnum):
+        # Simple implementation - just clear parent
+        self.parents[objnum] = 0
+
+    def get_shortname(self, objnum):
+        return self.shortnames.get(objnum, "object")
+
+    def get_property_data_address(self, objnum, propnum):
+        return self.property_data_addresses.get((objnum, propnum), 0)
+
+    def get_next_property(self, objnum, propnum):
+        return self.next_properties.get((objnum, propnum), 0)
+
+    def get_property_length(self, data_address):
+        # Simple mock - return 2 for non-zero addresses
+        return 2 if data_address != 0 else 0
+
+
+class ZMachineObjectOpcodeTests(TestCase):
+    """Test suite for Z-machine object tree opcodes."""
+
+    def setUp(self):
+        """Create a CPU with mocked object parser."""
+        self.memory = MockMemory()
+        self.stack = MockStackManager()
+        self.decoder = MockOpDecoder()
+        self.ui = MockUI()
+        self.objects = MockObjectParser()
+        self.string = Mock()
+        self.string.get = Mock(return_value="test object")
+
+        self.cpu = ZCpu(
+            self.memory,
+            self.decoder,
+            self.stack,
+            self.objects,
+            self.string,
+            Mock(),  # stream manager
+            self.ui,
+        )
+
+    def test_op_get_sibling_with_sibling(self):
+        """Test get_sibling stores sibling and branches if nonzero."""
+        self.objects.siblings[5] = 7
+        self.decoder.store_address = 0
+        self.decoder.branch_condition = True
+        self.decoder.branch_offset = 10
+        old_pc = self.cpu._opdecoder.program_counter
+
+        self.cpu.op_get_sibling(5)
+
+        # Should store 7
+        self.assertEqual(self.stack.pop_stack(), 7)
+        # Should branch (offset - 2)
+        self.assertEqual(self.cpu._opdecoder.program_counter, old_pc + 8)
+
+    def test_op_get_sibling_no_sibling(self):
+        """Test get_sibling with no sibling doesn't branch."""
+        self.objects.siblings[5] = 0
+        self.decoder.store_address = 0
+        self.decoder.branch_condition = True
+        old_pc = self.cpu._opdecoder.program_counter
+
+        self.cpu.op_get_sibling(5)
+
+        # Should store 0
+        self.assertEqual(self.stack.pop_stack(), 0)
+        # Should not branch
+        self.assertEqual(self.cpu._opdecoder.program_counter, old_pc)
+
+    def test_op_test_attr_true(self):
+        """Test test_attr branches when attribute is set."""
+        self.objects.attributes[(10, 5)] = 1
+        self.decoder.branch_condition = True
+        self.decoder.branch_offset = 20
+        old_pc = self.cpu._opdecoder.program_counter
+
+        self.cpu.op_test_attr(10, 5)
+
+        # Should branch
+        self.assertEqual(self.cpu._opdecoder.program_counter, old_pc + 18)
+
+    def test_op_test_attr_false(self):
+        """Test test_attr doesn't branch when attribute is clear."""
+        self.objects.attributes[(10, 5)] = 0
+        self.decoder.branch_condition = True
+        old_pc = self.cpu._opdecoder.program_counter
+
+        self.cpu.op_test_attr(10, 5)
+
+        # Should not branch
+        self.assertEqual(self.cpu._opdecoder.program_counter, old_pc)
+
+    def test_op_set_attr(self):
+        """Test set_attr sets attribute on object."""
+        self.cpu.op_set_attr(15, 3)
+        # Should have called set_attribute
+        self.assertEqual(self.objects.attributes.get((15, 3)), 1)
+
+    def test_op_clear_attr(self):
+        """Test clear_attr clears attribute on object."""
+        self.objects.attributes[(15, 3)] = 1
+        self.cpu.op_clear_attr(15, 3)
+        # Should have called clear_attribute
+        self.assertEqual(self.objects.attributes.get((15, 3)), 0)
+
+    def test_op_jin_true(self):
+        """Test jin branches when obj1 parent equals obj2."""
+        self.objects.parents[5] = 10
+        self.decoder.branch_condition = True
+        self.decoder.branch_offset = 15
+        old_pc = self.cpu._opdecoder.program_counter
+
+        self.cpu.op_jin(5, 10)
+
+        # Should branch
+        self.assertEqual(self.cpu._opdecoder.program_counter, old_pc + 13)
+
+    def test_op_jin_false(self):
+        """Test jin doesn't branch when obj1 parent not equal to obj2."""
+        self.objects.parents[5] = 8
+        self.decoder.branch_condition = True
+        old_pc = self.cpu._opdecoder.program_counter
+
+        self.cpu.op_jin(5, 10)
+
+        # Should not branch
+        self.assertEqual(self.cpu._opdecoder.program_counter, old_pc)
+
+    def test_op_remove_obj(self):
+        """Test remove_obj removes object from parent."""
+        self.objects.parents[7] = 3
+        self.cpu.op_remove_obj(7)
+        # Parent should be cleared
+        self.assertEqual(self.objects.parents.get(7), 0)
+
+    def test_op_print_obj(self):
+        """Test print_obj prints object's short name."""
+        self.objects.shortnames[12] = "brass lantern"
+        self.cpu.op_print_obj(12)
+        self.ui.screen.write.assert_called_once_with("brass lantern")
+
+    def test_op_get_prop_addr_found(self):
+        """Test get_prop_addr stores data address when property exists."""
+        self.objects.property_data_addresses[(20, 5)] = 0x5000
+        self.decoder.store_address = 0
+
+        self.cpu.op_get_prop_addr(20, 5)
+
+        # Should store the data address
+        self.assertEqual(self.stack.pop_stack(), 0x5000)
+
+    def test_op_get_prop_addr_not_found(self):
+        """Test get_prop_addr stores 0 when property doesn't exist."""
+        self.decoder.store_address = 0
+
+        self.cpu.op_get_prop_addr(20, 99)
+
+        # Should store 0
+        self.assertEqual(self.stack.pop_stack(), 0)
+
+    def test_op_get_next_prop_first(self):
+        """Test get_next_prop with propnum=0 returns first property."""
+        self.objects.next_properties[(25, 0)] = 15
+        self.decoder.store_address = 0
+
+        self.cpu.op_get_next_prop(25, 0)
+
+        # Should store first property number
+        self.assertEqual(self.stack.pop_stack(), 15)
+
+    def test_op_get_next_prop_next(self):
+        """Test get_next_prop with propnum>0 returns next property."""
+        self.objects.next_properties[(25, 10)] = 8
+        self.decoder.store_address = 0
+
+        self.cpu.op_get_next_prop(25, 10)
+
+        # Should store next property number
+        self.assertEqual(self.stack.pop_stack(), 8)
+
+    def test_op_get_prop_len(self):
+        """Test get_prop_len returns property data length."""
+        self.decoder.store_address = 0
+
+        self.cpu.op_get_prop_len(0x6000)
+
+        # Should store 2 (from mock)
+        self.assertEqual(self.stack.pop_stack(), 2)
+
+    def test_op_get_prop_len_zero_addr(self):
+        """Test get_prop_len with address 0 returns 0."""
+        self.decoder.store_address = 0
+
+        self.cpu.op_get_prop_len(0)
+
+        # Should store 0
+        self.assertEqual(self.stack.pop_stack(), 0)
+
+
+# Note: ZObjectParser methods are tested through integration tests
+# with real story files, not unit tests with mock memory, as the
+# interaction with ZStringFactory makes mocking complex.