Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 133 additions & 0 deletions docs/plans/role-visibility-matrix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
# Role visibility: Slender / Survivor / Spectator

Status: Implemented
Branch: `fix/role-visibility-matrix`

## 1. Target matrix

Who (row) may see whom (column) as an entity:

| Viewer | Slender | Survivor | Spectator |
|---|---|---|---|
| **Slender** | – | yes | **never** |
| **Survivor** | only while revealed | yes | **never** |
| **Spectator** | same as the survivor view | yes | **yes** |

"only while revealed" means `Tags.HIDDEN == VISIBLE`, set by `SlenderBar.enterDraining()`.

Chat:

| Sender | Recipients |
|---|---|
| Survivor | everyone |
| Slender | everyone |
| Spectator | spectators only — **in every phase** |

## 2. Root cause

Visibility used to be driven by **two competing mechanisms** that desynchronized:

1. **Viewable rule** (`updateViewableRule`) — maintains `EntityView.Option.bitSet`
2. **Manual viewer packets** (`updateNewViewer` / `updateOldViewer`) — purely packet based,
they never touch the bit set (Minestom `Entity.java:553` / `:579`)

Observed consequences:
- `GameStartListener:58` hid the slender through `updateOldViewer` while the bit set kept everyone
registered. A later `updateViewableRule()` therefore saw `isRegistered == true` and sent **no**
spawn packet.
- `enterDraining` sent `updateNewViewer` manually, after which `SlenderBarTrigger:58` re-evaluated
the rule -> **duplicated spawn packets**.
- The automatic transition once the bar ran dry (`SlenderBar:109`) went through mechanism 2 **only**
and was silently reverted by the next `updateViewableRule()`.

**Decision: a single layer.** Per-viewer predicates exclusively. Every manual
`updateNewViewer` / `updateOldViewer` / `broadcastPlayPacket` call is gone.

This works because the predicate receives the **viewer** in Minestom `2026.07.22-26.2`
(`EntityView.updateRule0`, `predicate.test(entity)`) and is also evaluated whenever somebody enters
range (`EntityView:73`).

Constraint: `addViewer` / `removeViewer` must **not** be used — they record the player in
`manualViewers`, and `update()` skips those players permanently (`EntityView:258`).

## 3. Findings and measures

### P0 — target matrix (implemented here)

| # | File:line | Finding | Measure |
|---|---|---|---|
| 1 | `spectator/SpectatorService.java:61` | `_ -> false` — spectators could not see each other | `viewer -> TeamHelper.isSpectatorTeam(viewer)` |
| 2 | `team/TeamHelper.java:77` | predicate ignored the viewer | explicit per-viewer rule via `VisibilityRules` |
| 3 | `listener/game/SlenderReviveListener.java:33-42` | set neither rule nor `Tags.HIDDEN` -> revived slender permanently visible | set both |
| 4 | `team/TeamHelper.java:75-79` | rule installed before `Tags.HIDDEN` existed -> slender visible between GamePrepare and GameStart | set `Tags.HIDDEN = HIDDEN` in `assignSlender` |
| 5 | `listener/stamina/StaminaStateChangeListener.java:26-37` | bypassed the rule, broadcast metadata to everyone | replaced by `updateViewableRule()` |
| 6 | `listener/game/GameStartListener.java:54-59` | same pattern | same |
| 7 | `stamina/SlenderBar.java:109` | automatic transition did not re-evaluate the rule | resolved by removing the competing path instead |
| 8 | `utils/ViewRuleUpdater.java:11-13` | `isViewAble` — dead code, inverted name | deleted |
| 9 | `utils/ViewRuleUpdater.java:15-22` | iterated survivors twice | simplified into `VisibilityRules.refresh` |
| 10 | `listener/PlayerChatListener.java:31-34` | fail-open: `null instanceof GamePhase == false` leaked spectator chat to everyone, plus a gap during the restart phase | fail-closed, independent of the phase |
| 11 | `stamina/SlenderBarHelper.java:82-91` | `setHealth` without team/game mode filter -> spectators took damage | survivors only |
| 12 | `stamina/SlenderBarHelper.java:123-133` | teleport sound without role filter -> spectators heard when the slender vanished | survivors only |
| 13 | `listener/player/CygnusPlayerTickListener.java:19-22` | jumpscare on every tick without filter -> slender received `DARKNESS` for 40 ticks | survivors only |

Follow-up chain of #11: a spectator died -> `PlayerDeathListener:55` broadcast a second death
message, `:57` removed the tag, `:58` fired another `SpectatorAddEvent` -> `SpectatorService.join`
ran twice, `:61` re-checked the finish condition. Fixing #11 removes the cause.

### P1 — adjacent leaks, deliberately NOT part of this change

Found during the analysis; each needs a design decision:

- **`team/TeamHelper.java:157-161`** — `updateTabList` gives the slender the display name
`"⛧ " + name` in red. `setDisplayName` broadcasts `UPDATE_DISPLAY_NAME` to everyone
(Minestom `Player.java:1188-1193`). **Every survivor and every spectator immediately sees who the
slender is in the tab list** — and the same display name sits under every chat message
(`PlayerChatListener:62`). The largest remaining leak, but possibly intentional design.
- **`common/.../page/PageProvider.java:165`** — page discoveries are broadcast to everyone, so the
slender learns in real time which survivor is making progress where.
- **`PlayerDeathListener.java:55`** — death messages go to everyone; the slender gets every kill
confirmed.
- **`entity/DeadPlayerMannequin.java:109-114`** — particles sent through
`instance.sendGroupedPacket` bypass every viewable rule and reveal the corpse position even while
a jumpscare hides it.
- **`listener/game/GameFinishListener.java:36-41`** — dead players carry `SPECTATOR_KEY`, so
`isSlenderTeam` is false and they all render as survivor boxes.
- **`utils/ScoreboardDisplay.java`** — dead code; `getTeamName:86-88` would map spectators into the
survivor team, and `TeamCreator:41-43` lacks the `TeamNameComponent`, which would throw an NPE.
- **`Cygnus.java`** (`finishGame`) — no reset of rule / `Tags.HIDDEN` / `TEAM_KEY`. Harmless today
(`RestartPhase` kicks everyone and calls `stopCleanly()`, one process serves exactly one round),
but immediately relevant once a round reset or map switch is introduced.

## 4. Implementation

### `game/.../visibility/VisibilityRules.java`

Holds the matrix in a single place, as per-viewer predicates:

- `slenderRule(Player slender)` -> `viewer -> !isHidden(slender)`
- `spectatorRule()` -> `TeamHelper::isSpectatorTeam`
- survivors deliberately get **no** rule (Minestom default = visible to everyone)
- `refresh(Player)` -> re-evaluates the rules of the player and of every other online player,
which is required because `spectatorRule()` tests the *viewer*

`ViewRuleUpdater` was removed and absorbed into this class.

### Tests

Based on `CygnusPlayerTestBase` + `MicrotusExtension`, asserting through `player.isViewer(other)`.

`SpectatorServiceTest.testJoinMakesPlayerInvisibleToOthers` cemented finding 1 and was replaced.

One test per matrix cell plus regression tests:
- spectator sees spectator, spectator sees survivor
- survivor does not see spectator, slender does not see spectator
- spectator does not see a hidden slender, spectator sees a revealed slender
- revived slender is hidden
- spectator chat reaches spectators only — game phase, restart phase and without an active phase
- spectator takes no slender damage, only survivors hear the sounds, no jumpscares for
slender/spectator

### Known follow-up

`stamina` -> `team` is now a package cycle, because `TeamHelper` reads the `HIDDEN` constant from
`SlenderBarHelper`. Moving `VISIBLE` / `HIDDEN` into a neutral holder would resolve it.
11 changes: 2 additions & 9 deletions game/src/main/java/net/onelitefeather/cygnus/Cygnus.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import net.theevilreaper.xerus.api.team.Team;
import net.theevilreaper.xerus.api.team.TeamService;
import net.minestom.server.MinecraftServer;
import net.minestom.server.entity.Player;
import net.minestom.server.event.player.AsyncPlayerConfigurationEvent;
import net.minestom.server.event.player.PlayerChatEvent;
import net.minestom.server.event.player.PlayerDeathEvent;
Expand Down Expand Up @@ -78,10 +77,8 @@
import net.onelitefeather.cygnus.stamina.SlenderBarTrigger;
import net.onelitefeather.cygnus.stamina.StaminaService;
import net.onelitefeather.cygnus.utils.StaminaHelper;
import net.onelitefeather.cygnus.utils.ViewRuleUpdater;
import net.onelitefeather.cygnus.view.GameView;
import net.onelitefeather.cygnus.view.GameViewImpl;
import org.jetbrains.annotations.NotNull;

import java.nio.file.Path;
import java.util.Optional;
Expand Down Expand Up @@ -162,7 +159,7 @@ private void initListener() {
this.resourcePackService.ifPresent(service -> service.registerListener(manager));
Team spectatorTeam = this.teamService.getTeam(GameConfig.SPECTATOR_KEY)
.orElseThrow(() -> new IllegalStateException("Spectator team not found"));
manager.addListener(PlayerChatEvent.class, new PlayerChatListener(spectatorTeam, phaseSupplier));
manager.addListener(PlayerChatEvent.class, new PlayerChatListener(spectatorTeam));
manager.addListener(GameMapLoadEvent.class, _ -> this.mapProvider.loadGameMap());
manager.addListener(GamePrepareEvent.class, _ -> StaminaHelper.initStaminaObjects(this.teamService, this.staminaService));
registerCancelListener(manager);
Expand All @@ -172,7 +169,7 @@ private void registerGameListener() {
Supplier<Phase> phaseSupplier = this.linearPhaseSeries::getCurrentPhase;
GlobalEventHandler handler = MinecraftServer.getGlobalEventHandler();

SlenderBarTrigger trigger = new SlenderBarTrigger(this.staminaService::getSlenderBar, this::triggerViewRuleUpdate);
SlenderBarTrigger trigger = new SlenderBarTrigger(this.staminaService::getSlenderBar);
handler.addListener(PlayerUseItemEvent.class, new SlenderItemListener(trigger));
handler.addListener(GameFinishEvent.class, new GameFinishListener());
handler.addListener(GameStartEvent.class, new GameStartListener(this.teamService, this.ambientProvider, this.staminaService, this.pageProvider));
Expand Down Expand Up @@ -225,8 +222,4 @@ private void finishGame() {
MinecraftServer.getPacketListenerManager().setPlayListener(ClientEntityActionPacket.class, EntityActionListener::listener);
MinecraftServer.getPacketListenerManager().setPlayListener(ClientSettingsPacket.class, SettingsListener::listener);
}

private void triggerViewRuleUpdate(@NotNull Player player) {
ViewRuleUpdater.updateViewer(player, this.teamService.getTeam(GameConfig.SURVIVOR_KEY).orElseThrow());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,59 @@
import net.kyori.adventure.text.Component;
import net.kyori.adventure.text.format.NamedTextColor;
import net.minestom.server.event.player.PlayerChatEvent;
import net.onelitefeather.cygnus.phase.GamePhase;
import net.onelitefeather.cygnus.team.TeamHelper;
import net.theevilreaper.xerus.api.phase.Phase;
import net.theevilreaper.xerus.api.team.Team;

import java.util.function.Consumer;
import java.util.function.Supplier;

/**
* Formats every chat message and enforces the spectator chat isolation.
* <p>
* The visibility matrix allows a spectator to read survivor and slender chat, but a message written
* by a spectator must never reach a survivor or the slender. That rule depends on the sender's team
* only and is deliberately <b>not</b> tied to the currently active phase:
* <ul>
* <li>a phase check is fail-open, because the phase series reports {@code null} while no phase
* is running and {@code null instanceof GamePhase} evaluates to {@code false}, which would leak
* spectator chat to everyone;</li>
* <li>the {@code RestartPhase} that runs after the game phase finished is not a
* {@code GamePhase} either, so the isolation would silently disappear for its whole runtime.</li>
* </ul>
* Outside a running match nobody carries the spectator team tag, so the team based check is a no-op
* there and no additional phase guard is needed.
*
* @author TheMeinerLP
* @version 2.0.0
* @since 1.0.0
**/
public final class PlayerChatListener implements Consumer<PlayerChatEvent> {

private static final Component MESSAGE_PREFIX = Component.text("≫", NamedTextColor.YELLOW);

private final Team spectatorTeam;
private final Supplier<Phase> phaseSupplier;

public PlayerChatListener(Team spectatorTeam, Supplier<Phase> phaseSupplier) {
/**
* Creates a new instance of the {@link PlayerChatListener}.
*
* @param spectatorTeam the team which receives the messages written by a spectator
*/
public PlayerChatListener(Team spectatorTeam) {
this.spectatorTeam = spectatorTeam;
this.phaseSupplier = phaseSupplier;
}

@Override
public void accept(PlayerChatEvent event) {
//TODO: Improve chat during each phase
event.setFormattedMessage(this.setLobbyLayout(event));

if (phaseSupplier.get() instanceof GamePhase && TeamHelper.isSpectatorTeam(event.getPlayer())) {
event.getRecipients().clear();
spectatorTeam.sendMessage(event.getFormattedMessage());
}
if (!TeamHelper.isSpectatorTeam(event.getPlayer())) return;

// Minestom pre-fills the recipients with every online player. Dropping all of them and
// re-delivering to the spectator team keeps the message inside the spectator group even if
// the team and the player tag ever drift apart, because the fallback is "nobody" and never
// "everybody".
event.getRecipients().clear();
this.spectatorTeam.sendMessage(event.getFormattedMessage());
}

private Component setLobbyLayout(PlayerChatEvent event) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package net.onelitefeather.cygnus.listener.game;

import net.kyori.adventure.text.Component;
import net.minestom.server.MinecraftServer;
import net.minestom.server.entity.Player;
import net.minestom.server.event.EventDispatcher;
import net.minestom.server.utils.PacketSendingUtils;
import net.onelitefeather.cygnus.ambient.AmbientProvider;
import net.onelitefeather.cygnus.common.Messages;
import net.onelitefeather.cygnus.common.Tags;
Expand All @@ -16,6 +14,7 @@
import net.onelitefeather.cygnus.stamina.StaminaService;
import net.onelitefeather.cygnus.team.TeamHelper;
import net.onelitefeather.cygnus.utils.Items;
import net.onelitefeather.cygnus.visibility.VisibilityRules;
import net.theevilreaper.xerus.api.team.Team;
import net.theevilreaper.xerus.api.team.TeamService;

Expand Down Expand Up @@ -51,12 +50,11 @@ private void handleSlenderStart() {
slenderPlayer.sendMessage(Messages.SLENDER_JOIN_PART);
Items.setSlenderEye(slenderPlayer);

PacketSendingUtils.broadcastPlayPacket(slenderPlayer.getMetadataPacket());
MinecraftServer.getConnectionManager().getOnlinePlayers()
.stream()
.filter(p -> !p.equals(slenderPlayer))
.forEach(slenderPlayer::updateOldViewer);
PacketSendingUtils.broadcastPlayPacket(slenderPlayer.getMetadataPacket());
// Hiding the slender goes exclusively through the viewable rule. The previous
// updateOldViewer/broadcastPlayPacket combination only sent packets: it left the viewer bit set
// untouched, so the next rule evaluation considered every player still registered and skipped the
// spawn packet when the slender revealed themselves again.
VisibilityRules.refresh(slenderPlayer);
}

private void handleSurvivorStart() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
import net.onelitefeather.cygnus.common.config.GameConfig;
import net.onelitefeather.cygnus.common.map.GameMap;
import net.onelitefeather.cygnus.event.SlenderReviveEvent;
import net.onelitefeather.cygnus.stamina.SlenderBarHelper;
import net.onelitefeather.cygnus.stamina.StaminaService;
import net.onelitefeather.cygnus.team.TeamHelper;
import net.onelitefeather.cygnus.utils.Items;
import net.onelitefeather.cygnus.visibility.VisibilityRules;

import java.util.function.Consumer;
import java.util.function.Supplier;
Expand All @@ -29,11 +31,21 @@ public SlenderReviveListener(Supplier<GameMap> gameMapSupplier, StaminaService s
this.staminaService = staminaService;
}

/**
* Turns the given player into the new slender.
* <p>
* Hidden state and viewable rule are set exactly like in the initial team allocation. Without them the
* revived slender would keep the survivor default and stay visible to everybody for the rest of the round.
*
* @param event the revive event carrying the player to promote
*/
@Override
public void accept(SlenderReviveEvent event) {
Player player = event.getPlayer();
staminaService.setSlenderBar(player, true);
player.setTag(Tags.TEAM_KEY, GameConfig.SLENDER_KEY);
player.setTag(Tags.HIDDEN, SlenderBarHelper.HIDDEN);
player.updateViewableRule(VisibilityRules.slenderRule(player));
GameMap gameMap = gameMapSupplier.get();
if (gameMap != null && gameMap.getSlenderSpawn() != null) {
player.teleport(gameMap.getSlenderSpawn());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,18 @@
import net.minestom.server.event.player.PlayerTickEvent;
import net.onelitefeather.cygnus.jumpscare.JumpScareManager;
import net.onelitefeather.cygnus.player.CygnusPlayer;
import net.onelitefeather.cygnus.team.TeamHelper;

import java.util.function.Consumer;

/**
* Handles the per tick logic of a {@link Player} like the jump scare detection, the sprint blocking
* and the heartbeat.
*
* @author theEvilReaper
* @version 1.1.0
* @since 1.0.0
*/
public final class CygnusPlayerTickListener implements Consumer<PlayerTickEvent> {

private final JumpScareManager jumpscareManager;
Expand All @@ -19,7 +28,9 @@ public CygnusPlayerTickListener(JumpScareManager jumpscareManager) {
public void accept(PlayerTickEvent event) {
Player player = event.getPlayer();

this.jumpscareManager.checkTurnAround(player);
if (isJumpScareTarget(player)) {
this.jumpscareManager.checkTurnAround(player);
}

if (!(player instanceof CygnusPlayer cygnusPlayer)) return;

Expand All @@ -30,5 +41,19 @@ public void accept(PlayerTickEvent event) {

cygnusPlayer.tickHeartbeat();
}
}

/**
* Checks whether the given player is allowed to receive a jump scare.
* <p>
* A jump scare spawns a phantom corpse and applies {@code DARKNESS} for 40 ticks. Only survivors
* may receive it: for the slender it would be a direct gameplay interference and for a spectator
* it would blind a player that is not part of the round anymore. The check is fail closed, so an
* untagged player never receives a scare either.
*
* @param player the player to check
* @return {@code true} if the player is a living survivor
*/
private static boolean isJumpScareTarget(Player player) {
return TeamHelper.isSurvivorTeam(player) && !player.isDead();
}
}
Loading
Loading