Skip to content

[Feature] Detach Registry #168

Description

@TaranDahl

Problem

Problem

Detach_This_From_All (code/tracker.cpp) sweeps type-tracked object lists and calls AbstractClass::Detach(target, all) on every entry. Two defects follow from this shape:

  • bool all cannot unambiguously say why the target is leaving. This causes real defects, such as Limbo making a unit lose its ownership to its Spawner.
  • The broadcast reaches every object of the tracked types, not just the objects that actually reference the target, wasting large numbers of useless callbacks.

Proposal

Event vocabulary

enum class DetachReason : uint8_t
{
    Destroyed,   // real death: UnInit, DTOR
    Dying,       // pre-death, still animated: crash, sinking, falling infantry
    Vanished,    // left perception: cloak, subterranean, fog (a known TODO source)
    Limbo,       // left the field without dying: entering a transport
    Ownership,   // changed owner: engineer capture
};

using DetachMask = uint8_t;   // one bit per reason

Only ObjectClass raises Dying, Vanished, Limbo, and Ownership. Destroyed also comes from heap teardown of non-objects.

Each reference member declares which reasons it reacts to through a mask. Destroyed is implicitly subscribed by every reference and cannot be opted out of: no reference may outlive its target. Mask declares only the additional non-death reasons.

Data model

References stay raw pointers inside DetachLink<T> members; the member is itself the subscription node. Dangling safety comes from the type system plus RAII rules:

  1. operator=(T*) is not provided, so a reference can only be written through Attach(), which maintains the target's subscriber chain.
  2. The node destructor unlinks the node from whatever chain it is on, so a dying owner cleans up without per-class teardown code.
  3. Notify(Destroyed) severs every node on the chain, ignoring masks.

Node base

class AbstractClass;

using DetachCallback = void (*)(DetachLinkBase & self,
                                AbstractClass * target,
                                DetachReason reason);

class DetachLinkBase
{
public:
    void Unlink();   // O(1), idempotent
    void Sever();    // Unlink + clear Target

protected:
    void LinkTo(AbstractClass * new_target);   // type-erased body of Attach

    DetachLinkBase* Prev = nullptr;
    DetachLinkBase* Next = nullptr;
    AbstractClass*  Target = nullptr;    // invariant: Target != nullptr iff linked
    DetachMask      Mask = 0;            // non-death reasons only
    DetachCallback  Callback = nullptr;  // null for a simple reference
};

Target doubles as the chain-owner back pointer, so Unlink repairs the chain head without extra storage:

void DetachLinkBase::Unlink()
{
    if (Target == nullptr) return;
    if (Prev) {
        Prev->Next = Next;
    } else {
        Target->FirstSubscriber = Next;
    }
    if (Next) Next->Prev = Prev;
    Prev = Next = nullptr;
}

void DetachLinkBase::Sever()
{
    Unlink();
    Target = nullptr;   // restores the invariant; later Unlink/Attach stay safe
}

AbstractClass gains one chain head:

class AbstractClass
{
    DetachLinkBase* FirstSubscriber = nullptr;
};

Typed link

template<class T>
class DetachLink : public DetachLinkBase
{
public:
    DetachLink(DetachMask mask, DetachCallback callback = nullptr);

    operator T*() const;
    T* operator->() const;

    void Attach(T * new_target);         // sole write path; no operator=(T*)
};

void DetachLinkBase::LinkTo(AbstractClass * new_target)   // called by DetachLink<T>::Attach
{
    if (new_target == Target) return;
    Sever();
    Target = new_target;
    if (Target == nullptr) return;
    Next = Target->FirstSubscriber;
    if (Next) Next->Prev = this;
    Target->FirstSubscriber = this;
}

Notification

void AbstractClass::Notify_Detach(DetachReason reason)
{
    DetachLinkBase* node = FirstSubscriber;
    while (node) {
        DetachLinkBase* next = node->Next;   // node may remove itself
        if (reason == DetachReason::Destroyed || (node->Mask & ReasonBit(reason))) {
            if (node->Callback) {
                node->Callback(*node, this, reason);
            }
            if (reason == DetachReason::Destroyed || node->Callback == nullptr) {
                node->Sever();
            }
        }
        node = next;
    }
    assert(reason != DetachReason::Destroyed || FirstSubscriber == nullptr);
}

Two properties carry the correctness of this loop:

  • next is captured before the handler runs, because a handler may Attach new references and thereby mutate the chain.
  • On Destroyed the mask is bypassed and every node is severed, so no node can keep Prev/Next pointers into a torn-down chain.

Non-Destroyed events leave the chain populated

A Dying or Vanished event severs only the nodes that care about that reason; the rest stay subscribed and are handled at Destroyed. This is intended, not a leak: the target is still alive, so a remaining subscription is not a dangling state. The chain empties only at Destroyed, and Notify_Detach guarantees that unconditionally.

Usage

Simple references. The matching branches in TechnoClass::Detach retire:

class TechnoClass : public FootClass
{
    DetachLink<AbstractClass> TarCom{ReasonBit(DetachReason::Ownership)};  // Destroyed implicit
    DetachLink<AbstractClass> ArchiveTarget{0};                            // Destroyed only
    DetachLink<ParticleSystemClass> ParticleSystems[ATTACHED_PARTICLE_COUNT] = {};
};

void TechnoClass::Assign_Target(ObjectClass * new_target)
{
    TarCom.Attach(new_target);   // was: TarCom = new_target
    // ...
}

The sensed-retention behavior for Dying/Vanished targets (today's Is_Sensed check) becomes a callback on the TarCom node.

Callback references. Today's HouseClass::Detach logic moves into a trampoline; the handler recovers this from &self via offsetof(HouseClass, ConYardLink):

class HouseClass : public AbstractClass
{
    DetachLink<AbstractClass> ConYardLink{0, [](DetachLinkBase & self,
                                              AbstractClass * target,
                                              DetachReason reason)
        { HouseClass::Ref_Detached(self, target, reason); }};
};

void HouseClass::Ref_Detached(DetachLinkBase & self, AbstractClass * target, DetachReason reason)
{
    if (reason == DetachReason::Destroyed) {
        ConYards.Delete(static_cast<BuildingClass *>(target));
    }
}

What retires

  • The type-tracker lists in code/tracker.h and the broadcast in code/tracker.cpp, replaced by target->Notify_Detach(reason).
  • The bool all parameter at every call site, replaced by a DetachReason argument.
  • The per-class Detach overrides and their manual null-and-decrement bodies.

Limits and failure modes

  • Bypassing the barrier is memory corruption, not just a missed notification. A raw write leaves a node on the target's chain whose owner may die later; Notify(Destroyed) would then sever a wild node. The barrier is therefore compile-enforced (no implicit assignment), and Debug builds keep the old sweep as an assertion that every reference it finds is registered.
  • Memory. A node is two chain pointers plus target, mask, and callback (24-40 bytes) versus 4-8 bytes for a raw pointer. A TechnoClass-scale class grows by roughly 150-250 bytes per object.
  • Notification order is registration order: deterministic, but not the old broadcast order. No behavior may depend on the order.
  • Container references (NavQueue, RouteQueue, FootClass::Member chains) do not fit the node model; they keep manual handling or use one callback node for the container.
  • offsetof recovery of this is conditionally supported for non-standard-layout classes; a trampoline that stores the known offset is the fallback if a compiler rejects it.

Migration

  1. Add DetachReason and map the bool all argument to a reason at every call site; keep the old Detach(target, all) semantics behind a shim. No behavior change.
  2. Convert one subsystem (TarCom/NavCom) to DetachLink while the old broadcast still runs; both paths must observe the same results.
  3. Migrate members one by one, deleting the corresponding branches from the class Detach overrides.
  4. Delete the tracker lists and the broadcast when no member uses them; keep the Debug sweep as the registration assertion.
  5. Fog-of-war transitions emit Vanished once that handling is implemented; the handler policies above already cover it.

Relationship to the GameHandle idea

A handle (stable ID plus generation counter) was the earlier candidate and is dropped. Everything it offered - auto-null on death, dangling safety - is provided here by Sever and the compile-enforced barrier, without the costs of a changed reference representation (per-access generation checks, save and CRC impact). Handles also cannot express the non-simple reactions, which require the event system regardless. The access-time self-validation of handles is replaced by the Debug registration assertion.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions