Skip to content

Tabbed online accounts with character and party columns - #952

Merged
sven-n merged 1 commit into
MUnique:masterfrom
eduardosmaniotto:feature/logged-in-tabs
Sep 21, 2026
Merged

sven-n merged 1 commit into
MUnique:masterfrom
eduardosmaniotto:feature/logged-in-tabs

Conversation

@eduardosmaniotto

Copy link
Copy Markdown
Contributor

Reworks the /logged-in (Online accounts) page:

  • Three tabs under the existing header: Players (default), Off-level Players (only when the /offlevel command plugin is active), Bot Players (only when the bot feature plugin is active).
  • New Character column on all tabs. Resolves via in-process game contexts; falls back to in distributed deployments.
  • New Party column: badge showing party master + member count, with a deterministic per-party color so members are visually grouped. Ordering is partied-first → master → account → character, so party members stay contiguous.
  • Bots tab is read-only (no Stop button: bots are tracked inside the private BotManager, so DisconnectAccountAsync cannot stop them cleanly — that would leave stale entries behind).

How

  • LoggedInAccount / OfflineAccount extended with CharacterName, PartyMaster, PartySize; new BotAccount + BotAccountService following the existing IDataService<T> pattern.
  • Shared OnlineAccountOrdering.OrderPartyGrouped<T>() over IPartyGroupedAccount instead of triplicated sort logic.
  • Shared PartyBadge.razor component instead of string-built HTML.
  • Cleanup: short type names, named PlayerInfo record instead of tuple, dead bot filter in OfflineAccountService replaced with an explanatory comment.
players-tab offlevel-players bot-players

…lumns

Split the logged-in page into Players / Off-level Players / Bot Players
tabs, gated on the offlevel and bot plugin states. Add selected-character
and party columns with party-grouped ordering shared across all three
tables, and cover the ordering and distributed fallbacks with tests.

sven-n commented Sep 21, 2026

Copy link
Copy Markdown
Member

Review

Overall this is a nice cleanup: the shared OrderPartyGrouped<T>() over IPartyGroupedAccount removes the triplicated sort, PartyBadge.razor replaces string-built HTML, and the new tests cover both the ordering rules and the distributed-deployment fallback (no in-process servers → no character, tabs hidden). I verified the assumptions the change rests on and they hold:

  • BotPlayer : OfflinePlayer, so the player is OfflinePlayer guard in GetPlayerLookupAsync does exclude bots too — the comment is accurate.
  • Party.PartyList is IReadOnlyList<IPartyMember> backed by an exactly-sized array (not a fixed MaxPartySize array), so PartyList.Count really is the member count, and reading it from the web thread is safe because the array is swapped, not mutated in place.
  • Bots do run through OfflinePlayer.InitializeAsync (BotManager line ~82), so StartTimestamp is a real UTC value on the bots tab.
  • Bots are never registered at the login server and never enter OfflinePlayerManager, so there is no double-listing across tabs.

Findings below, roughly by importance.

1. Off-level sessions become unreachable when the plugin is deactivated

_showOfflevel gates the whole tab on IsOfflevelFeatureAvailable(), i.e. on OfflineLevelingChatCommandPlugIn being active. But deactivating that plug-in only removes the /offlevel command — it does not stop sessions that are already running (OfflinePlayerManager has no deactivation hook). In that state the admin panel no longer lists them at all, and with the tab gone the Stop button is unreachable, which is exactly when an admin would want it. Before this PR the table was always rendered.

Suggestion: show the tab when the feature is available or when there is at least one active offline player.

2. PartyBadge colors are not actually deterministic

var hash = System.HashCode.Combine(this.PartyMaster!.ToUpperInvariant());

string.GetHashCode() (and therefore HashCode.Combine) is randomized per process in .NET Core. The color is stable within one admin-panel process, but changes on every restart, and differs between nodes if the panel is ever scaled out. If "deterministic per party master" is the intent, a small stable hash (e.g. FNV-1a over the uppercased chars) gets you that. Minor extras on the same line: HashCode.Combine(x) with a single argument is just x.GetHashCode(), ToUpperInvariant() allocates a string per row per render, and System.HashCode is fully qualified although short type names were one of the stated cleanups.

3. Full player-list copy per page load

LoggedInAccountService.GetAsync now calls GetPlayersAsync() on every in-process game server; that takes a reader lock and does a ToList() of the whole player list, on every page load and on every DataChanged refresh, just to render 20 rows. Same shape in BotAccountService.GetAsync. Building the full set before ordering is unavoidable if you want global party-grouped ordering with correct paging, so this is a trade-off rather than a bug — but on a populated server it is worth being aware of, and a short-lived cache of the lookup would take the sting out of the refresh path.

4. Tab dispatch falls through to Bots

The final else in LoggedIn.razor renders the bots table for any value that isn't Players or Offlevel. It is correct today, but an explicit else if (_activeTab == Tab.Bots) (plus && _showBots) keeps a future fourth tab from silently rendering bots.

5. IPartyGroupedAccount omits PartySize

All three implementers have it and PartyBadge needs it, so adding int PartySize { get; } to the interface would round it out. Related edge case: if Party is set but Party.PartyMaster is momentarily null, the row sorts as solo and shows even though PartySize > 0.

6. Smaller things

  • Resources.ActiveOfflinePlayer is now unused — its only consumer was the removed <h2>.
  • Hard-coded appears in four places, and title="Party of @PartyMaster: @PartySize members" is not localized while everything around it goes through Resources.
  • Accessibility: the tab buttons carry role="tab"/aria-selected but no id/aria-controls, and the panels have role="tabpanel" without aria-labelledby.
  • GetPlayerLookupAsync: ContainsKey followed by the indexer is a double lookup — TryAdd does it in one. The player is OfflinePlayer filter is also effectively defensive only, since OfflinePlayerManager.LogOffFromLoginServerAsync removes those accounts from the login-server snapshot; worth saying so in the comment.
  • New @code members mix _activeTab with this._showOfflevel; the surrounding file consistently uses the this. prefix.
  • The new entries in Resources.Designer.cs were inserted mid-file while the .resx entries were appended at the end, so the next regeneration will churn the diff. (The duplicate-BOM fix at the top of that file is a nice catch.)

CI is green on 8146959.


Generated by Claude Code

@sven-n
sven-n merged commit 215bc08 into MUnique:master Sep 21, 2026
2 checks passed
@eduardosmaniotto
eduardosmaniotto deleted the feature/logged-in-tabs branch September 21, 2026 23:07
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