Skip to content

Commit d933b25

Browse files
committed
refactor: address code review feedback for lobby chat feature
- Remove self-describing comments - Remove redundant isPublic() check in chat validation - Send username instead of clientID to prevent game history exposure - Add host designation badge in chat messages - Prettify CSS styles with proper formatting - Use transparent black backgrounds instead of solid colors - Revert unrelated OutlineFilter changes from StructureIconsLayer
1 parent 93cf256 commit d933b25

File tree

5 files changed

+69
-42
lines changed

5 files changed

+69
-42
lines changed

src/client/ClientGameRunner.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,10 +146,13 @@ export function joinLobby(
146146
}
147147
}
148148
if (message.type === "lobby_chat") {
149-
// Relay to UI components listening for lobby chat
150149
document.dispatchEvent(
151150
new CustomEvent("lobby-chat:message", {
152-
detail: { sender: message.sender, text: message.text },
151+
detail: {
152+
username: message.username,
153+
isHost: message.isHost,
154+
text: message.text,
155+
},
153156
bubbles: true,
154157
composed: true,
155158
}),

src/client/HostLobbyModal.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ export class HostLobbyModal extends LitElement {
5252
@state() private instantBuild: boolean = false;
5353
@state() private randomSpawn: boolean = false;
5454
@state() private compactMap: boolean = false;
55-
// New: Lobby chat toggle (private lobbies only)
5655
@state() private chatEnabled: boolean = false;
5756
@state() private lobbyId = "";
5857
@state() private copySuccess = false;

src/client/components/LobbyChatPanel.ts

Lines changed: 50 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ import { EventBus } from "../../core/EventBus";
55
import { SendLobbyChatEvent } from "../Transport";
66

77
interface ChatMessage {
8-
sender: string;
8+
username: string;
9+
isHost: boolean;
910
text: string;
1011
}
1112

@@ -15,20 +16,14 @@ export class LobbyChatPanel extends LitElement {
1516
@state() private inputText: string = "";
1617

1718
private bus: EventBus | null = null;
18-
private myClientID: string | null = null;
1919

2020
connectedCallback(): void {
2121
super.connectedCallback();
22-
// Listen for websocket relay from ClientGameRunner
2322
document.addEventListener("lobby-chat:message", this.onIncoming as any);
24-
// Attempt to attach global EventBus if available
2523
const globalBus = (window as any).__eventBus as EventBus | undefined;
2624
if (globalBus) {
2725
this.bus = globalBus;
2826
}
29-
// Capture my client ID for alignment
30-
this.myClientID = (window as any).__clientID ?? null;
31-
// Proactively bind when the bus becomes ready later
3227
document.addEventListener("event-bus:ready", this.onBusReady as any);
3328
}
3429

@@ -43,10 +38,10 @@ export class LobbyChatPanel extends LitElement {
4338
}
4439

4540
private onIncoming = async (
46-
e: CustomEvent<{ sender: string; text: string }>,
41+
e: CustomEvent<{ username: string; isHost: boolean; text: string }>,
4742
) => {
48-
const { sender, text } = e.detail;
49-
this.messages = [...this.messages, { sender, text }];
43+
const { username, isHost, text } = e.detail;
44+
this.messages = [...this.messages, { username, isHost, text }];
5045
await this.updateComplete;
5146
const container = this.renderRoot.querySelector(
5247
".lcp-messages",
@@ -59,25 +54,21 @@ export class LobbyChatPanel extends LitElement {
5954
if (globalBus) {
6055
this.bus = globalBus;
6156
}
62-
this.myClientID = (window as any).__clientID ?? this.myClientID;
6357
};
6458

6559
private sendMessage() {
6660
const text = this.inputText.trim();
6761
if (!text) return;
68-
// Lazily attach global EventBus if it wasn't ready at connect time
6962
if (!this.bus) {
7063
const globalBus = (window as any).__eventBus as EventBus | undefined;
7164
if (globalBus) {
7265
this.bus = globalBus;
7366
}
7467
}
75-
// If bus is still unavailable, surface the failure and do not clear input
7668
if (!this.bus) {
7769
console.warn("LobbyChatPanel: EventBus unavailable. Message not sent.");
7870
return;
7971
}
80-
// Enforce 300-char limit matching server SafeString.max(300)
8172
const capped = text.slice(0, 300);
8273
this.bus.emit(new SendLobbyChatEvent(capped));
8374
this.inputText = "";
@@ -88,11 +79,9 @@ export class LobbyChatPanel extends LitElement {
8879
<div class="lcp-container">
8980
<div class="lcp-messages">
9081
${this.messages.map((m) => {
91-
const isSelf =
92-
this.myClientID !== null && m.sender === this.myClientID;
93-
const cls = isSelf ? "lcp-msg lcp-right" : "lcp-msg lcp-left";
94-
return html`<div class="${cls}">
95-
<span class="lcp-sender">${m.sender}:</span> ${m.text}
82+
const displayName = m.isHost ? `${m.username} (Host)` : m.username;
83+
return html`<div class="lcp-msg">
84+
<span class="lcp-sender">${displayName}:</span> ${m.text}
9685
</div>`;
9786
})}
9887
</div>
@@ -126,17 +115,49 @@ export class LobbyChatPanel extends LitElement {
126115
}
127116
}
128117

129-
// Basic Tailwind-like classes, prefixed to avoid global conflicts
130118
const style = document.createElement("style");
131119
style.textContent = `
132-
.lcp-container { display:flex; flex-direction:column; gap:8px; max-height:240px; }
133-
.lcp-messages { overflow-y:auto; border:1px solid #444; border-radius:8px; padding:8px; height:180px; background:#111; color:#ddd; display:flex; flex-direction:column; gap:6px; }
134-
.lcp-msg { font-size: 0.9rem; max-width: 80%; padding:6px 10px; border-radius:10px; background:#1b1b1b; }
135-
.lcp-msg.lcp-left { align-self: flex-start; }
136-
.lcp-msg.lcp-right { align-self: flex-end; background:#243b55; }
137-
.lcp-sender { color:#9ae6b4; margin-right:4px; }
138-
.lcp-input-row { display:flex; gap:8px; }
139-
.lcp-input { flex:1; border-radius:8px; padding:6px 10px; color:#000; }
140-
.lcp-send { border-radius:8px; padding:6px 12px; }
120+
.lcp-container {
121+
display: flex;
122+
flex-direction: column;
123+
gap: 8px;
124+
max-height: 240px;
125+
}
126+
.lcp-messages {
127+
overflow-y: auto;
128+
border: 1px solid #444;
129+
border-radius: 8px;
130+
padding: 8px;
131+
height: 180px;
132+
background: rgba(0, 0, 0, 0.5);
133+
color: #ddd;
134+
display: flex;
135+
flex-direction: column;
136+
gap: 6px;
137+
}
138+
.lcp-msg {
139+
font-size: 0.9rem;
140+
padding: 6px 10px;
141+
border-radius: 10px;
142+
background: rgba(0, 0, 0, 0.6);
143+
}
144+
.lcp-sender {
145+
color: #9ae6b4;
146+
margin-right: 4px;
147+
}
148+
.lcp-input-row {
149+
display: flex;
150+
gap: 8px;
151+
}
152+
.lcp-input {
153+
flex: 1;
154+
border-radius: 8px;
155+
padding: 6px 10px;
156+
color: #000;
157+
}
158+
.lcp-send {
159+
border-radius: 8px;
160+
padding: 6px 12px;
161+
}
141162
`;
142163
document.head.appendChild(style);

src/client/graphics/layers/StructureIconsLayer.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ export class StructureIconsLayer implements Layer {
272272
canUpgrade: false,
273273
});
274274
this.ghostUnit.container.filters = [
275-
new OutlineFilter(2, 0xff0000) as any,
275+
new OutlineFilter({ thickness: 2, color: "rgba(255, 0, 0, 1)" }),
276276
];
277277
return;
278278
}
@@ -290,15 +290,15 @@ export class StructureIconsLayer implements Layer {
290290
);
291291
if (this.potentialUpgrade) {
292292
this.potentialUpgrade.iconContainer.filters = [
293-
new OutlineFilter(2, 0x00ff00) as any,
293+
new OutlineFilter({ thickness: 2, color: "rgba(0, 255, 0, 1)" }),
294294
];
295295
this.potentialUpgrade.dotContainer.filters = [
296-
new OutlineFilter(2, 0x00ff00) as any,
296+
new OutlineFilter({ thickness: 2, color: "rgba(0, 255, 0, 1)" }),
297297
];
298298
}
299299
} else if (unit.canBuild === false) {
300300
this.ghostUnit.container.filters = [
301-
new OutlineFilter(2, 0xff0000) as any,
301+
new OutlineFilter({ thickness: 2, color: "rgba(255, 0, 0, 1)" }),
302302
];
303303
}
304304

@@ -496,8 +496,12 @@ export class StructureIconsLayer implements Layer {
496496
render.iconContainer.alpha = structureInfos.visible ? 1 : 0.3;
497497
render.dotContainer.alpha = structureInfos.visible ? 1 : 0.3;
498498
if (structureInfos.visible && focusStructure) {
499-
render.iconContainer.filters = [new OutlineFilter(2, 0xffffff) as any];
500-
render.dotContainer.filters = [new OutlineFilter(2, 0xffffff) as any];
499+
render.iconContainer.filters = [
500+
new OutlineFilter({ thickness: 2, color: "rgb(255, 255, 255)" }),
501+
];
502+
render.dotContainer.filters = [
503+
new OutlineFilter({ thickness: 2, color: "rgb(255, 255, 255)" }),
504+
];
501505
} else {
502506
render.iconContainer.filters = [];
503507
render.dotContainer.filters = [];

src/server/GameServer.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -315,17 +315,17 @@ export class GameServer {
315315
break;
316316
}
317317
case "lobby_chat": {
318-
// Validate lobby chat usage: must be in lobby phase and chat enabled
319318
if (this.phase() !== GamePhase.Lobby) {
320319
return;
321320
}
322-
if (!this.gameConfig.chatEnabled || this.isPublic()) {
321+
if (!this.gameConfig.chatEnabled) {
323322
return;
324323
}
325-
// Broadcast to all clients in the same lobby
324+
const isHost = client.clientID === this.lobbyCreatorID;
326325
const payload = JSON.stringify({
327326
type: "lobby_chat",
328-
sender: client.clientID,
327+
username: client.username,
328+
isHost,
329329
text: (JSON.parse(message) as any).text,
330330
});
331331
this.activeClients.forEach((c) => c.ws.send(payload));

0 commit comments

Comments
 (0)