Placeholder#46
Conversation
Co-authored-by: NicholaiMadias <73684379+NicholaiMadias@users.noreply.github.com>
✅ Deploy Preview for gulfnexus ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a “Veil-aware” language rendering helper module and adds a small preview UI in index.html wired up from main.js, allowing toggling hasSeenTheVeil to see how copy shifts.
Changes:
- Add
veil-language.jswith exported helpers for Veil-dependent text rendering. - Extend
main.jsto manage a localplayerState.hasSeenTheVeiltoggle and render preview text into new DOM nodes. - Add a new “Assumed Remembrance (Veil)” section + styles to
index.html.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| veil-language.js | New helper module providing Veil-aware string rendering functions. |
| main.js | Imports Veil helpers and wires up a checkbox-driven preview renderer. |
| index.html | Adds the Veil preview section markup and styling. |
| import { initMatchMaker } from './match-maker-ui.js'; | ||
| import { | ||
| renderArchivistDiagnostic, | ||
| renderAssumedLine, | ||
| renderEnvironmentPrompt, | ||
| renderVeilVariant, | ||
| } from './veil-language.js'; |
There was a problem hiding this comment.
The PR title/description are placeholders ("Placeholder" / "temp"), which makes it hard to review intent and scope. Please update the PR metadata to describe the Veil language module + preview UI being added.
| export function renderVeilVariant(lineSpec, player) { | ||
| const seen = veilSeen(player); | ||
| if (seen && lineSpec.veilVariant) { | ||
| return lineSpec.veilVariant; | ||
| } | ||
| return renderAssumedLine(lineSpec.base, lineSpec.veilFragment, seen); | ||
| } |
There was a problem hiding this comment.
renderVeilVariant computes seen but then passes that boolean into renderAssumedLine, which recomputes seen via veilSeen(...) and also makes the third parameter semantically confusing (it's named player). Consider passing player through (or refactoring renderAssumedLine to accept seen explicitly) to avoid redundant work and clarify the API.
| return renderAssumedLine(lineSpec.base, lineSpec.veilFragment, seen); | ||
| } | ||
|
|
||
| export function renderArchivistDiagnostic(base, veilFragment, player) { | ||
| const seen = veilSeen(player); | ||
| const clipped = seen ? firstSentence(base) : base; | ||
| return renderAssumedLine(clipped, veilFragment, seen); |
There was a problem hiding this comment.
Same pattern here: renderArchivistDiagnostic computes seen but passes the boolean into renderAssumedLine, which then calls veilSeen(...) again. Passing player (or adjusting the helper signature) would keep parameter meaning consistent and avoid the extra conversion step.
| return renderAssumedLine(lineSpec.base, lineSpec.veilFragment, seen); | |
| } | |
| export function renderArchivistDiagnostic(base, veilFragment, player) { | |
| const seen = veilSeen(player); | |
| const clipped = seen ? firstSentence(base) : base; | |
| return renderAssumedLine(clipped, veilFragment, seen); | |
| return renderAssumedLine(lineSpec.base, lineSpec.veilFragment, player); | |
| } | |
| export function renderArchivistDiagnostic(base, veilFragment, player) { | |
| const seen = veilSeen(player); | |
| const clipped = seen ? firstSentence(base) : base; | |
| return renderAssumedLine(clipped, veilFragment, player); |
temp