Feedback #1
Feedback #1
Conversation
![github-classroom[bot]](https://avatars.githubusercontent.com/in/68187?s=60&v=4){kind=small}
|
|
||
| In the second part we're going to add random map generator, saving/loading, scores and a more sophisticated AI. | ||
| In the second part we're going to add recursive boxes. |
There was a problem hiding this comment.
Chodzi o to, żeby pudełka mogły być jakby mini-poziomami, opcjonalnie zawierającymi inne pudełka/punkty końcowe. Podobnie jak w grze Patrick's Parabox: https://store.steampowered.com/app/1260520/Patricks_Parabox/
There was a problem hiding this comment.
Funkcje próbują za dużo robić. Trochę jest spaghetti code, który wynika, zdaje się, głównie z niezbyt dobrego ogarnięcia konceptów ECS. W "keyboard_input_system" nie powinniście wywoływać funkcji, które potem wywołują jeszcze inne funkcje odpowiadające za logikę gry... Systemy powinny być od siebie maksymalnie niezależne i brać jak najmniej inputów.
4/5
src/game/display.rs
Outdated
| } | ||
|
|
||
| //render an object with a given image and position | ||
| fn spawn_entity<T>( |
There was a problem hiding this comment.
ta funkcja próbuje za dużo robić. Proponuję osobno dodać obrazek do materials, a zamiast ręcznego spawnowania wewnątrz tej funkcji proponuję stworzyć nowy Bundle (Bundle mogą być zagnieżdżone w Bundlach) i zrobić konstruktor do niego
src/game/display.rs
Outdated
| for j in left_border..(right_border + 1) { | ||
| let entity = board | ||
| .entities | ||
| .get(&Position { x: j, y: i }) |
There was a problem hiding this comment.
to może lepiej x zamiast j i y zamiast i...?
src/game/game_objects.rs
Outdated
| impl PartialEq for GameObjects { | ||
| fn eq(&self, other: &Self) -> bool { | ||
| match (self, other) { | ||
| (Self::Box(_), Self::Box(_)) => true, //we do not care about the insides of Box when comapring | ||
| (Self::Wall, Self::Wall) => true, | ||
| (Self::Empty, Self::Empty) => true, | ||
| _ => false, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
skoro nadpisujecie PartialEq to warto się zastanowić, czy ten enum jest dobrym miejscem do trzymania tych danych, które tutaj ignorujecie, in the first place
src/game/game_objects.rs
Outdated
| Down, | ||
| Left, | ||
| Right, | ||
| None, |
src/game/movement.rs
Outdated
| board.entities.remove(&old_position); | ||
| board | ||
| .entities | ||
| .insert(next_position, GameObjects::Box(Some(current_entity))); |
There was a problem hiding this comment.
a czemu nie zmienić po prostu tego jednego komponentu? (zamiast usuwać i dodawać nową entity)
src/game/movement.rs
Outdated
| .insert(next_position, GameObjects::Box(Some(current_entity))); | ||
| } | ||
|
|
||
| fn update_box( |
There was a problem hiding this comment.
bardzo mało mówiąca nazwa funkcji, która próbuje zrobić bardzo dużo różnych rzeczy
src/game/movement.rs
Outdated
| let (mut player, mut transform) = sprite_position.single_mut(); | ||
| let new_position = player.position.neighbour(direction); | ||
| if direction == Direction::None { | ||
| () |
src/game/movement.rs
Outdated
| } | ||
| } | ||
|
|
||
| fn update_pos( |
src/game/movement.rs
Outdated
| // move is only possible once every MOVE_DELAY seconds so only when timer is finished | ||
| let dir = set_direction(keyboard_input); | ||
| if timer.0.finished() { | ||
| update_pos(sprite_position, dir, timer, &mut board, &mut boxes); | ||
| } else { | ||
| timer.0.tick(time.delta()); | ||
| } |
There was a problem hiding this comment.
to niedobry pomysł... zastanawiałem się czemu to chodzenie jest takie nieresponsywne. Ja bym chyba to zrobił tak: przez MOVE_DELAY czasu zbierać input i pamiętać ostatni (jeśli jakikolwiek był - Option, a nie Direction::None!) w resource, a po upływie MOVE_DELAY przeprocesować ruch
src/menu/spawn_button.rs
Outdated
| @@ -0,0 +1,38 @@ | |||
| use bevy::prelude::*; | |||
|
|
|||
| pub fn spawn_button<T>(parent: &mut ChildBuilder, entity: T, menu_font: Handle<Font>, value: &str) | |||
There was a problem hiding this comment.
takie różne "utils" to raczej do mod.rs powinny iść, a na pewno nie do osobnego pliku, bo potem trzeba go importować przez use cośtam::spawn_button::spawn_button
👋! 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: @jkurys @Kinzoku99