Conversation
| this.room = thisRoom; | ||
| } | ||
|
|
||
| private cachedSpyUsernames?: string[]; |
There was a problem hiding this comment.
| private cachedSpyUsernames?: string[]; | |
| private cachedSpyUsernames: string[] = []; |
Something like this? Can just default it to empty list :).
There was a problem hiding this comment.
Also let's call the list spiesThatMelronSees or something similar :).
There was a problem hiding this comment.
Changed the name of the list - but if the default state is an empty list, it never initializes making a list, since right now it's triggered by there being no list. I can rework if that's a sloppy way to do it but for now this is necessary.
| this.room = thisRoom; | ||
| } | ||
|
|
||
| private cachedSpyUsernames?: string[]; |
There was a problem hiding this comment.
Also tabbing is a bit off here haha. Can you use prettier to format this? It's a tool that helps you format files. I have a .prettierrc file in the repo that your tool should pick up automatically.
| if (p.alliance === Alliance.Spy) { | ||
| realSpyCount++; | ||
| if (p.role === Role.Mordred || p.role === Role.MordredAssassin) { | ||
| hasMordred = true; |
There was a problem hiding this comment.
Can we just do realSpyCount-- here instead of tracking hasMordred?
Let's us remove L47 below as well.
There was a problem hiding this comment.
Good catch, changed.
| const self = this.getSelfUsername(); | ||
| const pool = this.room.playersInGame | ||
| .map((p: any) => p.username) | ||
| .filter((u: string) => u !== self); |
There was a problem hiding this comment.
Can you instead filter based on role? So something like
const pool = this.room.playersInGame
.map((p: any) => p.role)
.filter((role: Role) => role !== Melron);
This means we can also remove this.getSelfUsername()
There was a problem hiding this comment.
Now filtering on role and got rid of getselfusername
| // Shuffle once and cache picks | ||
| for (let i = pool.length - 1; i > 0; i--) { | ||
| const j = Math.floor(Math.random() * (i + 1)); | ||
| [pool[i], pool[j]] = [pool[j], pool[i]]; | ||
| } |
There was a problem hiding this comment.
Could we pull this logic out to a helper or utils file? I think this logic also existed in game.ts from memory.
Would be great to unify these!
There was a problem hiding this comment.
Used shuffleArray, thanks!
| this.room = thisRoom; | ||
| } | ||
|
|
||
| private cachedSpyUsernames?: string[]; // includes self username as first element |
There was a problem hiding this comment.
Let's rename this as well similar to my comment in melron.ts :)
There was a problem hiding this comment.
As above, in current architecture having the default state by empty breaks things
| for (const p of this.room.playersInGame) { | ||
| if (p.alliance === Alliance.Spy) { | ||
| realSpyCount++; | ||
| if (p.role === Role.Oberon) hasOberon = true; |
There was a problem hiding this comment.
similar to hasMord, let's just realSpyCount--
|
|
||
| const pool = this.room.playersInGame | ||
| .map((p: any) => p.username) | ||
| .filter((u: string) => u !== self); |
There was a problem hiding this comment.
Let's do a similar thing here too :). Filter by role so we can remove getSelfUsername.
| if (!this.room.gameStarted) return { spies: [], roleTags: {} }; | ||
|
|
||
| const self = this.getSelfUsername(); | ||
| if (!self) return { spies: [], roleTags: {} }; |
There was a problem hiding this comment.
I don't think we'll need this check. If a Moregano object/role exists in the game, I think it's safe to assume there'll be a username that exists :).
| return this.room.playersInGame[i].username; | ||
| } | ||
| } | ||
| return ''; |
There was a problem hiding this comment.
If we were to keep this function, we'd probably first move it to game.ts and then it'd be a good idea to return a string | null, so that it's clear to the caller that the return value may not be valid.
There was a problem hiding this comment.
The function is gone anyway :)
| [MordredAssassin.role]: MordredAssassin, | ||
| [Hitberon.role]: Hitberon, | ||
|
|
||
| [Melron.role]: Melron, |
| MordredAssassin = 'MordredAssassin', | ||
| Hitberon = 'Hitberon', | ||
|
|
||
| // ADD THESE TWO |
| for (let i = 0; i < this.playersInGame.length; i++) { | ||
| const p = this.playersInGame[i]; | ||
| if (p.role === Role.Melron) { | ||
| p.displayRole = Role.Merlin; // Melron thinks they are Merlin | ||
| } else if (p.role === Role.Moregano) { | ||
| p.displayRole = Role.Morgana; // Moregano thinks they are Morgana | ||
| (p as any).displayAlliance = Alliance.Spy; | ||
| } | ||
| } |
There was a problem hiding this comment.
Is it possible to use a for (const player of this.playersInGame) loop?
There was a problem hiding this comment.
Good idea, implemented.
| p.displayRole = Role.Merlin; // Melron thinks they are Merlin | ||
| } else if (p.role === Role.Moregano) { | ||
| p.displayRole = Role.Morgana; // Moregano thinks they are Morgana | ||
| (p as any).displayAlliance = Alliance.Spy; |
There was a problem hiding this comment.
Can you remove the as any's here?
| const effectiveAlliance = | ||
| (playerRoles[i] as any).displayAlliance !== undefined | ||
| ? (playerRoles[i] as any).displayAlliance | ||
| : playerRoles[i].alliance; |
There was a problem hiding this comment.
Can we simplify this to
const effectiveAlliance = playerRoles[i].displayAlliance ? playerRoles[i].displayAlliance : playerRoles[i].alliance?
| const effectiveAlliance = | ||
| (player as any).displayAlliance !== undefined | ||
| ? (player as any).displayAlliance | ||
| : player.alliance; |
There was a problem hiding this comment.
Let's also simplify this - see prior comment pls
There was a problem hiding this comment.
I wasn't actually able to figure out how to simplify this one - it doesn't compile if I use analogous language to game.ts. If this is a problem I'll try to figure it out...
| this.room = thisRoom; | ||
| } | ||
|
|
||
| private cachedSpyUsernames?: string[]; |
There was a problem hiding this comment.
Do you think it'd be nice for the game to announce who was melron and moregano and what team they saw?
Can you also confirm whether the moment the game ends, melron and moregano see the true spy team in the end screen?
There was a problem hiding this comment.
Got that postgame announcement feature working!
There was a problem hiding this comment.
Can you ask AI to add a unit test to ensure that moregano fail attempts are changed to a success pls?
There was a problem hiding this comment.
Unit test added and succeeds.

Adding two new roles:
Melron is a Resistance member who thinks that they are Merlin. They see an appropriate number of players as Spies, but their information is random.
Moregano is a Resistance member who thinks that they are Morgana. They see an appropriate number of players as Spymates, but their information is random. They can try to Fail, but it will be logged as a Success.