feat: ✨ Catfishes know about other catfishes #264
Conversation
gchamb
commented
Dec 31, 2022
gchamb
commented
- catfishes know about each other when roles are displayed
- highlight name in red in matched name if they're both catfishes
- highlight name in red for other catfishes in vote if they're a catfish
…-me-if-you-can into gchamb/show-catfishes
| allow get: if request.auth.uid == uid; | ||
| allow list: if isCatfish() && isCatfishResource(); |
There was a problem hiding this comment.
do we really want catfish to be able to read the entire privatePlayer document?
There was a problem hiding this comment.
They're only allowed to read other catfish private player docs in which only holds the their role(which we want them to know), stalker(wouldn't matter if they know because they're working together), and prompt in which they would have the same prompt.
There was a problem hiding this comment.
hmm I guess so but it feels wrong
There was a problem hiding this comment.
If you have a better idea lmk. We'd be only duplicating a little bit of data if you do have one
| @@ -156,6 +163,25 @@ | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| $: privatePlayer, getCatfishes(); | |||
| function getCatfishes() { | |||
There was a problem hiding this comment.
shouldn't we call unsubscribeCatfishes here?
There was a problem hiding this comment.
I think I understand but explain why?
There was a problem hiding this comment.
I'm pretty sure the way the code is currently written we resubscribe every time the private player changes (each time there is a new prompt for example) but we never unsubscribe so that will put a lot of unwanted stress on the database.
There was a problem hiding this comment.
why is unsubscribeCatfishes not called if the state is ROLE?
There was a problem hiding this comment.
So you saying subscribe for that moment then unsubscribe? And a side note I think the getDocs function might be better since we're not using the newly updated documents to display to the user or anything
There was a problem hiding this comment.
No I'm saying unsubscribe before you subscribe
There was a problem hiding this comment.
Yeah getDocs makes sense to me
There was a problem hiding this comment.
No I'm saying unsubscribe before you subscribe
Yeah, that makes sense. You want to make sure you unsubscribe from the previous subscription before you subscribe again in the Role phases case.
| if (catfishes == undefined) { | ||
| return; | ||
| } |
client/src/components/Vote.svelte
Outdated
| @@ -60,7 +61,9 @@ | |||
| on:click={() => vote(lobby.uids[i])} | |||
| > | |||
| <AvatarImg {avatar} /> | |||
| <span class="mdc-typography--subtitle1">{displayName ?? ""}</span> | |||
| <span class="mdc-typography--subtitle1 " class:catfish={catfishes.includes(lobby.uids[i])} | |||
| >{displayName ?? ""}</span | |||
There was a problem hiding this comment.
personally I like this
<span>
text
</span>more than this
<span
>text</span
>
client/src/components/Vote.svelte
Outdated
| @@ -60,7 +61,9 @@ | |||
| on:click={() => vote(lobby.uids[i])} | |||
| > | |||
| <AvatarImg {avatar} /> | |||
| <span class="mdc-typography--subtitle1">{displayName ?? ""}</span> | |||
| <span class="mdc-typography--subtitle1 " class:catfish={catfishes.includes(lobby.uids[i])} | |||
There was a problem hiding this comment.
you acedentaly added space in the class
| @@ -156,6 +163,25 @@ | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| $: privatePlayer, getCatfishes(); | |||
| function getCatfishes() { | |||
There was a problem hiding this comment.
why is unsubscribeCatfishes not called if the state is ROLE?
| .catfish { | ||
| color: #d32f2f; | ||
| } |
There was a problem hiding this comment.
this code is duplicated in ReceiveRole maybe it would be best to move all three role colors into the theme
…-me-if-you-can into gchamb/show-catfishes
