Red herrings #770
Red herrings #770
Conversation
Added keep_lesser, and keep_greater Added retain_mut Added with_capacity
|
this also makes a change to confusion & drunk aura that allows you apply them to a player temporarily. |
|
includes stuff in #763 |
The merge fucked it up
|
It should be good now |
…e to prevent conflicts.
There was a problem hiding this comment.
Split this into multiple PRs. Red herrings should be its own
However i have alot to talk about with this so can i just call you
There should be able to be multiple red herrings
Red herrings should not be stored on rolestate, instead we can make a new "component"
I dont like duration but i also dont get it. Duration is a bad name because there is a rust type in std called duration
There was a problem hiding this comment.
Can you remove the status duration thing
Put drunk aura back to what it was before if you can
If you want status duration thing, put it in another PR
Also it should iterate the timer on its own
is_evil is a bad name. What is evil?
Can you just make it so floor(1/3) of the lobby is a red herring for now
Especially because red-herring might mess with different roles in different ways
Also can you make Detective::player_is_suspicious_confused(game, target_ref) to just make the code cleaner. Do this for all the other red-herring roles
The detective game logic for determining if a player is suspicious should be RedHerring OR Suspicious Aura!
Gossip should just call the detective functions instead of repeating the code
For philosopher, treat RedHerring the same as suspicious aura, like detective
If i wasn't clear please ask
Remove all the extra println!
Move VecMap stuff to another PR you dont need it here
Address comments
Please dont put multiple things in PR
Dispite all the negative feedback this looks pretty good i wanna add this alot. Keep it up!
| @@ -104,6 +109,10 @@ impl<K, V> VecMap<K, V> where K: Eq { | |||
| self.vec.retain(|(k, v)| f(k, v)); | |||
| } | |||
|
|
|||
| pub fn retain_mut<F>(&mut self, mut f: F) where F: FnMut(&mut K, &mut V) -> bool { | |||
| self.vec.retain_mut(|(k, v)| f(k, v)); | |||
There was a problem hiding this comment.
this shouldnt have a mutable reference to the key, key should be immutable
Also this should not be in this PR
| self.vec.push((key, value)); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Almost never use [i] and if you do, put it in an unsafe block i think
Also move all this stuff to another PR
There was a problem hiding this comment.
if I had it as get, the other half of the match statement would be an unreachable macro, its also completely safe because its accessing a value that must be in bound because position got it.
…out the player changes. added get_confusion_data func. updated tests
There was a problem hiding this comment.
Regarding the chat message rename
You need to do it in ChatMessage.tsx in 2 locations (switch statement and type)
I still dont like the is_evil function name, especially because that would need to be explained in the manual.
If you want to keep it your way instead of being 1/3 of the lobby. (im starting to see your vision)
Rename is_evil to this
adds_red_herring(outline: &RoleOutline)->booland move the function to be inside an implementation of confused
Also dont forget about the manual & my other review
added red herrings for gossip, detective, & philosopher.