jadijadi / riverraidrust

A text based river raid clone in Rust; streamed
GNU General Public License v3.0
91 stars 27 forks source link

Code refactor (No changes in game logic) #69

Closed immmdreza closed 5 months ago

immmdreza commented 5 months ago

Changes made

Using traits Drawable and StdoutExt

These two traits make life easier. for instance, simply use

stdout.draw(locattion, drawable)

// Instead of
stdout.queue(curor.MoveTo(...)).queue(style.Print(...))

and things like that.

Unleash more of enums power

Rust enums can contain data, for instance we can make sure the DeathCause enum can only be used if the PlayerStatus is Death.

pub enum PlayerStatus {
    Dead(DeathCause),

    ...
}

Use Iterator's method or for item in items

Instead of looping over indexes, we can loop over objects itself.

for index in (0..world.fuels.len()).rev() {
   let fuel = world.fuels[index]
}

can written as

for fuel in world.fuels.iter() { ... }

// If you need to mutate state
for fuel in world.fuels.iter_mut() { ... }

if item are going to be removed while iterating, use .retain or .retain_mut

self.enemies.retain_mut(|enemy| {
    enemy.location.l += 1;
    // Retain enemies within the screen
    enemy.location.l < self.maxl
});

Safety in rust

Don't use mut or mut & where it's forbidden or not necessary. As instance, draw method should not be able to mutate World's state! and its only duty is to draw.

Finally I moved/renamed a lot of namespaces around to make things more clear (at least to myself).

esafb52 commented 5 months ago

great job

immmdreza commented 5 months ago

great job

Thanks, You should checkout new-structure branch of my fork. it's getting interesting.

jadijadi commented 5 months ago

Thanks @immmdreza for the great contribution. I was worried that what I've written is not rust in the true sense :D

But... but how can I add color now? I merged this live and wanted to add color later but after the merge, found out that I can not use the crossterm's SetForegroundColor and such anymore. Is it possible for you to add the ANSI color control possibility? Say as a call to draw_char / draw_line?

immmdreza commented 5 months ago

Thanks @immmdreza for the great contribution. I was worried that what I've written is not rust in the true sense :D

But... but how can I add color now? I merged this live and wanted to add color later but after the merge, found out that I can not use the crossterm's SetForegroundColor and such anymore. Is it possible for you to add the ANSI color control possibility? Say as a call to draw_char / draw_line?

Yeah, I was able to find a quick solution for this and it seems working (#77).

ghost commented 5 months ago

@immmdreza I just checked your PR now It's very great that we have such Talented friends around us And there'll be an opportunity To learn from you guys thank you.

immmdreza commented 5 months ago

@immmdreza I just checked your PR now It's very great that we have such Talented friends around us And there'll be an opportunity To learn from you guys thank you.

Thank you, I am honored to help.