refactor: extract shared overlay foundation - #177
Open
TheMeinerLP wants to merge 12 commits into
Open
Conversation
Contributor
Test results 285 files 285 suites 1m 56s ⏱️ For more details on these failures, see this check. Results for commit f2f377c. ♻️ This comment has been updated with latest results. |
This was referenced Aug 12, 2026
This comment has been minimized.
This comment has been minimized.
TheMeinerLP
force-pushed
the
refactor/overlay-foundation
branch
from
August 23, 2026 09:17
af3dead to
e45d219
Compare
theEvilReaper
requested changes
Aug 23, 2026
Contributor
There was a problem hiding this comment.
Commands should be removed before merging
Contributor
There was a problem hiding this comment.
Test should be removed before merging
Four different services (tunnel vision, blood splatter, slender gaze, and the stamina bar) each hand-rolled the same start/stop guard around a Minestom Task: a nullable field, a null-check before scheduling, and a null-check before cancelling, repeated with small variations at every call site. Extract that guard once as RepeatingTask so the follow-up feature branches can adopt a single, tested implementation instead of copying the pattern a fifth time.
Five separate hand-rolled per-player maps existed across the tunnel vision, blood splatter, and slender gaze code, each keyed by UUID and each managing its own put-on-join/remove-on-leave lifecycle by hand. The duplication made every one of those call sites a place a leak or a stale-entry bug could hide. PlayerState wraps that lifecycle once behind a small, tested type, so the three follow-up feature branches can store their per-player values without re-deriving the same map bookkeeping.
The tunnel vision and slender gaze code each clamped values into range with their own Math.min(max, Math.max(min, value)) expression, which is easy to get backwards (min/max swapped) and gives no shared place to fix it once. Add int and double overloads of Helper.clamp so the follow-up feature branches can call one well-tested method instead of repeating the expression.
Three texture-key builders existed across the tunnel vision, blood splatter, and slender gaze designs, each assembling equipment-slot texture paths with its own naming convention (stage_<n>, <direction>_<variant>_<frame>, level_<level>_<frame>), so nothing about them could be reused or tested together. Add the shared overlay package: ScreenOverlay and OverlayLayer describe where an overlay sits and how it renders, OverlayProperties and EquipmentScreenOverlay handle applying it to a player's equipment slot, and OverlayTextureKeys unifies the three texture-key conventions behind one type. This is the rendering base the three follow-up feature branches (tunnel vision, blood splatter, slender gaze) build their per-effect logic on top of.
The three preview commands (tunnel vision, blood, glitch) each hand-rolled an identical private static asPlayer(CommandSender) check to narrow a CommandSender down to a Player, differing only in the error string sent back to the console. Extract that check once as CommandSenders.asPlayer, a stateless static method rather than an abstract base command, since narrowing the sender is the only thing the three commands have in common and a shared base class would force them into one constructor shape and inheritance chain for a single one-line check.
PlayerState and RepeatingTask sat in common/util, but common is what game and setup share, and setup uses neither. They move to game/utils, next to the other game-side helpers (Items, ScoreboardDisplay, StaminaHelper, ViewRuleUpdater), which also removes the second util package common/util vs game/utils opened up. Helper.clamp is dropped again: its body was a straight delegation to Math.clamp, and the codebase already calls Math.clamp directly in LobbyWaitingTask, CygnusPlayer and ColorUtil. HelperTest covered nothing else and goes with it. AmbientProvider now uses RepeatingTask instead of its own nullable Task field with the guard-and-return pair. It was one of the four copies the type was extracted from, so the extraction pays for itself here rather than only in the effects that land on top of this branch. RecordingScreenOverlay joins the test sources: every effect test needs the same recording ScreenOverlay, and writing it per test class is the same duplication this branch removes from the production code.
…lding it asPlayer assembled "<red>Only players " + reason from a sentence fragment each caller passed in. That put message assembly in a command helper while every other player-facing text in the project is a Component in Messages, and it left each caller holding half a sentence that only made sense once concatenated here. It now takes the finished Component and only sends it. The callers land in the three follow-up PRs and each bring their own Messages entry.
Each effect branch wrapped its own registration in its own `if (OverlayProperties.enabled())`. Those blocks land on the same lines, and the closing brace sits behind the conflict marker, so resolving one by keeping both sides yields two opened ifs and one brace - it does not compile, which is at least loud, but it is a conflict nobody should have to think about three times. registerOverlayListeners holds the gate once. Each effect adds its own line to it and nothing else, and the property named cygnus.overlays now actually governs all of them rather than however many blocks happened to be written.
PlayerState and RepeatingTask moved to game/utils in the foundation, and Helper.clamp is gone in favour of Math.clamp; both follow here. clearAll() becomes cleanUp(), which is what the round teardown is called on main (PageProvider, StaminaService, JumpScareManager). The two player-facing texts move into Messages: the usage line as a builder, next to the other builders that interpolate a value, and the players-only message as a constant now that CommandSenders takes a finished Component. show(Player, int) and hide(Player) collapse into preview(Player, int), where SlenderGaze.NONE clears. That is not an invented sentinel - tick() already branches on NONE coming out of SlenderGaze.levelOf, so hide() was a second spelling of a level the domain type already had. The recording ScreenOverlay both tests stood up is the foundation's shared one. The comment gating the effect in Cygnus described a vignette font that no longer exists, and OverlayProperties only named two of its three layers.
theEvilReaper
force-pushed
the
refactor/overlay-foundation
branch
from
August 23, 2026 19:57
0be1b7e to
f2f377c
Compare
Contributor
Job Summary for GradleBuild PR :: build
|
Contributor
Job Summary for GradleBuild PR :: build
|
Contributor
Job Summary for GradleBuild PR :: build
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
This is the shared foundation extracted from the mixed-together tunnel vision work, split
out on its own so it can land ahead of the three feature branches that build on it:
tunnel vision, blood splatter, and slender gaze. Each of those effects draws
something onto the player's screen and tracks per-player state, and each was about to
reinvent the same handful of building blocks independently. This branch adds those
building blocks once, tested, so the three feature PRs can adopt them instead of
duplicating them a third and fourth time.
What each new type replaces
common/util/PlayerStatereplaces five hand-rolled per-player maps spread acrossthe tunnel vision, blood splatter, and slender gaze code, each keyed by UUID and each
managing its own put-on-join/remove-on-leave lifecycle by hand — a natural place for a
leak or a stale entry to hide.
common/util/RepeatingTaskreplaces four copies of the same scheduler start/stopguard (tunnel vision, blood splatter, slender gaze, and the stamina bar), each a
nullable
Taskfield with a null-check before scheduling and a null-check beforecancelling.
overlay/OverlayTextureKeysreplaces three texture-key builders with threedifferent conventions (
stage_<n>,<direction>_<variant>_<frame>,level_<level>_<frame>), unifying them behind one type so equipment-slot texture pathsare assembled the same way everywhere.
command/CommandSendersreplaces three copies of the same command sender check(
TunnelVisionCommand,BloodCommand,GlitchCommand), each hand-rolling an identicalprivate static @Nullable Player asPlayer(CommandSender), differing only in the errorstring sent back to the console.
common/util/Helper#clamp(int and double overloads) replaces the hand-rolledMath.min(max, Math.max(min, value))expressions the tunnel vision and slender gaze codeeach wrote independently.
overlay/ScreenOverlay,OverlayLayer,OverlayProperties,EquipmentScreenOverlayform the shared rendering base: where an overlay sits, how it renders, and how it gets
applied to a player's equipment slot. The three feature branches layer their per-effect
logic (tunnel vision stages, blood splatter frames, slender gaze intensity) on top of
this instead of each building their own equipment-overlay plumbing.
Scope
Only the foundation types and their tests are included. No feature code (tunnel vision,
blood splatter, slender gaze, the preview commands, or
Cygnus.javawiring) is part ofthis branch — those land as separate follow-up PRs on top of this one.
How to verify
All existing and new tests pass; both modules compile cleanly against
main.