Feedback #1
Feedback #1
Conversation
![github-classroom[bot]](https://avatars.githubusercontent.com/in/68187?s=60&v=4){kind=small}
|
|
||
| ## Libraries | ||
| - Bevy | ||
| - Serde (for serialization) |
There was a problem hiding this comment.
Nie jestem pewien, czy łączenie Bevy z Nannou to dobry pomysł: obydwa raczej działają z założeniem, że zarządzają własnym oknem i grafiką. Chyba lepiej byłoby do bevy użyć egui albo bevy_prototype_lyon (do rysowania prostych kształtów jak svg)
rog1gor
force-pushed
the
main
branch
2 times, most recently
from
da55829 to
0d445aa
Compare
There was a problem hiding this comment.
Bardzo kiepski error handling (unwrapy).
Typy są w niektórych miejscach, a w innych nie ma (np. let connection_established: bool = false - wiadomo przecież, że to bool, ani nie polepsza to czytelności, ani nie pomaga kompilatorowi w inferencji typów...)
Problemy ze współbieżnością.
Kod niepoformatowany.
Trochę się obawiam, czy nawet jak kolega @obukaj wrzuci swoją część kodu, to czy uda się Wam zrobić drugą część (GUI).
Zamiast dwóch osobnych projektów proponuję zrobić jeden projekt (tzn. jeden plik Cargo.toml) z podfolderami i dwoma binarkami (w katalogu bin).
Kod w tym momencie oceniłbym na 3/5, ale ponieważ mieliście dwa dni spóźnienia, to zgodnie z tym co ustaliliśmy, mnożę to razy 0.8. Czyli ostatecznie wychodzi 2.4/5
rusty_cards/client/src/main.rs
Outdated
|
|
||
| use p2p_client::Client; | ||
|
|
||
| // Sending a message that should explain what kind of message is expected |
There was a problem hiding this comment.
Raczej w dokumentacji się pisze, że funkcja coś robi, a nie że funkcja to jest robienie czegoś. Cf. https://doc.rust-lang.org/std/primitive.bool.html#implementations
rusty_cards/client/src/main.rs
Outdated
| } | ||
|
|
||
| // Sending a message that we're expecting a socket address of a server | ||
| // reading that socket addres from stdin and parsing it into SocketAddress |
There was a problem hiding this comment.
| // reading that socket addres from stdin and parsing it into SocketAddress | |
| // reading that socket address from stdin and parsing it into SocketAddress |
rusty_cards/client/src/main.rs
Outdated
| // reading that socket addres from stdin and parsing it into SocketAddress | ||
| // If the address is incorrect, then returns a default socket address | ||
| fn provide_server() -> SocketAddr { | ||
| let input = provide_input("Provide a socket that the server will be binded to"); |
There was a problem hiding this comment.
| let input = provide_input("Provide a socket that the server will be binded to"); | |
| let input = provide_input("Provide a socket that the server will be bound to"); |
rusty_cards/client/src/main.rs
Outdated
|
|
||
| // Sending a message that should explain what kind of message is expected | ||
| // and reading that message from stdin | ||
| fn provide_input(msg: &str) -> String { |
There was a problem hiding this comment.
chyba lepsza byłaby nazwa ask_for_input
rusty_cards/client/src/main.rs
Outdated
| socket_addr | ||
| } | ||
|
|
||
| // Sending a message that we're expecting a socket address of a TcpListener |
There was a problem hiding this comment.
a tak w ogóle to to przecież nie jest żadne wysyłanie, tylko wypisywanie (print, a nie send)
rusty_cards/client/src/p2p_client.rs
Outdated
|
|
||
| // Function that provides steps for the player who received Send message from the server | ||
| pub fn received_send(&self, addr: SocketAddr, password: String) -> Option<TcpStream> { | ||
| self.going_first.store(true, Ordering::Relaxed); // This player goes first |
There was a problem hiding this comment.
dlaczego ta zmienna jest atomowa? Nie musi być, bo i tak dotykamy jej tylko z jednego wątku
rusty_cards/client/src/p2p_client.rs
Outdated
| let mut buffer = [0; 1024]; | ||
| let mut num_bytes = 0; | ||
| while num_bytes == 0 { | ||
| num_bytes = opponent_stream.read(&mut buffer).unwrap(); | ||
| } |
rusty_cards/server/src/server.rs
Outdated
| } | ||
|
|
||
| s.spawn(|| { | ||
| match Self::handle_connection(&mut players.lock().unwrap(), stream) { |
There was a problem hiding this comment.
to niedobrze dawać wnętrze zablokowanego muteksa do funkcji, bo blokuje to inne wątki
There was a problem hiding this comment.
to jest bardzo poważny błąd z dziedziny programowania współbieżnego
| fn handle_connection(players: &mut Vec<ClientsAddr>, mut stream: TcpStream) -> io::Result<()> { | ||
| println!("Handling connection..."); | ||
| let mut buffer = [0; 1024]; | ||
| let num_bytes = stream.read(&mut buffer)?; |
There was a problem hiding this comment.
na tym wywołaniu funkcja może się zablokować na dowolnie długo, ale przecież nie potrzeba jej w tym miejscu jeszcze do niczego referencji do players
rusty_cards/server/src/server.rs
Outdated
| pub fn port(&self) -> u16 { | ||
| self.tcp_listener.local_addr().unwrap().port() | ||
| } | ||
| } |
Project is ready for grading
| Action::PlayCard(n1, n2) => is_legal_to_play(n1, n2, game_state), | ||
| Action::EndTurn => true, | ||
| Action::Help => true, | ||
| _ => panic!("Unknown or not ingame action"), // this should never be reached |
| rand_int: i32, // used for deck shuffling | ||
| } | ||
|
|
||
| impl Ord for Minion { |
There was a problem hiding this comment.
raczej nie powinno się rącznie nadpisywać Ord i Cmp, lepiej opakować ten typ w NewType używany do sortowania, a w strukturze Minion zostawić naturalne sortowanie/porównywanie
struct SortedMinion(i32, Minion)
👋! GitHub Classroom created this pull request as a place for your teacher to leave feedback on your work. It will update automatically. Don’t close or merge this pull request, unless you’re instructed to do so by your teacher.
In this pull request, your teacher can leave comments and feedback on your code. Click the Subscribe button to be notified if that happens.
Click the Files changed or Commits tab to see all of the changes pushed to
mainsince the assignment started. Your teacher can see this too.Notes for teachers
Use this PR to leave feedback. Here are some tips:
mainsince the assignment started. To leave comments on specific lines of code, put your cursor over a line of code and click the blue + (plus sign). To learn more about comments, read “Commenting on a pull request”.main. Click a commit to see specific changes.For more information about this pull request, read “Leaving assignment feedback in GitHub”.
Subscribed: @rog1gor