Skip to content

Conversation

@4z0t
Copy link
Member

@4z0t 4z0t commented Dec 25, 2025

Description of the proposed changes

Hides chat in NIS mode for better immersion in coop missions.

Checklist

Summary by CodeRabbit

  • Bug Fixes
    • Fixed chat display behavior during non-interactive sequences to prevent unnecessary UI updates and unintended scrolling, ensuring a cleaner viewing experience during cinematics and other special game moments.

✏️ Tip: You can customize this high-level summary in your review settings.

@4z0t 4z0t requested review from lL1l1 and speed2CZ December 25, 2025 07:16
@coderabbitai
Copy link

coderabbitai bot commented Dec 25, 2025

📝 Walkthrough

Walkthrough

The changes modify NIS-mode chat UI behavior by relocating the OnNISBegin() function and adding a conditional guard in ReceiveChatFromSim that prevents chat display updates when in NIS mode.

Changes

Cohort / File(s) Change Summary
NIS Mode Chat Handling
lua/ui/game/chat.lua
Repositioned OnNISBegin() function to a different location in the file. Added guard condition in ReceiveChatFromSim to skip chat line recalculation and scrolling when NIS mode is active, preventing chat history UI updates during non-interactive sequences.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A hop through chat with NIS's call,
Where functions dance and guards install,
No updates flutter when sequences play,
The UI rests till scripted scenes sway.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive The description covers the main purpose and includes completed checklist items, but lacks the required changelog snippet documentation and reviewer assignment as specified in the template. Add a changelog snippet following the repository guidelines and request 2-3 reviewers from the recommended list to complete the checklist requirements.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Hide chat in NIS mode' clearly and specifically summarizes the main change: disabling chat visibility during non-interactive sequences for better immersion.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b1d2d40 and 9272b94.

📒 Files selected for processing (1)
  • lua/ui/game/chat.lua
🔇 Additional comments (1)
lua/ui/game/chat.lua (1)

880-888: Good approach to prevent chat updates during NIS mode.

The conditional guard correctly prevents chat display updates when in NIS mode while still preserving messages in history. The IsNISMode() function properly checks the NISActive flag set by NISMode() in gamemain.lua, and chat is correctly hidden/restored through the OnNISBegin() callback and the global NISMode(state) restoration logic when NIS mode ends.

Comment on lines +1565 to +1574
function OnNISBegin()
CloseChatConfig()
GUI.bg:Hide()
GUI.chatEdit.edit:AbandonFocus()
GUI.bg:SetNeedsFrameUpdate(false)
for i, v in GUI.chatLines do
v:SetNeedsFrameUpdate(false)
v:Hide()
end
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for registration of OnNISBegin callback
rg -n "OnNISBegin|RegisterNIS|NIS.*[Bb]egin" lua/ui/game/ -A3 -B3

Repository: FAForever/fa

Length of output: 1279


🏁 Script executed:

#!/bin/bash
# Look for OnNISEnd or chat show/restore logic
rg -n "OnNISEnd|function.*NIS.*End" lua/ui/game/ -A10

Repository: FAForever/fa

Length of output: 38


🏁 Script executed:

#!/bin/bash
# Check the broader context around OnNISBegin function
rg -n "function OnNISBegin" lua/ui/game/chat.lua -A20 -B5

Repository: FAForever/fa

Length of output: 433


🏁 Script executed:

#!/bin/bash
# Search for OnNISEnd more broadly across entire repo
rg -n "OnNISEnd|state == 'off'" lua/ -A5 -B2

Repository: FAForever/fa

Length of output: 619


🏁 Script executed:

#!/bin/bash
# Check gamemain.lua for NIS state handling when turning off
rg -n "NISActive|state == 'off'" lua/ui/game/gamemain.lua -A8 -B2

Repository: FAForever/fa

Length of output: 1690


🏁 Script executed:

#!/bin/bash
# Get the complete NISMode function to see the 'off' case
sed -n '935,1000p' lua/ui/game/gamemain.lua

Repository: FAForever/fa

Length of output: 2838


🏁 Script executed:

#!/bin/bash
# Check if OnNISBegin in chat.lua has any guards for uninitialized GUI
sed -n '1565,1574p' lua/ui/game/chat.lua

Repository: FAForever/fa

Length of output: 291


🏁 Script executed:

#!/bin/bash
# Check when chat is initialized and if there's a guard needed
rg -n "function CreateChat|GUI = |GUI\.bg = " lua/ui/game/chat.lua -B2 -A3

Repository: FAForever/fa

Length of output: 1433


🏁 Script executed:

#!/bin/bash
# Check when CreateChat is called - look for calls to it
rg -n "CreateChat\(\)" lua/ -B3 -A1

Repository: FAForever/fa

Length of output: 530


🏁 Script executed:

#!/bin/bash
# Check if there's a chat restoration or Show logic when NIS ends
rg -n "GUI.bg:Show|GUI.chatEdit.*Show|RestoreChat" lua/ui/game/chat.lua

Repository: FAForever/fa

Length of output: 149


🏁 Script executed:

#!/bin/bash
# Check how other modules handle NIS restoration (saveload and options OnNISBegin)
rg -n "function OnNISBegin" lua/ui/dialogs/ -A15

Repository: FAForever/fa

Length of output: 727


🏁 Script executed:

#!/bin/bash
# Check if PostNIS in multifunction handles chat restoration
rg -n "function PostNIS" lua/ui/game/multifunction.lua -A20

Repository: FAForever/fa

Length of output: 771


🏁 Script executed:

#!/bin/bash
# Check what happens in HideGameUI when called with 'off' - does it affect chat?
sed -n '867,900p' lua/ui/game/gamemain.lua

Repository: FAForever/fa

Length of output: 1754


🏁 Script executed:

#!/bin/bash
# Search for any mechanism that might auto-show hidden UI elements
rg -n "Show\(\)|SetNeedsFrameUpdate.*true" lua/ui/game/chat.lua | head -20

Repository: FAForever/fa

Length of output: 468


🏁 Script executed:

#!/bin/bash
# Check what function contains the Show() calls around line 893-901
sed -n '880,910p' lua/ui/game/chat.lua

Repository: FAForever/fa

Length of output: 948


🏁 Script executed:

#!/bin/bash
# Check if there's any automatic restoration logic or if chat restoration is missing
rg -n "function ReceiveChat|AddChatLine" lua/ui/game/chat.lua -A10 | head -50

Repository: FAForever/fa

Length of output: 689


🏁 Script executed:

#!/bin/bash
# Check if chat module has OnNISEnd or any restoration hook
rg -n "OnNISEnd|NIS.*End|function.*NIS" lua/ui/game/chat.lua

Repository: FAForever/fa

Length of output: 82


🏁 Script executed:

#!/bin/bash
# Check what other UI modules might be responsible for restoring chat
sed -n '969,1010p' lua/ui/game/gamemain.lua

Repository: FAForever/fa

Length of output: 1815


Add OnNISEnd function to restore chat UI when NIS ends.

The OnNISBegin() function correctly hides the chat UI, but there is no corresponding restoration when NIS ends. When NISMode() is called with state == 'off' in gamemain.lua, the chat UI remains hidden because no restoration call is made. Add an OnNISEnd() function to lua/ui/game/chat.lua that shows the chat UI and re-enables frame updates, and invoke it from the NIS end handler in gamemain.lua (similar to how OnNISBegin() is called when NIS starts).

🤖 Prompt for AI Agents
In lua/ui/game/chat.lua around lines 1565-1574, add a new OnNISEnd() function
that reverses OnNISBegin(): call GUI.bg:Show(),
GUI.bg:SetNeedsFrameUpdate(true), iterate GUI.chatLines and call
v:SetNeedsFrameUpdate(true) and v:Show() for each, and restore the chat edit
widget (e.g., ensure GUI.chatEdit.edit can receive input); then update
gamemain.lua’s NISMode handler to call OnNISEnd() when state == 'off' (mirror
the existing call to OnNISBegin() when NIS starts).

@speed2CZ
Copy link
Member

I saw the message "Player unpaused the game" each time the cinematics start, but afaik thats not pausing the game. Do you know why that message is triggered in the first place?

@4z0t
Copy link
Member Author

4z0t commented Dec 25, 2025

It comes from LockInput in worldview.lua

@speed2CZ
Copy link
Member

Can that be changed? Since I would say the main issue here is that there's a message triggered when it shouldnt.
Hiding the chat during NIS mode is a good idea as well, but currently its just hiding the main issue.

@4z0t
Copy link
Member Author

4z0t commented Dec 25, 2025

I don't know what side effects could be there if I remove it

@lL1l1
Copy link
Contributor

lL1l1 commented Dec 28, 2025

I think it would be fine to add a way to force resuming the session without sending a chat message, like a param to the hooked function or a separate global. Then you get to remove the chat message without removing the resumption behavior.

@4z0t
Copy link
Member Author

4z0t commented Dec 28, 2025

I think it would be fine to add a way to force resuming the session without sending a chat message, like a param to the hooked function or a separate global. Then you get to remove the chat message without removing the resumption behavior.

Tbf I'd remove it completely from coop

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.

3 participants