Skip to content

Commit 997cfea

Browse files
Fixes lobby team preview: clan players aren't assigned a team + add nation count + other small fixes (#2536)
## Description: Fixes for v28. In #2444, lobby team preview was added. But for players with clan tags, this doesn't work correctly. They don't get assigned to the same team in the preview, while they do get assigned to the same team in the actual game. Also added some small fixes and QoL improvements like showing the number of Nations next to the number of Players. Since we needed that info anyway. Did not choose to show and assign Nations to teams (just the numbers), see why under CONSIDERED OPTIONS THAT I DIDN'T WENT WITH. **BEFORE:** https://youtu.be/AV_aDJ4PgOk <img width="767" height="117" alt="Malformed argument because of double accolades for remove_player" src="https://github.com/user-attachments/assets/7de1114e-7ce1-4a8f-95cc-6b0528a61e3b" /> **AFTER:** https://youtu.be/aDCKkwedqes Cause of bug: maxTeamSize is a number in assignTeams, only used to assign clan players. It uses the length of the players array as input. At actual game start, the Nations are also in the players array. But at lobby preview the Nations aren't yet fetched. So when 2 players want to do a 3 Teams private lobby with them using clan tags to be in the same team. maxTeamCount would do Math.ceil(2/3)=1. So only 1 clan player per team as a result. While actually there could be 10 Nations which would result in maxTeamCount Math.ceil(12/3)=4 so in the game they would actually be assigned to the same team. Fix for bug: fetch Nations count in HostLobbyModal and pass on to LobbyTeamView. Add it to the number of players for maxTeamCount that assignTeamsLobbyPreview uses for its calculation. Also added nation count to the similar teamMaxSize in LobbyTeamView itself, to display the same and correct number of max players. For random maps, we now need to know the random map before the game starts to get its Nation count. So made some changes for that too in HostLobbyModal. Also fixed: - willUpdate ran comptuteTeamPreview every second, now checks if properties like 'client' actually changed. PollPlayers in HostLobbyModal 'changes' the clients property every second even if there are no actual changes. Checks if the other properties are actually changed too, to make it more future proof. - cache teamsList so it is only fetched once instead of first in computeTeamPreview and then again for showTeamColors. - don't show the "Empty Teams" header if there are no empty teams. - console error ICU format error SyntaxError: MALFORMED ARGUMENT. Because of double accolades around remove_player in translation value. - remove fallback for comparing clients on clientID, which used client name. Players may have the same names so it's not safe to compare based on name. Also show number of Nations next to number of Players: now we now the nationCount since it is needed for the fix, show number of Nations next to number of Players. It's handy and it prevents confusion as to why it says 2/32 for two teams if there are only 2 players; it is because there are 61 Nations as well on the World map for example. Also determine number of teams based on Players + Nations: now we now the nationCount since it is needed for the fix, use it to determine the number of teams. Just like populateTeams in GameImpl does. This means for Duos on the World map, a minimum of 31 teams will be shown since there are 61 Nations. This is better than just show two teams based on 1 or 2 humans in the lobby. Also it makes more clear how many teams there'll be the game and how the players and nations are divided over the teams. Choose to not show the Nations' team assignments though. That could be for another PR. See explanation under CONSIDERED OPTIONS THAT I DIDN'T WENT WITH. Also show Nations team as pre-filled for HumansVSNations: now we now the nationCount since it is needed for the fix, for HumansVSNations, show the Nations team as fully assigned and non-empty. For example for World map it shows Nations 61/61. Don't show them as Empty Team but as Assigned Team. Although i choose not to show the actual Nations (see CONSIDERED OPTIONS THAT I DIDN'T WENT WITH), this makes it clear their team is pre-filled and how many Nations you're actually up against. Whereas for other Team game types like 7 Teams or Duos, it will display the teams that the Nations will fill up as empty. CONSIDERED OPTIONS THAT I DIDN'T WENT WITH - Use an optional param 'nationsCount' to assignTeams with default = 0. And simply add nationsCount to the players.length count for maxTeamSize. This would be error prone; 'nationsCount' should then never be assigned a value except when called from LobbyTeamView. But in the future someone might assign it a value even when called from somewhere else. Then you could say, check if there are Nations in the players array and if so, do not use Nationscount because we know they are already counted from players.length. But if Disable Nations is enabled, there would be no Nations in the players array but if nationsCount was somehow given a value we still should not use it. So again, too developer error prone. - Not only fetch the number of Nations, but actually get the Nations themselves to show them in the team assign preview as well. They are shuffled on team assignment but of course deterministicly (nation player ID assigned based on Pseudorandom seeded with GameID in GameRunner, then shuffled in TeamAssignment with Pseudorandom seeded with map's first Nation's playerID), so we could replicate it. But then, how to distinguish humans and Nations in the UI? This feels like something for a follow-up PR. FOR A FUTURE PR - change the way Clan team overflow is handled. Now they're "kicked" as in not assigned to a team without their knowing, are loaded into the game but cannot spawn. This UX could need some improvement. And the logic can be improved too, ie. don't "kick" too soon, check the number of Clans in the lobby and the number of teams to decide if we can assign the 'overflowing' Clan player to another team that doesn't have rivalling Clan players. Far out of scope for this PR. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced ## Please put your Discord username so you can be contacted if a bug or regression is found: tryout33
1 parent 96cf177 commit 997cfea

File tree

4 files changed

+85
-28
lines changed

4 files changed

+85
-28
lines changed

resources/lang/en.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,14 +275,16 @@
275275
"enables_title": "Enable Settings",
276276
"player": "Player",
277277
"players": "Players",
278+
"nation_players": "Nations",
279+
"nation_player": "Nation",
278280
"waiting": "Waiting for players...",
279281
"random_spawn": "Random spawn",
280282
"start": "Start Game",
281283
"host_badge": "Host",
282284
"assigned_teams": "Assigned Teams",
283285
"empty_teams": "Empty Teams",
284286
"empty_team": "Empty",
285-
"remove_player": "Remove {{username}}"
287+
"remove_player": "Remove {username}"
286288
},
287289
"team_colors": {
288290
"red": "Red",

src/client/HostLobbyModal.ts

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ import "./components/Difficulties";
2828
import "./components/LobbyTeamView";
2929
import "./components/Maps";
3030
import { JoinLobbyEvent } from "./Main";
31+
import { terrainMapFileLoader } from "./TerrainMapFileLoader";
3132
import { renderUnitTypeOptions } from "./utilities/RenderUnitTypeOptions";
32-
3333
@customElement("host-lobby-modal")
3434
export class HostLobbyModal extends LitElement {
3535
@query("o-modal") private modalEl!: HTMLElement & {
@@ -58,11 +58,13 @@ export class HostLobbyModal extends LitElement {
5858
@state() private disabledUnits: UnitType[] = [];
5959
@state() private lobbyCreatorClientID: string = "";
6060
@state() private lobbyIdVisible: boolean = true;
61+
@state() private nationCount: number = 0;
6162

6263
private playersInterval: NodeJS.Timeout | null = null;
6364
// Add a new timer for debouncing bot changes
6465
private botsUpdateTimer: number | null = null;
6566
private userSettings: UserSettings = new UserSettings();
67+
private mapLoader = terrainMapFileLoader;
6668

6769
connectedCallback() {
6870
super.connectedCallback();
@@ -553,13 +555,21 @@ export class HostLobbyModal extends LitElement {
553555
? translateText("host_modal.player")
554556
: translateText("host_modal.players")
555557
}
558+
<span style="margin: 0 8px;"></span>
559+
${this.nationCount}
560+
${
561+
this.nationCount === 1
562+
? translateText("host_modal.nation_player")
563+
: translateText("host_modal.nation_players")
564+
}
556565
</div>
557566
558567
<lobby-team-view
559568
.gameMode=${this.gameMode}
560569
.clients=${this.clients}
561570
.lobbyCreatorClientID=${this.lobbyCreatorClientID}
562571
.teamCount=${this.teamCount}
572+
.nationCount=${this.disableNPCs ? 0 : this.nationCount}
563573
.onKickPlayer=${(clientID: string) => this.kickPlayer(clientID)}
564574
></lobby-team-view>
565575
</div>
@@ -613,6 +623,7 @@ export class HostLobbyModal extends LitElement {
613623
});
614624
this.modalEl?.open();
615625
this.playersInterval = setInterval(() => this.pollPlayers(), 1000);
626+
this.loadNationCount();
616627
}
617628

618629
public close() {
@@ -631,12 +642,15 @@ export class HostLobbyModal extends LitElement {
631642

632643
private async handleRandomMapToggle() {
633644
this.useRandomMap = true;
645+
this.selectedMap = this.getRandomMap();
646+
await this.loadNationCount();
634647
this.putGameConfig();
635648
}
636649

637650
private async handleMapSelection(value: GameMapType) {
638651
this.selectedMap = value;
639652
this.useRandomMap = false;
653+
await this.loadNationCount();
640654
this.putGameConfig();
641655
}
642656

@@ -794,10 +808,6 @@ export class HostLobbyModal extends LitElement {
794808
}
795809

796810
private async startGame() {
797-
if (this.useRandomMap) {
798-
this.selectedMap = this.getRandomMap();
799-
}
800-
801811
await this.putGameConfig();
802812
console.log(
803813
`Starting private game with map: ${GameMapType[this.selectedMap as keyof typeof GameMapType]} ${this.useRandomMap ? " (Randomly selected)" : ""}`,
@@ -857,6 +867,17 @@ export class HostLobbyModal extends LitElement {
857867
}),
858868
);
859869
}
870+
871+
private async loadNationCount() {
872+
try {
873+
const mapData = this.mapLoader.getMapData(this.selectedMap);
874+
const manifest = await mapData.manifest();
875+
this.nationCount = manifest.nations.length;
876+
} catch (error) {
877+
console.warn("Failed to load nation count", error);
878+
this.nationCount = 0;
879+
}
880+
}
860881
}
861882

862883
async function createLobby(creatorClientID: string): Promise<GameInfo> {

src/client/components/LobbyTeamView.ts

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
Team,
1414
Trios,
1515
} from "../../core/game/Game";
16-
import { assignTeams } from "../../core/game/TeamAssignment";
16+
import { assignTeamsLobbyPreview } from "../../core/game/TeamAssignment";
1717
import { ClientInfo, TeamCountConfig } from "../../core/Schemas";
1818
import { translateText } from "../Utils";
1919

@@ -31,19 +31,23 @@ export class LobbyTeamView extends LitElement {
3131
@property({ type: String }) lobbyCreatorClientID: string = "";
3232
@property({ attribute: "team-count" }) teamCount: TeamCountConfig = 2;
3333
@property({ type: Function }) onKickPlayer?: (clientID: string) => void;
34+
@property({ type: Number }) nationCount: number = 0;
3435

3536
private theme: PastelTheme = new PastelTheme();
3637
@state() private showTeamColors: boolean = false;
3738

3839
willUpdate(changedProperties: Map<string, any>) {
3940
// Recompute team preview when relevant properties change
41+
// clients is 'changed' every 1s from pollPlayers, chose to not compare for actual change
4042
if (
4143
changedProperties.has("gameMode") ||
4244
changedProperties.has("clients") ||
43-
changedProperties.has("teamCount")
45+
changedProperties.has("teamCount") ||
46+
changedProperties.has("nationCount")
4447
) {
45-
this.computeTeamPreview();
46-
this.showTeamColors = this.getTeamList().length <= 7;
48+
const teamsList = this.getTeamList();
49+
this.computeTeamPreview(teamsList);
50+
this.showTeamColors = teamsList.length <= 7;
4751
}
4852
}
4953

@@ -60,8 +64,12 @@ export class LobbyTeamView extends LitElement {
6064
}
6165

6266
private renderTeamMode() {
63-
const active = this.teamPreview.filter((t) => t.players.length > 0);
64-
const empty = this.teamPreview.filter((t) => t.players.length === 0);
67+
const active = this.teamPreview.filter(
68+
(t) => t.players.length > 0 || t.team === ColoredTeams.Nations,
69+
);
70+
const empty = this.teamPreview.filter(
71+
(t) => t.players.length === 0 && t.team !== ColoredTeams.Nations,
72+
);
6573
return html` <div
6674
class="flex flex-col md:flex-row gap-3 md:gap-4 items-stretch max-h-[65vh]"
6775
>
@@ -96,9 +104,11 @@ export class LobbyTeamView extends LitElement {
96104
</div>
97105
</div>
98106
<div>
99-
<div class="font-semibold text-gray-200 mb-1 text-sm">
100-
${translateText("host_modal.empty_teams")}
101-
</div>
107+
${empty.length > 0
108+
? html`<div class="font-semibold text-gray-200 mb-1 text-sm">
109+
${translateText("host_modal.empty_teams")}
110+
</div>`
111+
: ""}
102112
<div class="w-full grid grid-cols-1 sm:grid-cols-2 gap-2 md:gap-3">
103113
${repeat(
104114
empty,
@@ -136,6 +146,16 @@ export class LobbyTeamView extends LitElement {
136146
}
137147

138148
private renderTeamCard(preview: TeamPreviewData, isEmpty: boolean = false) {
149+
const displayCount =
150+
preview.team === ColoredTeams.Nations
151+
? this.nationCount
152+
: preview.players.length;
153+
154+
const maxTeamSize =
155+
preview.team === ColoredTeams.Nations
156+
? this.nationCount
157+
: this.teamMaxSize;
158+
139159
return html`
140160
<div class="bg-gray-800 border border-gray-700 rounded-xl flex flex-col">
141161
<div
@@ -148,9 +168,7 @@ export class LobbyTeamView extends LitElement {
148168
></span>`
149169
: null}
150170
<span class="truncate">${preview.team}</span>
151-
<span class="text-white/90"
152-
>${preview.players.length}/${this.teamMaxSize}</span
153-
>
171+
<span class="text-white/90">${displayCount}/${maxTeamSize}</span>
154172
</div>
155173
<div class="p-2 ${isEmpty ? "" : "flex flex-col gap-1.5"}">
156174
${isEmpty
@@ -190,7 +208,7 @@ export class LobbyTeamView extends LitElement {
190208

191209
private getTeamList(): Team[] {
192210
if (this.gameMode !== GameMode.Team) return [];
193-
const playerCount = this.clients.length;
211+
const playerCount = this.clients.length + this.nationCount;
194212
const config = this.teamCount;
195213

196214
if (config === HumansVsNations) {
@@ -230,13 +248,12 @@ export class LobbyTeamView extends LitElement {
230248
}
231249
}
232250

233-
private computeTeamPreview() {
251+
private computeTeamPreview(teams: Team[] = []) {
234252
if (this.gameMode !== GameMode.Team) {
235253
this.teamPreview = [];
236254
this.teamMaxSize = 0;
237255
return;
238256
}
239-
const teams = this.getTeamList();
240257

241258
// HumansVsNations: show all clients under Humans initially
242259
if (this.teamCount === HumansVsNations) {
@@ -252,17 +269,19 @@ export class LobbyTeamView extends LitElement {
252269
(c) =>
253270
new PlayerInfo(c.username, PlayerType.Human, c.clientID, c.clientID),
254271
);
255-
const assignment = assignTeams(players, teams);
272+
const assignment = assignTeamsLobbyPreview(
273+
players,
274+
teams,
275+
this.nationCount,
276+
);
256277
const buckets = new Map<Team, ClientInfo[]>();
257278
for (const t of teams) buckets.set(t, []);
258279

259280
for (const [p, team] of assignment.entries()) {
260281
if (team === "kicked") continue;
261282
const bucket = buckets.get(team);
262283
if (!bucket) continue;
263-
const client =
264-
this.clients.find((c) => c.clientID === p.clientID) ??
265-
this.clients.find((c) => c.username === p.name);
284+
const client = this.clients.find((c) => c.clientID === p.clientID);
266285
if (client) bucket.push(client);
267286
}
268287

@@ -277,7 +296,7 @@ export class LobbyTeamView extends LitElement {
277296
// Fallback: divide players across teams; guard against 0 and empty lobbies
278297
this.teamMaxSize = Math.max(
279298
1,
280-
Math.ceil(this.clients.length / teams.length),
299+
Math.ceil((this.clients.length + this.nationCount) / teams.length),
281300
);
282301
}
283302
this.teamPreview = teams.map((t) => ({

src/core/game/TeamAssignment.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { PlayerInfo, PlayerType, Team } from "./Game";
55
export function assignTeams(
66
players: PlayerInfo[],
77
teams: Team[],
8+
maxTeamSize: number = getMaxTeamSize(players.length, teams.length),
89
): Map<PlayerInfo, Team | "kicked"> {
910
const result = new Map<PlayerInfo, Team | "kicked">();
1011
const teamPlayerCount = new Map<Team, number>();
@@ -25,8 +26,6 @@ export function assignTeams(
2526
}
2627
}
2728

28-
const maxTeamSize = Math.ceil(players.length / teams.length);
29-
3029
// Sort clans by size (largest first)
3130
const sortedClans = Array.from(clanGroups.entries()).sort(
3231
(a, b) => b[1].length - a[1].length,
@@ -87,3 +86,19 @@ export function assignTeams(
8786

8887
return result;
8988
}
89+
90+
export function assignTeamsLobbyPreview(
91+
players: PlayerInfo[],
92+
teams: Team[],
93+
nationCount: number,
94+
): Map<PlayerInfo, Team | "kicked"> {
95+
const maxTeamSize = getMaxTeamSize(
96+
players.length + nationCount,
97+
teams.length,
98+
);
99+
return assignTeams(players, teams, maxTeamSize);
100+
}
101+
102+
export function getMaxTeamSize(numPlayers: number, numTeams: number): number {
103+
return Math.ceil(numPlayers / numTeams);
104+
}

0 commit comments

Comments
 (0)