Dummy PR for review #1
Conversation
The game is not over even then, if there is an alive troop.
There was a problem hiding this comment.
Összességében egy első játékszervernek teljesen rendben van a kód, szép munka 👍
A hibakezeléssel itt is több szinten kell majd még foglalkoznunk, hogy a szerver se haldokoljon és a játékosoknak is legyen valami fogalma hogy mi a baj a botjukkal (vissza kéne küldeni hibaüzeneteket a botnak és / vagy kiírni a vizualizálóval).
Azt a néhány komolyabb hibát / átgondolni valót ami találtam ❗ -el jelöltem. A többi csak elírás, tipp, ízlés kérdése.
BotWraper.ts
Outdated
| import { stringify } from 'node:querystring'; | ||
| import { Queue } from 'queue-typescript'; | ||
|
|
||
| export enum ErrorCode { |
There was a problem hiding this comment.
A Success is error code? 🤔
Nem inkább ExitCode?
tsconfig.json
Outdated
| @@ -8,7 +8,7 @@ | |||
| "outDir": "./dist", | |||
| "rootDir": "./", | |||
| "sourceMap": true, | |||
| "target": "ESNext", | |||
| "target": "ES6", | |||
| "moduleResolution": "node" | |||
There was a problem hiding this comment.
| "moduleResolution": "node" | |
| "moduleResolution": "node", | |
| "strict": true |
Recommended, a TS szerint is https://www.typescriptlang.org/tsconfig#strict
BotWraper.ts
Outdated
|
|
||
| export class Data { | ||
| id: number; | ||
| data?: string; |
There was a problem hiding this comment.
| data?: string; | |
| data: string | null; |
Az, hogy TLE-nél data: null-t adsz vissza, az tök jó. A null egy explicit semmi, az undefined meg csak úgy "épp nem jutott oda adat".
Viszont itt a ? lényegében string | undefined-ot jelent, strict módban szól is miatta a compiler.
https://www.typescriptlang.org/docs/handbook/2/objects.html#optional-properties
BotWraper.ts
Outdated
| error_code: ErrorCode; | ||
| active: boolean; | ||
| process: ChildProcess; | ||
| std_out: Queue<string>; |
There was a problem hiding this comment.
Miért Queue? Nekem úgy tűnik, mintha a sima array is elég lenne.
nanowar.ts
Outdated
| // Add planets to | ||
| planetWaitingList[planet].push(user); | ||
| console.log("Arrived:", planetWaitingList); | ||
| state.tick.troops.splice(i, 1); // Removing troop from list |
There was a problem hiding this comment.
Ha a tick.troops egy endTick szerint indexelt Map lenne, akkor
- itt egy lépésben ki tudnád törölni az összeset aki ebben a körben érkezett meg O(n^2) helyett
- a for ciklusban sem kellene az összes troopot végignézni
No description provided.