johnthagen / rust-belt

:rocket: Asteroids-like arcade game implemented in Rust ✨
72 stars 6 forks source link

Implement enemies #14

Open johnthagen opened 7 years ago

mcdenhoed commented 7 years ago

Initial try at implementation can be found in the enemy branch

mcdenhoed commented 7 years ago

@johnthagen Any thoughts on how I broke this out (minus the obviously stand-in parts like rendering)? Not sure how idiomatic it is.

mcdenhoed commented 7 years ago

Updated the logic in 1e03596d0199d9c0646480696d0d36ebcd6460c0 to account for the fact that the canvas is a torus.

mcdenhoed commented 7 years ago

@johnthagen Check out the new push on enemy. Merged in the traits you wrote, also changed how they work a tiny bit. I feel like there has to be a better way to share more code, but this is what I have for now. Due to the architectural changes, I can't think of a clean way to feed the enemy the player's position, so it's just accelerating toward a fixed point right now.

johnthagen commented 7 years ago

@mcdenhoed First off, I love how you can see the enemies' thrusters now. So funny watching it make tons of little corrections 😄

Tiny thing, I think you accidentally checked in game.rs.bk. rustfmt makes these if you using --write-mode replace instead of --write-mode overwrite

Ah, I see what you did with the update function.

pub fn update<T: Updateable>(x: &mut T, args: UpdateArgs) {
    x.update_location();
    x.update_logic(args);
}

And then:

update(&mut self.player, args);
update(&mut self.enemy, args);

Couldn't update() be part of the Updateable trait? In fact, traits can have default implementations so you can still share this code. (See: http://rustbyexample.com/trait.html)

And then Game could just call:

player.update(args)
enemy.update(args)

I think it makes a little more sense to take &mut self than <T: Updateable>, but I see what you did there! :) In fact, I think you actually just created a trait method the manual way.

I do like the idea of having private trait methods that get called in a default trait implementation to try to reuse code.

mcdenhoed commented 7 years ago

I guess my thought was, if we do end up having large lists of things, would it be easier to call "update" on an updateable thing than to call the "update" method of an updatable thing? The second seems like an OOP pattern, and the first seemed to me to be more idiomatic. I can change that back though. Still not sure how we can cleanly get shared code for something like position updates with this system. Trait default implementations are still limited to calling other methods defined/guaranteed by the trait, so I feel like our hands are still tied there.

johnthagen commented 7 years ago

I guess my thought was, if we do end up having large lists of things, would it be easier to call "update" on an updateable thing than to call the "update" method of an updatable thing?

They really are functionally equivalent, so I think it's really just semantics. To me it seemed slightly more idiomatic to keep all of the related functionality within a single trait, but agree one is more object/type based and the other more procedural/functional.

One nice thing is that no matter which way, if we have large lists of Updateable types, we can pass them into generic functions that take <T: Updateable> (like your current implementation) and Rust will generate the vtable dispatch.

Trait default implementations are still limited to calling other methods defined/guaranteed by the trait, so I feel like our hands are still tied there.

Absolutely, the only thing I can really think of is that we have to model the problem with composition. Instead of Player and Enemy being a special type of Actor, they need to both contain an Actor and the Actor type implements the shared code on itself.

Since enemies are quite a bit more complex than the weapon #5 and asteroids #7 , maybe we should try those out first and see what kinds of shared code comes out of that.

Also, I've had good luck asking this kind of question on https://users.rust-lang.org/ so we could try that too.

mcdenhoed commented 7 years ago

I might try readding the actor code then

johnthagen commented 7 years ago

@mcdenhoed Is the enemy branch still valid at this point? Should we delete it for now, or is there still useful code in it?