Refactor: Remove TowerRegistry, replicate towers by TypeID

Removing redundant workflow and making sure there's just once source of truth for the list of available towers.
This commit is contained in:
Matt F 2026-06-25 11:21:43 -07:00
parent 63b8195985
commit a87b221d87
5 changed files with 33 additions and 159 deletions

View file

@ -68,8 +68,8 @@ namespace TD.Gameplay
"the prefab based on the player's RaceSelection.")] "the prefab based on the player's RaceSelection.")]
public GameObject BuilderPrefab; public GameObject BuilderPrefab;
[Tooltip("STUB (Phase 1.8): tower roster available to this race. TowerRegistry will " + [Tooltip("STUB: tower roster available to this race/builder. The draft pool or " +
"filter to this list when the active player belongs to this race.")] "starting deck will eventually filter to this list for this race.")]
public TowerDefinition[] Towers; public TowerDefinition[] Towers;
} }
} }

View file

@ -1,5 +1,4 @@
// Assets/_Project/Scripts/Gameplay/TowerInstance.cs // Assets/_Project/Scripts/Gameplay/TowerInstance.cs
using Unity.Collections;
using Unity.Netcode; using Unity.Netcode;
using UnityEngine; using UnityEngine;
using TD.Core; using TD.Core;
@ -32,11 +31,11 @@ namespace TD.Gameplay
/// <see cref="SetOccupied"/> are idempotent writes, so double-stamping is safe.</para> /// <see cref="SetOccupied"/> are idempotent writes, so double-stamping is safe.</para>
/// ///
/// <para><b>Definition reference replication.</b> TowerDefinition assets live in the /// <para><b>Definition reference replication.</b> TowerDefinition assets live in the
/// project on all clients. We replicate the asset by name via a /// project on all clients. We replicate the tower's <c>TowerTypeId</c> (its index in
/// <see cref="NetworkVariable{T}"/> holding a <c>FixedString64Bytes</c>, then look /// <see cref="TowerPlacementManager"/>'s catalog) via a <see cref="NetworkVariable{T}"/>,
/// up the asset locally. This avoids serializing the full ScriptableObject over the /// then resolve the full asset locally with <see cref="TowerPlacementManager.GetDefinition"/>.
/// network. The lookup uses a <see cref="TowerRegistry"/> singleton that must be /// This avoids serializing the ScriptableObject over the network and reuses the same
/// present in the scene. (Temporary: will be driven by RaceDefinition in Path E.)</para> /// identifier placement and the deck already use — a single source of truth.</para>
/// ///
/// <para><b>Combat.</b> No combat logic here yet. Combat fields live stubbed on /// <para><b>Combat.</b> No combat logic here yet. Combat fields live stubbed on
/// <see cref="TowerDefinition"/>; they will be consumed by a future /// <see cref="TowerDefinition"/>; they will be consumed by a future
@ -58,12 +57,13 @@ namespace TD.Gameplay
// ----- Networked state ------------------------------------------------ // ----- Networked state ------------------------------------------------
// The name of the TowerDefinition asset for this tower. Replicated so all // The TowerTypeId (index into TowerPlacementManager's catalog) for this tower.
// clients can look up the full definition locally without sending the whole // Replicated so every client resolves the full definition locally via the catalog —
// ScriptableObject over the wire. // the same identifier placement and the deck already use. 0 = unset/invalid (the
private readonly NetworkVariable<FixedString64Bytes> definitionName = // reserved catalog index). Replaces the old replicate-by-name + TowerRegistry path.
new NetworkVariable<FixedString64Bytes>( private readonly NetworkVariable<int> definitionTypeId =
default, new NetworkVariable<int>(
0,
readPerm: NetworkVariableReadPermission.Everyone, readPerm: NetworkVariableReadPermission.Everyone,
writePerm: NetworkVariableWritePermission.Server); writePerm: NetworkVariableWritePermission.Server);
@ -95,8 +95,8 @@ namespace TD.Gameplay
// ----- Local resolved state ------------------------------------------- // ----- Local resolved state -------------------------------------------
// Resolved on every client in OnNetworkSpawn from definitionName. // Resolved on every client in OnNetworkSpawn from definitionTypeId via the catalog.
// Null if the lookup fails (missing TowerRegistry or unknown name). // Null if the lookup fails (TypeId not in the catalog on this peer).
private TowerDefinition resolvedDefinition; private TowerDefinition resolvedDefinition;
// ----- Pre-spawn initialization data ---------------------------------- // ----- Pre-spawn initialization data ----------------------------------
@ -192,7 +192,7 @@ namespace TD.Gameplay
hasPendingInit = true; hasPendingInit = true;
// Cache the resolved definition on the server immediately — clients // Cache the resolved definition on the server immediately — clients
// will resolve via the registry lookup once definitionName arrives. // resolve from the replicated TowerTypeId via the catalog once it arrives.
resolvedDefinition = def; resolvedDefinition = def;
} }
@ -206,7 +206,17 @@ namespace TD.Gameplay
// every client sees correct values on its very first OnNetworkSpawn. // every client sees correct values on its very first OnNetworkSpawn.
if (IsServer && hasPendingInit) if (IsServer && hasPendingInit)
{ {
definitionName.Value = new FixedString64Bytes(pendingDefinition.name); // Resolve the catalog index for this definition and replicate that.
int typeId = 0;
var pm = TowerPlacementManager.Instance;
if (pm == null || !pm.TryGetTypeId(pendingDefinition, out typeId))
{
Debug.LogError($"[TowerInstance] Could not resolve a TowerTypeId for " +
$"'{pendingDefinition?.name}'. Ensure it is in the " +
$"TowerPlacementManager catalog (towerDefinitions).");
}
definitionTypeId.Value = typeId;
anchorTile.Value = pendingAnchor; anchorTile.Value = pendingAnchor;
ownerSlot.Value = pendingOwner; ownerSlot.Value = pendingOwner;
@ -331,27 +341,13 @@ namespace TD.Gameplay
// Already resolved (server path via InitializeServer). // Already resolved (server path via InitializeServer).
if (resolvedDefinition != null) return; if (resolvedDefinition != null) return;
string defName = definitionName.Value.ToString(); int typeId = definitionTypeId.Value;
if (string.IsNullOrEmpty(defName)) resolvedDefinition = TowerPlacementManager.GetDefinition(typeId);
{
Debug.LogError($"[TowerInstance] NetworkObject {NetworkObjectId}: " +
$"definitionName is empty. Cannot resolve TowerDefinition.");
return;
}
var registry = TowerRegistry.Instance;
if (registry == null)
{
Debug.LogError($"[TowerInstance] NetworkObject {NetworkObjectId}: " +
$"No TowerRegistry found in scene. Cannot resolve '{defName}'.");
return;
}
resolvedDefinition = registry.Get(defName);
if (resolvedDefinition == null) if (resolvedDefinition == null)
{ {
Debug.LogError($"[TowerInstance] NetworkObject {NetworkObjectId}: " + Debug.LogError($"[TowerInstance] NetworkObject {NetworkObjectId}: " +
$"TowerRegistry does not contain a definition named '{defName}'."); $"no TowerDefinition for TypeId {typeId}. Is the " +
$"TowerPlacementManager catalog populated on this peer?");
} }
} }

View file

@ -1,120 +0,0 @@
// Assets/_Project/Scripts/Gameplay/TowerRegistry.cs
using System.Collections.Generic;
using UnityEngine;
using TD.Towers;
namespace TD.Gameplay
{
/// <summary>
/// Scene singleton that holds every <see cref="TowerDefinition"/> available in the
/// current match and lets any code look one up by asset name.
/// </summary>
/// <remarks>
/// <para><b>Why this exists.</b> <see cref="TowerInstance"/> replicates a tower's
/// definition by name (a <c>FixedString64Bytes</c> over the network), then resolves
/// the full ScriptableObject locally on every client. TowerRegistry is the lookup
/// table that makes that resolution possible without hard-coding asset paths.</para>
///
/// <para><b>Registration.</b> Drag every <see cref="TowerDefinition"/> asset into the
/// <c>Definitions</c> list on this component in the inspector. Assets can live anywhere
/// in the project — no special folder required.</para>
///
/// <para><b>Path E upgrade path.</b> In Path E the registry will filter to only the
/// definitions belonging to the active match's <c>RaceDefinition</c> rosters. For now
/// all assigned assets are registered.</para>
///
/// <para><b>Plain MonoBehaviour.</b> Not a NetworkBehaviour — the registry is
/// identical on every peer (same assets, same names), so there is nothing to sync.</para>
/// </remarks>
public class TowerRegistry : MonoBehaviour
{
// ----- Singleton --------------------------------------------------
/// <summary>
/// The active TowerRegistry. Null before Awake or after the scene unloads.
/// Always null-check before use.
/// </summary>
public static TowerRegistry Instance { get; private set; }
// ----- Inspector --------------------------------------------------
[Tooltip("All TowerDefinition assets available in this match. " +
"Drag assets here from Assets/_Project/Data/TowerDefinitions/ " +
"(or wherever they live). Asset name is used as the registry key.")]
[SerializeField] private TowerDefinition[] definitions;
// ----- Internal lookup table --------------------------------------
// Keyed by TowerDefinition.name (the asset name, not DisplayName).
private readonly Dictionary<string, TowerDefinition> byName
= new Dictionary<string, TowerDefinition>();
// ----- Lifecycle --------------------------------------------------
private void Awake()
{
if (Instance != null && Instance != this)
{
Debug.LogError("[TowerRegistry] Multiple instances detected. " +
"Only one TowerRegistry should exist per scene.");
return;
}
Instance = this;
BuildLookupTable();
}
private void OnDestroy()
{
if (Instance == this) Instance = null;
}
// ----- Public API -------------------------------------------------
/// <summary>
/// Returns the <see cref="TowerDefinition"/> whose asset name equals
/// <paramref name="assetName"/>, or null if no match is found.
/// </summary>
public TowerDefinition Get(string assetName)
{
byName.TryGetValue(assetName, out var def);
return def;
}
/// <summary>
/// Returns all registered tower definitions. Enumerates the internal
/// dictionary values — do not modify the returned collection.
/// </summary>
public IEnumerable<TowerDefinition> All => byName.Values;
// ----- Private ----------------------------------------------------
private void BuildLookupTable()
{
byName.Clear();
if (definitions == null || definitions.Length == 0)
{
Debug.LogWarning("[TowerRegistry] No TowerDefinition assets assigned. " +
"Drag assets into the Definitions list on the TowerRegistry component.");
return;
}
foreach (var def in definitions)
{
if (def == null) continue;
if (byName.ContainsKey(def.name))
{
Debug.LogWarning($"[TowerRegistry] Duplicate asset name '{def.name}'. " +
$"Only the first entry will be used. Rename one of the assets.");
continue;
}
byName[def.name] = def;
}
Debug.Log($"[TowerRegistry] Registered {byName.Count} tower definition(s).");
}
}
}

View file

@ -1,2 +0,0 @@
fileFormatVersion: 2
guid: a9dc0fbbe4422bc479ab8db7658c082b

View file

@ -90,7 +90,7 @@ Unity **6.4 (6000.4.4f1)**, URP, IL2CPP, .NET Standard 2.1, Linear color space,
- **Visuals are placeholder** throughout (primitive/cone towers, sourced enemy models). See roadmap art TODOs. - **Visuals are placeholder** throughout (primitive/cone towers, sourced enemy models). See roadmap art TODOs.
- **Enemies lack idle animation support** — needs adding across all enemy prefabs. - **Enemies lack idle animation support** — needs adding across all enemy prefabs.
- **Tower footprint visuals don't fill their 2×2 space** — the cone/mesh sits centered and reads smaller than the tile area it occupies; should visually reflect the full footprint. - **Tower footprint visuals don't fill their 2×2 space** — the cone/mesh sits centered and reads smaller than the tile area it occupies; should visually reflect the full footprint.
- **Two redundant tower lists:** `TowerPlacementManager.towerDefinitions[]` (by-index catalog) and `TowerRegistry.definitions[]` (by-name, for instance resolution). Candidate for consolidation. - **Tower catalog is now a single source of truth:** `TowerPlacementManager.towerDefinitions[]` (by-index, = `TowerTypeId`). `TowerInstance` replicates that index and resolves via `TowerPlacementManager.GetDefinition`. *(The old by-name `TowerRegistry` was removed 2026-06-24 — add new towers only to `towerDefinitions` + `DraftPool`.)*
- **Catalog index 0 is a reserved sentinel** (valid `TowerTypeId`s start at 1) — easy to forget when wiring the catalog in the inspector. - **Catalog index 0 is a reserved sentinel** (valid `TowerTypeId`s start at 1) — easy to forget when wiring the catalog in the inspector.
- **Catalog-index identifiers aren't session-stable** — blocks cross-match persistence until a stable ID is added. - **Catalog-index identifiers aren't session-stable** — blocks cross-match persistence until a stable ID is added.
- **Paint system frozen**; **Race vs Builder** naming unresolved; the gold **"Buy Roll available any time"** rule is provisional and may change. - **Paint system frozen**; **Race vs Builder** naming unresolved; the gold **"Buy Roll available any time"** rule is provisional and may change.