-
-
Notifications
You must be signed in to change notification settings - Fork 215
Add the leave reason to :leavelogs #2005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add the leave reason to :leavelogs #2005
Conversation
WalkthroughThe PlayerRemoving handler now accepts a removal reason parameter and maps reason.Name values to human-readable strings; server Leaves log entries include that reason in the description ("left the server ()"). No other behavioral changes were made. Changes
Sequence DiagramsequenceDiagram
actor Roblox as Roblox Platform
participant Handler as PlayerRemoving Handler
participant Mapping as Reasons Mapping
participant Log as Server Log
Roblox->>Handler: PlayerRemoving(player, reason)
Handler->>Mapping: Look up reason.Name (or fallback)
Mapping-->>Handler: Human-readable reason string
Handler->>Log: Write "left the server (reason)"
Log-->>Handler: Entry recorded
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
MainModule/Server/Core/Process.luau(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{lua,luau}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{lua,luau}: Use task.wait() instead of wait()
Use interpolated strings instead of string concatenation
Files:
MainModule/Server/Core/Process.luau
🔇 Additional comments (1)
MainModule/Server/Core/Process.luau (1)
837-837: LGTM - Function signature correctly updated.The signature change properly accepts the
PlayerRemoveReasonparameter from Roblox'sPlayerRemovingevent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
MainModule/Server/Core/Process.luau (1)
861-861: Add nil safety for the exit reason parameter (duplicate concern).The current implementation lacks defensive coding: if
ris nil, accessingr.Namewill cause a runtime error. This concern was raised in previous reviews but remains unaddressed.Apply this diff to add nil safety:
- local reason = reasons[r.Name] or r.Name + local reason = "Unknown Reason" + if r and r.Name then + reason = reasons[r.Name] or r.Name + end
🧹 Nitpick comments (1)
MainModule/Server/Core/Process.luau (1)
855-859: Consider mapping additional exit reasons.The reasons table currently maps only three exit reasons (Unknown, PlatformKick, CreatorKick). According to the Roblox documentation, the PlayerExitReason enum includes additional values such as GameShutdown, LostConnection, PlayerMutuallyDisconnected, and others. While the fallback to
r.Namehandles unmapped cases, adding more human-readable mappings would improve log clarity for administrators.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
MainModule/Server/Core/Process.luau(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{lua,luau}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{lua,luau}: Use task.wait() instead of wait()
Use interpolated strings instead of string concatenation
Files:
MainModule/Server/Core/Process.luau
🔇 Additional comments (1)
MainModule/Server/Core/Process.luau (1)
837-837: Approved: Function signature correctly updated.The addition of the
rparameter properly captures the PlayerExitReason from Roblox's PlayerRemoving event.
Modified the description of each log in :leavelogs to show the Roblox reason they have left the game
Summary by CodeRabbit