Restore console title and startup beep lost in United cores - #466
Merged
Conversation
PR #463 (a683a24, 1039 files, no reviews) removed two admin-visible console behaviours without mentioning either, and left a config key renamed on the code side only. This restores all three. Console title: the live "N Players - N Connections" window title added in #337 was relocated intact by #448, then deleted outright by #463. Restored into Master::PublishConsoleStatus(), which already runs on a one-second cadence, placed above that function's early-out because the window title is owned by the OS rather than by the console frame and must stay current even when the full-screen UI is off. Startup beep: #463 gated the existing beep behind !ConsoleUI::Active(), and the full-screen UI is on by default, so it never fired interactively. Removing the gate alone is not sufficient: ConsoleLogWriter::Emit() routes a raw record to ConsoleUI::PushRaw() as text when the UI is active, so a BEL sent through sLog is displayed rather than sounded. The bell now bypasses the log pipeline entirely. Config keys: #448 shipped Console.Style correctly on both sides; #463 replaced the reader with a bool Console.FullScreen and left the conf untouched, so Console.Style became dead, Console.FullScreen existed in no config file, and there was no shipped way to disable the UI. Console.Style is restored as the reader and its doc block now states honestly that "fancy" behaves as "auto". Separately, the conf still said BeepAtStart while the code read Console.BeepAtStart; the conf now matches. Verified: builds clean with 0 errors; mangosd boots against a real 1.12.1 dataset and the title was observed live as "Mangos Zero (0 Players - 0 Connections)"; the beep was confirmed audible on the rebuilt binary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 4 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Restores two admin-visible console behaviours that #463 ("United cores", 1039 files, merged with no reviews) removed without mentioning either, plus the config key it renamed on only one side.
Windows console title
The live
N Players - N Connectionswindow title added deliberately in #337 was relocated intact by #448 intoMaster.cpp, then deleted outright by #463 with no replacement.Restored into
Master::PublishConsoleStatus(), which already runs on a one-second cadence, rather than reintroducing a separate tick counter. It sits above that function'sif (!ui.Active()) return;early-out on purpose: the window/taskbar title is owned by the OS, not by the console frame, so it must stay current even when the full-screen UI is switched off — and it stays readable while the server is minimised, which the in-frame status bar does not.Startup completion beep
#463 gated the existing beep behind
!ConsoleUI::Instance().Active(). The full-screen UI is on by default, so the beep never fired interactively.Removing that gate alone is not sufficient, and this was verified rather than assumed.
ConsoleLogWriter::Emit()(src/shared/Log/ConsoleLogWriter.cpp:158-174) checksConsoleUI::Active()first and routes a raw record toConsoleUI::PushRaw()as text, so a BEL sent throughsLog.ConsoleEmitRaw()lands in the scrollback buffer and is displayed rather than sounded. The bell therefore has to bypass the log pipeline entirely:MessageBeep(MB_OK)on Windows, directfputs("\a")+fflushon POSIX.Config keys
Console.Style. ACE removed. New network engine. OpenSSL 3.x mandatory. New mangosd UI. #448 shipped this correctly on both sides — a documented tri-state (auto/fancy/plain) with a reader. United cores #463 unilaterally replaced the reader with a boolConsole.FullScreenand left the conf untouched. Result:Console.Stylewas read by nothing,Console.FullScreenappeared in no config file, and there was no shipped way to turn the full-screen UI off — which matters, because that UI is what suppressed the beep and hosted the deleted title. RestoredConsole.Styleas the reader rather than deleting the documented key. Its doc block now states honestly that"fancy"currently behaves as"auto", since forcing the UI on for an undetectable terminal is still not implemented.BeepAtStart. The code readsConsole.BeepAtStart; the shipped conf still said plainBeepAtStart, so the admin's setting was dead. Conf now matches the code.Verification
RelWithDebInfo, MSVC).MessageBeeplinks without adding a library —user32.libcomes from CMake's MSVC default standard libraries.Mangos Zero (0 Players - 0 Connections). Beep confirmed audible on the rebuilt binary.git diff --checkclean.Notes for review
UpdateConsoleTitleuses a function-localstatic std::string.PublishConsoleStatushas exactly one caller, inWorldLoop, so this is world-thread-only and not a data race.GetOpenConnectionCount()is a relaxed atomic load; the listener starts before the world loop and stops only after it exits, so it is never read against a torn-down network layer.Console.FullScreen=0loses that override. It appears in no shipped config file, so this should affect nobody.Master.cppgains#include "SystemConfig.h"forMANGOS_PACKAGENAME;mangosd.cppalready included it.A wider audit of #448/#463 turned up further config drift not addressed here — including two with security and audit impact (the
Ra.*/RA.*mismatch leavesRa.Strictedsilently ignored, andGmLogFile/GMLogFilemeans the GM command audit log is never written). Those are deliberately left out to keep this PR single-purpose and will follow separately.This change is