Add Mafia search capability #10467
Add Mafia search capability #10467
Conversation
A new theme includes a role that can change someone's alignment as a discard. Players should not be informed according to their old alignment.
Change a longer definition to a predefined variable.
Merge updates
Scav HTML Hunts
There was a problem hiding this comment.
Ok, I'm not done with this review yet but I need to go deal with other things and there appears to be some fundamental design issues here since you just stuck [key: string]: any on MafiaData, lets figure out how we want to deal with that before anything else.
server/chat-plugins/mafia.ts
Outdated
| this.hostid = toID(""); | ||
| this.host = ""; |
There was a problem hiding this comment.
| this.hostid = toID(""); | |
| this.host = ""; |
There was a problem hiding this comment.
Your setting these values below, why set them to empty strings here?
server/chat-plugins/mafia.ts
Outdated
|
|
||
| this.hostid = host.id; | ||
| this.host = Utils.escapeHTML(host.name); | ||
| this.host = Utils.escapeHTML(host.name) && ''; |
There was a problem hiding this comment.
| this.host = Utils.escapeHTML(host.name) && ''; | |
| this.host = Utils.escapeHTML(host.name); |
&& ''; will mean this always returns '', you probably meant || ''; to say "use the HTML escaped host name or an empty string as a default". However, that isn't needed, if you look at the definition for escapeHTML, you can see that it always returns a string (even an empty one if needed).
server/chat-plugins/mafia.ts
Outdated
| hostPregame() { | ||
|
|
||
| } | ||
|
|
||
| hostDay() { | ||
|
|
||
| } | ||
|
|
||
| hostNight() { | ||
|
|
||
| } |
There was a problem hiding this comment.
Why do we have three empty methods?
server/chat-plugins/mafia.ts
Outdated
| @@ -1,6 +1,7 @@ | |||
| import {Utils, FS} from '../../lib'; | |||
|
|
|||
| interface MafiaData { | |||
| [key: string]: any; | |||
There was a problem hiding this comment.
This typing says "you can put anything in a MafiaData object at all! We have no idea whats in here as a result" and thats bad. Is there a more specific typing you could be using? Why do we need this typing?
server/chat-plugins/mafia.ts
Outdated
| if (!MafiaData.themes.nominations) { | ||
| MafiaData.themes.nominations = { | ||
| name: `Nominations`, | ||
| desc: `Every odd night, the Mafia select three players. The next day, you can only vote for one of these three players.`, | ||
| 7: `Mafia Goon, Mafia Goon, Villager, Villager, Villager, Villager, Villager`, | ||
| 11: `Mafia Goon, Mafia Goon, Mafia Goon, Villager, Villager, Villager, Villager, Villager, Villager, Villager, Villager`, | ||
| 15: `Mafia Goon, Mafia Goon, Mafia Goon, Mafia Goon, Villager, Villager, Villager, Villager, Villager, Villager, Villager, Villager, Villager, Villager, Villager`, | ||
| 19: `Mafia Goon, Mafia Goon, Mafia Goon, Mafia Goon, Mafia Goon, Villager, Villager, Villager, Villager, Villager, Villager, Villager, Villager, Villager, Villager, Villager, Villager, Villager, Villager`, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Why does nommy need to be hardcoded here, isn't theme data usually just located in the appropriate JSON file?
server/chat-plugins/mafia.ts
Outdated
| @@ -3988,7 +4018,7 @@ export const commands: Chat.ChatCommands = { | |||
| let foundTarget = false; | |||
| for (const entry of ['alignments', 'roles', 'themes', 'IDEAs', 'terms'] as (keyof MafiaData)[]) { | |||
| const dataEntry = MafiaData[entry]; | |||
| if (from in dataEntry) return this.errorReply(`${from} is already a ${entry.slice(0, -1)}`); | |||
| if (from in dataEntry) return this.errorReply(`${from} is already a ${entry.toString().slice(0, -1)}`); | |||
There was a problem hiding this comment.
entry should already implicitly defined as a string, this extra method call isn't needed.
If its not its likely a problem caused by introducing [key: string]: any as a type on MafiaData.
Overhauls the 'mafia listdata' command, to instead show data in a neater way. Allows for the user to search within the various Mafia data categories, and also generate random data, like the 'ds' command.
Collaboration with @Alexander489