Conversation
|
I think its good for review. Right now its still early to tell how it'll hold up with a full lobby, but it works for debugging cases. I tried to optimize it as much as I can. This is my first pr here so i hope i didn't write code that will burn your eyes I tested this multiple ways:
|
iamteapot422
left a comment
There was a problem hiding this comment.
Some descriptions like /// <param name="playerThe player's ckey</param> and /// <param name="player"></param> The player we want to handle spawning for have wrong formats. Remove empty descriptions.
Rename Reserved to IsAvailable (I like Reserved, but the second name is more clear about the purpose). Make it a public getter and private setter. Rename Reserve() to Use()
Call Use() only after spawning the player in EntitySubSystem. Before this moment, SpawnPoint's or Manger's internal state shouldn't be changed.
Rename Job to RoundStart
| public void HandleRoundStateChanged(ref EventContext context, in RoundStateUpdated state) | ||
| { | ||
| // Clear all spawn points for new round | ||
| if (state.RoundState == RoundState.Preparing) |
There was a problem hiding this comment.
Invert if to reduce nesting
|
|
||
| // Register the spawn point based on role | ||
| RoleData role = spawnPoint.SpawnPointData.RoleData; | ||
| if (role) |
There was a problem hiding this comment.
invert if to reduce nesting
| _spawnPoints[role] = points; | ||
| } | ||
|
|
||
| points.Add(spawnPoint); |
There was a problem hiding this comment.
this line should probably be in the if block above. If so, combine and invert the ifs
| /// The data needed to validate the spawning conditions. | ||
| /// For example, checking whether the player can spawn on this SpawnPoint based on their job (e.g. an assistant can't spawn on security SpawnPoint) | ||
| /// </summary> | ||
| public SpawnPointData SpawnPointData; |
There was a problem hiding this comment.
Turn into public getter, private setter; serialize for unity inspector
| } | ||
|
|
||
| /// <summary> | ||
| /// Synces the position of a networkobject with the spawn point |
There was a problem hiding this comment.
| /// Synces the position of a networkobject with the spawn point | |
| /// Syncs the position of a networkobject with the spawn point |
| /// </summary> | ||
| /// <param name="spawnPoints"></param> The spawn points to check | ||
| /// <returns></returns> | ||
| private SpawnPoint GetValidSpawnPoint(List<SpawnPoint> spawnPoints) |
| [Server] | ||
| public SpawnPoint HandleSpawning(Player player, bool isLateJoin) | ||
| { | ||
| RoleData roleData = _roleSubSystem.GetRoleFromPlayer(player); |
There was a problem hiding this comment.
Move this check to the else block below. Log an error, but return a LateJoin spawn
| /// <param name="player"></param> The player we want to handle spawning for | ||
| /// <param name="isLateJoin"></param> Whether the entity is late-joining or not | ||
| [Server] | ||
| public SpawnPoint HandleSpawning(Player player, bool isLateJoin) |
There was a problem hiding this comment.
Rename to GetAvailableSpawnPoint
| public RoundState RoundState | ||
| { | ||
| get => _roundState; | ||
| set => _roundState = value; |
There was a problem hiding this comment.
| set => _roundState = value; | |
| protected set => _roundState = value; |
Summary
Adds SpawnPoints which can be used by mappers for jobs and late-join players.
Map Editor has 3 new prefabs, SecuritySpawnPoint, AssistantSpawnPoint and LateJoinSpawnPoint.
The current maps have late join spawn points mapped in them (they act as fallback but work for now)
PR checklist
Pictures/Videos
2026-02-12.16-38-20.mp4
Testing
Networking checklist
Changes
Related issues/PRs
Closes #1435