v5.13.2 - SLP fix and discord improvements#161
Conversation
… discord status panel
…ched processes. Fixes #159
There was a problem hiding this comment.
Pull request overview
Release v5.13.2 improvements across SLP management, Discord status panel UX, and SSUI self-update/self-restart behavior to prevent detached/duplicate game server processes (fixes #159).
Changes:
- Add SLP “Reinstall” capability that preserves mods/modconfig while reinstalling the SLP plugin.
- Improve Discord status panel with buttons for game version and next auto-restart time, backed by runtime tracking of the next restart timestamp.
- Stop the game server before SSUI update/self-restart to avoid leaving detached game server processes.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/web/slp-launchpad.go | Adds API handler for SLP reinstall. |
| src/web/routes.go | Registers /api/v2/slp/reinstall route. |
| src/modding/launchpad.go | Implements ReinstallSLP() preserving mods/modconfig. |
| src/managers/gamemgr/autorestart.go | Tracks next auto-restart time in config at runtime. |
| src/discordbot/serverstatuspanel.go | Adds Discord buttons + handlers for game version and next restart. |
| src/config/vars.go | Adds runtime NextAutoRestartTime. |
| src/config/setters.go | Adds setter for NextAutoRestartTime. |
| src/config/getters.go | Adds getter for NextAutoRestartTime. |
| src/setup/update/updater.go | Stops game server prior to applying SSUI update. |
| src/setup/update/runandexit.go | Stops game server prior to SSUI self-restart. |
| src/config/config.go | Bumps version to 5.13.2. |
| UIMod/onboard_bundled/ui/config.html | Adds “Reinstall SLP” UI button/description. |
| UIMod/onboard_bundled/assets/js/slp.js | Adds reinstallSLP() frontend action. |
| UIMod/onboard_bundled/assets/css/config.css | Styles new SLP manage “center” column. |
Comments suppressed due to low confidence (1)
src/managers/gamemgr/autorestart.go:60
NextAutoRestartTimeis set once for the minutes-based schedule, but it isn't updated after each restart. Also, when the goroutine exits early because the server is not running (theinternalIsServerRunningNoLock()check), the value is left populated instead of being cleared. Consider updatingNextAutoRestartTimeon each successful tick (e.g., now+interval) and clearing it on all exit paths (including "server not running").
config.SetNextAutoRestartTime(time.Now().Add(time.Duration(minutesInt) * time.Minute))
ticker := time.NewTicker(time.Duration(minutesInt) * time.Minute)
defer ticker.Stop()
for {
select {
case <-ticker.C:
mu.Lock()
if !internalIsServerRunningNoLock() {
mu.Unlock()
logger.Core.Info("Auto-restart skipped: server is not running")
return
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| } else { | ||
| logger.Install.Info("SLP not currently installed; proceeding with fresh install") |
There was a problem hiding this comment.
os.Stat errors other than "not exists" (e.g., permission issues) are currently treated as "SLP not installed" and the reinstall proceeds. That can mask real filesystem problems and lead to confusing failures later. Consider handling os.IsNotExist(err) explicitly and returning an error for other Stat failures.
| } else { | |
| logger.Install.Info("SLP not currently installed; proceeding with fresh install") | |
| } else if os.IsNotExist(err) { | |
| logger.Install.Info("SLP not currently installed; proceeding with fresh install") | |
| } else { | |
| return "", fmt.Errorf("failed to check existing SLP installation: %w", err) |
| @@ -146,7 +155,19 @@ func scheduleDailyRestart(t time.Time, done chan struct{}) { | |||
| } | |||
There was a problem hiding this comment.
For the daily schedule, NextAutoRestartTime is updated when skipping/not-stopping, but it isn't updated after a successful restart. That means the stored "next restart" can become stale (in the past) until the next loop iteration recalculates it. Consider calling setNextDailyRestartTime(t) immediately after a successful restart (or before starting the timer each loop) so the Discord panel always shows the upcoming restart time.
| } | |
| } | |
| // Schedule next day after a successful restart | |
| setNextDailyRestartTime(t) |
| buttons = append(buttons, discordgo.Button{ | ||
| Label: "🎮 Get Game Version", | ||
| Style: discordgo.SecondaryButton, | ||
| CustomID: ButtonGetGameVersion, | ||
| }) |
There was a problem hiding this comment.
The PR description says the 🎮 Game Version button "only appears once a version is detected", but the button is currently always added. Either gate this button on config.GetExtractedGameVersion() != "" (and/or a dedicated "version detected" flag) or update the release notes/description to match the actual behavior.
| // Stop the game server before launching the new version to prevent detached processes | ||
| if config.GetIsGameServerRunning() { | ||
| logger.Install.Info("🛑 Stopping game server before applying update...") | ||
| if err := gamemgr.InternalStopServer(); err != nil { | ||
| logger.Install.Warn(fmt.Sprintf("⚠️ Failed to stop game server before update: %v. Proceeding anyway.", err)) | ||
| } | ||
| } |
There was a problem hiding this comment.
If InternalStopServer() fails here, the updater continues and launches the new binary anyway. That can leave the old server process running and recreate the "detached/duplicate server" situation this change is trying to prevent. Consider attempting a stop unconditionally (don't rely solely on config.GetIsGameServerRunning()), and abort the update if stopping fails for reasons other than "server not running".
| if config.GetIsGameServerRunning() { | ||
| logger.Install.Info("🛑 Stopping game server before restart...") | ||
| if err := gamemgr.InternalStopServer(); err != nil { | ||
| logger.Install.Warn(fmt.Sprintf("⚠️ Failed to stop game server before restart: %v. Proceeding anyway.", err)) |
There was a problem hiding this comment.
Same as the update flow: this restart path logs and proceeds even if InternalStopServer() fails. If the stop fails while the server is still running, restarting SSUI can leave the game server detached and/or running alongside a new instance. Consider aborting the restart on stop failure (except when the server is already stopped), or performing a stronger termination before proceeding.
| if config.GetIsGameServerRunning() { | |
| logger.Install.Info("🛑 Stopping game server before restart...") | |
| if err := gamemgr.InternalStopServer(); err != nil { | |
| logger.Install.Warn(fmt.Sprintf("⚠️ Failed to stop game server before restart: %v. Proceeding anyway.", err)) | |
| wasRunning := config.GetIsGameServerRunning() | |
| if wasRunning { | |
| logger.Install.Info("🛑 Stopping game server before restart...") | |
| if err := gamemgr.InternalStopServer(); err != nil { | |
| // Re-check running state: if still running, abort restart to avoid detached/duplicate server | |
| if config.GetIsGameServerRunning() { | |
| logger.Install.Error(fmt.Sprintf("⚠️ Failed to stop game server before restart and server still appears to be running: %v. Aborting restart.", err)) | |
| return | |
| } | |
| logger.Install.Warn(fmt.Sprintf("⚠️ Failed to stop game server before restart: %v, but server no longer appears to be running. Proceeding with restart.", err)) |
| fetch('/api/v2/slp/reinstall') | ||
| .then(response => response.json()) |
There was a problem hiding this comment.
The UI calls /api/v2/slp/reinstall with a default GET. The release notes (and typical HTTP semantics) suggest this should be a POST since it changes server state. Consider changing this fetch to method: 'POST' (and handling non-2xx responses), in sync with the backend method enforcement.
| fetch('/api/v2/slp/reinstall') | |
| .then(response => response.json()) | |
| fetch('/api/v2/slp/reinstall', { method: 'POST' }) | |
| .then(async response => { | |
| let data = null; | |
| try { | |
| data = await response.json(); | |
| } catch (e) { | |
| // Ignore JSON parse errors; will fall back to a generic message below if needed. | |
| } | |
| if (!response.ok) { | |
| const message = | |
| (data && data.error) || | |
| ('Request failed with status ' + response.status + ' ' + (response.statusText || '')); | |
| throw new Error(message); | |
| } | |
| return data || {}; | |
| }) |
| w.Write([]byte(`{"success": true}`)) | ||
| } | ||
|
|
||
| func ReinstallSLPHandler(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
This handler performs a destructive action (reinstall) but doesn't enforce an HTTP method. The release notes say this endpoint is a POST; currently it will also run on GET, which makes it easier to trigger unintentionally (and can increase CSRF risk depending on auth). Consider requiring POST (return 405 otherwise) and keep the UI in sync with that method.
| func ReinstallSLPHandler(w http.ResponseWriter, r *http.Request) { | |
| func ReinstallSLPHandler(w http.ResponseWriter, r *http.Request) { | |
| if r.Method != http.MethodPost { | |
| w.Header().Set("Content-Type", "application/json") | |
| w.WriteHeader(http.StatusMethodNotAllowed) | |
| json.NewEncoder(w).Encode(map[string]interface{}{ | |
| "success": false, | |
| "error": "method not allowed, use POST", | |
| }) | |
| return | |
| } |
v5.13.2 Release Notes
Added 🎮 Game Version button to the server status panel - shows the detected game version as an ephemeral message (button only appears once a version is detected)
Added 🔄 Next Auto Restart button -shows the next scheduled restart time in the user's local timezone using Discord's native timestamp formatting, including a live countdown (button only appears when auto-restart is enabled)
Auto-restart schedule now tracks the next restart time at runtime; the value is cleared when the server stops or auto-restart is cancelled
Bug Fix: Detached Game Server on SSUI Update/Restart ( Fixes #159)
Applying an SSUI update while the game server was running caused the server process to become detached — SSUI would lose track of it and start a second instance on the next "Start Server" command
SSUI now gracefully stops the game server before launching the new executable, on both Windows and Linux
Same fix applies to RestartMySelf() used during the post-update self-restart
SLP: Reinstall without losing mods
Added a Reinstall SLP button in the SLP management panel
Reinstall removes only the SLP plugin directory, preserving all installed mods and modconfig.xml, then downloads and installs the latest SLP version
Available via POST /api/v2/slp/reinstall and UI -> config -> SLP