Skip to content

Add "Ghosts can Talk Ingame" and "Grace Period" settings for Meetings/Lobby Only mode - #384

Open
rewalo wants to merge 4 commits into
OhMyGuus:nightlyfrom
rewalo:feature/GhostsCanTalkIngameGracePeriod
Open

Add "Ghosts can Talk Ingame" and "Grace Period" settings for Meetings/Lobby Only mode#384
rewalo wants to merge 4 commits into
OhMyGuus:nightlyfrom
rewalo:feature/GhostsCanTalkIngameGracePeriod

Conversation

@rewalo

@rewalo rewalo commented Dec 1, 2025

Copy link
Copy Markdown

Added two new lobby settings that enhance the "Meetings/Lobby Only" voice mode:

1. Ghosts can Talk Ingame

  • Requirement: Can only be enabled when "Meetings/Lobby Only" is enabled
  • Functionality:
    • Allows ghosts to talk and hear each other during the game (TASKS state)
    • Ghosts can hear each other anywhere on the map (distance-independent, like meetings)
    • Ghosts can hear alive players during grace period (if enabled)
    • In meetings, ghosts can hear both alive players and other ghosts

2. Grace Period

  • Requirement: Can only be enabled when "Meetings/Lobby Only" is enabled
  • Functionality:
    • Customizable delay (0-10 seconds, 0.5s increments) before voices are cut off
    • Applies when transitioning from LOBBY -> TASKS, LOBBY -> DISCUSSION, or between TASKS <-> DISCUSSION
    • Prevents abrupt voice cutoff when games/meetings start
    • Allows both hearing and talking during the grace period

Note: I didn't add extra translations because I don't speak the languages. Sorry :C

@rewalo
rewalo marked this pull request as draft December 2, 2025 03:32
@rewalo
rewalo marked this pull request as ready for review December 2, 2025 03:36
@rewalo
rewalo force-pushed the feature/GhostsCanTalkIngameGracePeriod branch from c69b487 to c6e603b Compare December 2, 2025 03:51
@greluc

greluc commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

I'm not a maintainer here, so please treat this as input rather than a verdict.

Two things up front:

The feature isn't a duplicate. I checked v3.2.1 (64a321a, current nightly): no ghostsCanTalkIngame, no gracePeriod, and GameState.TASKS still has only the unconditional if (activeLobbySettings.meetingGhostOnly) { endGain = 0; }. Both settings are worth having.

But the PR is built against a tree that no longer exists. The 3.2.0 restructure deleted both files this PR edits. Rebasing won't help; this needs a port:

PR touches Now lives in
src/renderer/Voice.tsxcalculateVoiceAudio src/renderer/voice/spatialAudio.ts — now a pure function, VoiceAudioInputVoiceAudioResult
src/renderer/Voice.tsxdefaultlocalLobbySettings defaultLobbySettings in src/renderer/voice/types.ts
src/renderer/settings/Settings.tsx → lobby checkboxes src/renderer/settings/sections/LobbySection.tsx, via <SwitchRow>
src/renderer/settings/SettingsStore.tsx → schema src/main/settingsStore.ts
call site is src/renderer/voice/AudioController.ts:513

The pure-function change is the one that matters most for this feature: spatialAudio.ts has no access to React refs, so the grace-period timestamp can't live in a useRef any more — it has to enter through VoiceAudioInput (which, as it happens, fixes two of the problems below for free).

Everything after this point is from reading c6e603b against its merge base 403dd4f. I did not run the toolchain, so the lint points are read off .eslintrc.yml / .prettierrc.yaml and the source, not from a linter run. I'm listing them anyway because they're the substance worth carrying into the port.


Logic to fix while porting

1. The DISCUSSION branch is a no-op — Voice.tsx:399

if (lobbySettings.meetingGhostOnly && lobbySettings.ghostsCanTalkIngame) {
    if (!me.isDead && other.isDead) {
        endGain = 0;
    }
} else {
    if (!me.isDead && other.isDead) {
        endGain = 0;
    }
}

Both arms are identical, so the whole thing collapses to the pre-existing if (!me.isDead && other.isDead) endGain = 0;. Nothing in the DISCUSSION case reads isInGracePeriod, so the "Applies when transitioning from ... LOBBY -> DISCUSSION, or between TASKS <-> DISCUSSION" part of the description isn't actually implemented — and the matching stamp at Voice.tsx:1317 (oldGameState === TASKS && gameState === DISCUSSION) is dead along with it.

Worth deciding which you meant. If entering a meeting never needs a grace period (audio is already open there), drop both the branch and that stamp. If it should, DISCUSSION needs to actually consult the flag.

2. One frame of silence at exactly the transition — Voice.tsx:1307

gracePeriodRef.current = Date.now() is set in a useEffect, which runs after commit. But calculateVoiceAudio is called from the otherPlayers useMemo (line 1245), which runs during render.

So on the frame where gameState flips LOBBY -> TASKS, the memo runs first, gracePeriodRef.current is still 0 (reset while in LOBBY), isInGracePeriod is false, and every peer's gain is set to 0. The effect stamps the ref afterwards, and the next frame restores gain to 1. That's a ~30–50 ms hole with two hard discontinuities at precisely the moment the feature exists to smooth over — the click you're removing still fires, it just gets an echo.

Deriving the grace start from the state transition rather than from a post-commit effect avoids it. In the 3.2.x layout this is largely forced on you anyway: the timestamp has to be computed by the caller and passed in through VoiceAudioInput.

3. Raising the grace period mid-round re-opens a closed window — Voice.tsx:323

The window is recomputed every frame from a stored start timestamp plus the live setting:

Date.now() < gracePeriodRef.current + gracePeriodMs

Round starts at T with Grace Period = 2s, window closes at T+2s. At T+30s the host drags the slider to 60s — the next frame evaluates T+30s < T+60000 as true, the grace period re-opens 30 seconds into the round, and every alive player unmutes mid-game. Storing a deadline at stamp time (Date.now() + gracePeriodMs) makes it immune to later slider moves.

4. "Only Ghosts can Talk/Hear" leaves the dependent settings on — Settings.tsx:584

The meetingGhostOnly handler correctly resets both new settings when it's switched off (lines 605–613), but the deadOnly handler clears meetingGhostOnly without touching them:

() => updateLocalLobbySettingsBuffer({ meetingGhostOnly: false, deadOnly: newValue }),

Enable Meetings/Lobby Only → tick Ghosts can Talk Ingame → Grace Period 5s → tick Only Ghosts can Talk/Hear. meetingGhostOnly goes false, but the two values stay in the buffer: both controls render checked/at-5s and disabled, and the values are persisted on close and broadcast to every peer. Re-ticking Meetings/Lobby Only later brings them silently back. That's the one path that doesn't enforce "can only be enabled when Meetings/Lobby Only is enabled". The same pair of handlers exists in LobbySection.tsx:98 and :113 today, so the fix ports across unchanged.

5. gracePeriod from the host is used unclamped — Voice.tsx:1114

const newSettings = {...defaultlocalLobbySettings, ...parsedData};

The slider clamps to 0–10; the receive path doesn't. A host on a modified client sending {"meetingGhostOnly":true,"gracePeriod":1e12} makes Date.now() < stamp + 1e15 permanently true, so Meetings/Lobby Only becomes a no-op for the whole lobby while every client's UI still shows it enabled. A non-numeric value gives NaN and the setting silently does nothing instead. A clamp next to the default merge covers both. (maxDistance has the same gap today, so this is inherited rather than introduced — but this field switches a restriction off, which makes it worth more than the others.)

6. Don't port the else if change at Voice.tsx:390

-} else {
-    if (other.isDead && !me.isDead) {
+} else if (other.isDead && !me.isDead) {
+    if (!lobbySettings.meetingGhostOnly) {
         endGain = 0;
     }
 }

This is behaviour-preserving today, because the new block at 325–343 already zeroes that combination. But it turns a general "the living never hear the dead" backstop into something conditional on an unrelated setting, leaving one place enforcing it instead of two. Upstream has since made the same structural change unconditionallyspatialAudio.ts:97 is now } else if (other.isDead && !me.isDead) { endGain = 0; } — so the port should keep it that way and drop the !meetingGhostOnly inner check.

7. lobbySettings switched to a ref that updates after render — Voice.tsx:300

const lobbySettings = lobbySettingsRef.current; replaces the render-scoped value, but that ref is assigned in a useEffect (line 732) which runs after the useMemo (line 1200) that calls this function — so gain decisions lag lobby-setting changes by one game frame. Moot after the port (activeLobbySettings arrives as a parameter), just don't reintroduce it.

Tooling / merge

8. no-case-declarationsVoice.tsx:322

case GameState.TASKS:
    endGain = 1;

    const gracePeriodMs = (lobbySettings.gracePeriod || 0) * 1000;

js.configs.recommended is in both the old .eslintrc.yml and the current eslint.config.mjs, so npm run lint should fail here either way. It's also a genuine TDZ hazard, not just style: const in a bare case is scoped to the whole switch block, so a case that runs before TASKS referencing either name throws ReferenceError: Cannot access 'gracePeriodMs' before initialization. Wrapping the case body in { } fixes it.

9. static/locales/en/translation.json is rewritten CRLF → LF

166 lines deleted, 171 added, for five real keys. It makes that file's diff unreadable, and since every line differs it's a guaranteed whole-file conflict — the current copy is still CRLF throughout and has grown to 197 lines. Note that .prettierrc.yaml on nightly now sets endOfLine: 'auto', so nothing is asking for the conversion. Re-committing with the original endings and only the five added lines clears it.

10. Prettier violations

'prettier/prettier': 'error' with printWidth: 120, trailingComma: 'es5'. I count trailing whitespace at Settings.tsx:607/608 and a whitespace-only line at Voice.tsx:398; eight lines over 120 columns (Settings.tsx 623, 635, 637, 638, 643, 654, 658, 663 — longest 152); and a missing ES5 trailing comma at Settings.tsx:611. npm run format clears all of them.

Smaller things

Repeated unreachable ternary — `Settings.tsx:623, 635, 643, 658.

!canChangeLobbySettings || !(canChangeLobbySettings ? localLobbySettingsBuffer.meetingGhostOnly : hostLobbySettings.meetingGhostOnly)

|| short-circuits, so the ternary only evaluates when canChangeLobbySettings is true — the hostLobbySettings arm is dead and the expression means !canChangeLobbySettings || !localLobbySettingsBuffer.meetingGhostOnly. Pasted four times (plus three value ternaries at 637, 654, 659), and it's what pushes six of those lines past 120 columns. Mostly moot after the port, since LobbySection.tsx already resolves this once into values / disabled / disabledReason.

The two _description strings are never used. ghosts_can_talk_ingame_description and grace_period_description are added but never passed to t(), so the explanations you wrote never reach a user. Good news for the port: <SwitchRow> in LobbySection.tsx takes a description prop that every other row already uses — they'd drop straight in.

Translations. Only en is updated, but fallbackLng: 'en' in src/renderer/language/i18n.ts means the other 34 locales show the English text rather than a raw key path, so nothing breaks. No need to apologise for that in the description :)

**Unit and formatting — Settings.tsx:654.** {t('...grace_period')}: {value}sputs the unit outsidet()so it can't be localised, and the raw value flips between1sand1.5s. The Voice Distance label right above uses .toFixed(1)` for that reason.

One design thought, take it or leave it. The stated problem is "prevents abrupt voice cutoff when games/meetings start", but gain is still assigned instantly — AudioController.ts:493, peer.gain.gain.value = gain. When the window expires the cut is exactly as abrupt as before, just at an unpredictable moment mid-round rather than at the meeting boundary. A short ramp at that one line (setTargetAtTime(gain, ctx.currentTime, 0.05)) would smooth every mute and unmute in the app, with no new lobby setting, no wire-format change and no host coordination. The grace period would then be a deliberate gameplay choice on top of that rather than a workaround for an unramped gain node — which I think is the stronger version of this PR, and the two are independent enough to land separately.


Scope, to be clear: the port is the blocker; 1–4 are what I'd want fixed while doing it; the rest is the maintainers' call. Thanks for building this.

Reviewed with the help of Claude Code. Every claim was checked against the source by hand, but the linter itself was not run.

@greluc

greluc commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

@rewalo — heads-up so you don't find it by accident: I've ported this to the 3.2.x tree as #411, because the two files this PR edits no longer exist on nightly and a rebase wasn't going to bridge that.

The feature and the design are yours — you're co-author on the commit, and the differences from your version are the points from my review above. If you'd rather carry it yourself, say the word and I'll close mine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants