Vector Module #102
Vector Module #102
Conversation
src/game/models/mod.rs
Outdated
|
|
||
| use opengl_graphics::GlGraphics; | ||
| use piston_window::{Context, UpdateArgs, Size}; | ||
| use rand; | ||
| use piston_window::{Context, UpdateArgs}; | ||
|
|
||
| pub mod player; |
There was a problem hiding this comment.
Minor, but I try to alphabetize lists in general to keep things easy to scan through and find. Let's do that with the pub mod's here too.
There was a problem hiding this comment.
Since these were already out of order before this PR, should I do it in a separate one, or do it in here? Eh, I'll do it in here.
There was a problem hiding this comment.
Yeah let's just do it here, but you're right that this was an oversight from (probably me) from before.
| @@ -0,0 +1,144 @@ | |||
| use rand; | |||
There was a problem hiding this comment.
Let's use the standard grouping for imports: rust-lang/style-team#24 (comment)
Three sections, with one blank line after each:
- Standard library imports
- External crate imports
- Internal imports
So in this instance, rand would be imported after the std::'s and alphabetically after piston_window in the "3rd party" group.
There was a problem hiding this comment.
Ah, ok. I had thought that rand was provided by rust, but wasn't in std, and was trying to alphabetize it. I'll move it down.
| @@ -6,7 +6,8 @@ use piston_window::{Context, polygon, Size, Transformed, UpdateArgs}; | |||
| use rand; | |||
|
|
|||
| use super::super::color; | |||
| use super::{Collidable, Drawable, PI_MULT_2, Positioned, Updateable, Vector}; | |||
| use super::{Collidable, Drawable, PI_MULT_2, Positioned, Updateable}; | |||
There was a problem hiding this comment.
Not needed for this PR, but I think we should consider ditching all of the super::'s and just use absolute imports for everything.
There was a problem hiding this comment.
Seems like a good idea.
src/game/models/mod.rs
Outdated
|
|
||
| pub mod player; | ||
| pub mod bullet; | ||
| pub mod asteroid; | ||
| pub mod vector; |
There was a problem hiding this comment.
vector does not actually need to be pub at this time.
src/game/models/mod.rs
Outdated
|
|
||
| pub mod player; | ||
| pub mod bullet; | ||
| pub mod asteroid; | ||
| pub mod vector; | ||
|
|
||
| pub const PI_MULT_2: f64 = 2.0 * PI; |
There was a problem hiding this comment.
PI_MULT_2 doesn't need to be pub at this time.
|
Restarting AppVeyor. Seems |
|
To fix AppVeyor we should just replace the |
| @@ -2,156 +2,16 @@ | |||
|
|
|||
| use std::f64; | |||
There was a problem hiding this comment.
This import isn't actually used. (Why doesn't rustc warn about this case?)
There was a problem hiding this comment.
Oh yeah, that is true. Maybe it doesn't care because it's going to be linked into the final executable anyway?
There was a problem hiding this comment.
I wonder if it's because we use use std::f64::consts::PI; below. This feels bug report-worthy to me.
There was a problem hiding this comment.
Weird, this program does warn. I wonder if the import checker more globally to your crate?
use std::f64; // warning: unused import: `std::f64`, #[warn(unused_imports)] on by default
use std::f64::consts::PI;
fn main() {
println!("PI {}", PI);
}There was a problem hiding this comment.
Oh... wow. It's because you are actually importing the type f64??
use std::f64; // No warning.
fn main() {
let x: f64 = 0.0;
println!("{}", x);
}
mcdenhoed
force-pushed
the
vector-module
branch
from
2071539
to
f24815e
Compare
Self-explanatory. Implements #101.