From f4d0e3731fe26799f9bc8e7be48b2f59f465d409 Mon Sep 17 00:00:00 2001 From: "Derek T. Jones" Date: Sun, 15 Mar 2026 18:12:12 -0700 Subject: [PATCH 1/3] Do not create attributes as a side effect of reading them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AttributeValue::dereference_() was calling setAttribute() to stamp an UNDEFINED value onto an object whenever a nonexistent attribute was read. This caused phantom attributes to accumulate over time, inflating save files and polluting the object model. Instead, return UNDEFINED directly without mutation — hasAttribute() already walks the parent chain for inherited lookups. The setAttribute call was introduced in 4fdcb62 (2014) to make the getAttributeValue() path uniform, but returning early for the not-found case is simpler and avoids the side effect. Co-Authored-By: Claude Opus 4.6 --- src/Value.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Value.cc b/src/Value.cc index 64ea8b2..fe9b250 100644 --- a/src/Value.cc +++ b/src/Value.cc @@ -350,14 +350,10 @@ namespace archetype { Value AttributeValue::dereference_() const { ObjectPtr obj = Universe::instance().getObject(objectId_); - if (not obj) { + if (not obj or not obj->hasAttribute(attributeId_)) { return Value{new UndefinedValue}; } - if (not obj->hasAttribute(attributeId_)) { - obj->setAttribute(attributeId_, Value{new UndefinedValue}); - } - ContextScope c; c->selfObject = obj; return obj->getAttributeValue(attributeId_); From bea9fd4d466405ea7afbe1cc97dc6601b5334fa4 Mon Sep 17 00:00:00 2001 From: "Derek T. Jones" Date: Sat, 18 Apr 2026 13:41:47 -0700 Subject: [PATCH 2/3] Add regression test for ghost attribute creation --- src/TestObject.cc | 19 +++++++++++++++++++ src/TestObject.hh | 1 + 2 files changed, 20 insertions(+) diff --git a/src/TestObject.cc b/src/TestObject.cc index f98330f..7575d76 100644 --- a/src/TestObject.cc +++ b/src/TestObject.cc @@ -166,10 +166,29 @@ namespace archetype { ARCHETYPE_TEST_EQUAL(actual2, expected2); } + void TestObject::testReadDoesNotMutate_() { + ObjectPtr subject = Universe::instance().defineNewObject(); + Universe::instance().assignObjectIdentifier(subject, "subject"); + int ghost_id = Universe::instance().Identifiers.index("ghost"); + + ARCHETYPE_TEST(not subject->hasAttribute(ghost_id)); + + Expression expr = make_expr_from_str("subject.ghost"); + Value val = expr->evaluate()->valueConversion(); + ARCHETYPE_TEST(not val->isDefined()); + + ARCHETYPE_TEST(not subject->hasAttribute(ghost_id)); + + expr->evaluate(); + expr->evaluate(); + ARCHETYPE_TEST(not subject->hasAttribute(ghost_id)); + } + void TestObject::runTests_() { testObjects_(); testInheritance_(); testMethods_(); testMessagePassing_(); + testReadDoesNotMutate_(); } } diff --git a/src/TestObject.hh b/src/TestObject.hh index 288570c..2e373a0 100644 --- a/src/TestObject.hh +++ b/src/TestObject.hh @@ -19,6 +19,7 @@ namespace archetype { void testInheritance_(); void testMethods_(); void testMessagePassing_(); + void testReadDoesNotMutate_(); protected: virtual void runTests_() override; public: From c5385cdd7b19921ea2210e30f0bb64517d4f0020 Mon Sep 17 00:00:00 2001 From: "Derek T. Jones" Date: Sat, 18 Apr 2026 13:45:48 -0700 Subject: [PATCH 3/3] Add hasLocalAttribute method and pin inherited-read invariant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Distinguishes locally-set attributes from inherited ones — useful for SITREP-style introspection and as a regression guard. The extended test asserts that reading an inherited attribute returns the parent's value without creating a local entry on the child. Co-Authored-By: Claude Opus 4.7 --- src/Object.cc | 4 ++++ src/Object.hh | 1 + src/TestObject.cc | 18 ++++++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/src/Object.cc b/src/Object.cc index 7b1fe84..dc53781 100644 --- a/src/Object.cc +++ b/src/Object.cc @@ -33,6 +33,10 @@ namespace archetype { return attributes_.count(attribute_id) > 0 or (p and p->hasAttribute(attribute_id)); } + bool Object::hasLocalAttribute(int attribute_id) const { + return attributes_.count(attribute_id) > 0; + } + Value Object::getAttributeValue(int attribute_id) const { auto where = attributes_.find(attribute_id); if (where != attributes_.end()) { diff --git a/src/Object.hh b/src/Object.hh index 194754e..0be1ec9 100644 --- a/src/Object.hh +++ b/src/Object.hh @@ -62,6 +62,7 @@ namespace archetype { ObjectPtr parent() const; bool hasAttribute(int attribute_id) const; + bool hasLocalAttribute(int attribute_id) const; Value getAttributeValue(int attribute_id) const; void setAttribute(int attribute_id, Expression expr); void setAttribute(int attribute_id, Value val); diff --git a/src/TestObject.cc b/src/TestObject.cc index 7575d76..e731de1 100644 --- a/src/TestObject.cc +++ b/src/TestObject.cc @@ -182,6 +182,24 @@ namespace archetype { expr->evaluate(); expr->evaluate(); ARCHETYPE_TEST(not subject->hasAttribute(ghost_id)); + + ObjectPtr animal = Universe::instance().defineNewObject(); + animal->setPrototype(true); + Universe::instance().assignObjectIdentifier(animal, "animal"); + int legs_id = Universe::instance().Identifiers.index("legs"); + animal->setAttribute(legs_id, Value(new NumericValue(4))); + + ObjectPtr dog = Universe::instance().defineNewObject(animal->id()); + Universe::instance().assignObjectIdentifier(dog, "dog"); + + ARCHETYPE_TEST(dog->hasAttribute(legs_id)); + ARCHETYPE_TEST(not dog->hasLocalAttribute(legs_id)); + + Expression inherited = make_expr_from_str("dog.legs"); + Value inherited_val = inherited->evaluate()->numericConversion(); + ARCHETYPE_TEST_EQUAL(inherited_val->getNumber(), 4); + + ARCHETYPE_TEST(not dog->hasLocalAttribute(legs_id)); } void TestObject::runTests_() {