Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#91 machine erweitern #132

Merged
merged 18 commits into from
Nov 25, 2022
Merged

#91 machine erweitern #132

merged 18 commits into from
Nov 25, 2022

Conversation

SanderForGodot
Copy link
Contributor

@Nereuxofficial ich will den merge buton clicken weills toll ist

Copy link
Contributor

@Nereuxofficial Nereuxofficial left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sieht schon gut aus, fix die angemerkten Sachen noch und dann kann man gemeinsam die Sachen fixen und refactoren (PS: cargo clippy ist dein Freund)

pub fn create_machine(&mut self) {
dbg!(format!("create_machine",));
let new_ms = Mashine::default(self);
self.areas.push(Box::new(new_ms));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bitte nicht die Funktion default nennen wenn man ein Argument übergeben muss

let machine = area.get_graphic();
let pos = Vec2 {
x: area.get_collision_area().x,
y: area.get_collision_area().y,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vec2::new ist kürzer

info!("In interaction area: {:?}", self.get_interactable());
let player_ref = &self.player.clone();
self.get_interactable().unwrap().interact(player_ref); //TODO: change the unwrap
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Statt unwrap können wir hier
if let Ok(interactable) = self.getinteractable(){ interactable.interact() }. Das panicken verhindern

Copy link
Contributor

@mfloto mfloto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


impl GameState {
pub fn create_machine(&mut self) {
dbg!(format!("create_machine",));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aus dem dbg bitte ein Info!() fürs logging

pub struct Maschine {
//gamestate:GameState,
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Mashine {
Copy link
Member

@Flippchen Flippchen Nov 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Immer noch falsch geschrieben xD

Copy link
Member

@Flippchen Flippchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Änderungen sehen gut aus, warte aber noch auf Bene xD

Copy link
Contributor

@mfloto mfloto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM again

@SanderForGodot SanderForGodot merged commit ce77d4e into dev Nov 25, 2022
@SanderForGodot SanderForGodot deleted the #91_Machine_erweitern branch November 25, 2022 20:59
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants