Some Position utility methods for approximate offsets #243
Conversation
daboross
force-pushed
the
approximate-offset-position-methods
branch
from
6a262a8 to
7f9f0d3
Compare
There was a problem hiding this comment.
The documentation needs some refining.
As to whether to include this in the api... I'd say "no", but it's very useful 😂 It's definitely not just an interface to screeps in js. This opens the door to more code that makes life easier. As the main author, it's your call.
If we're not sure, maybe keep a separate branch from master that has goodies included (although I feel we'll eventually just merge it with master anyway).
| use super::Position; | ||
|
|
||
| impl Position { | ||
| /// Calculates an approximate midpoint between this point and the target. |
There was a problem hiding this comment.
This is not a midpoint (as in, it's not in the middle). It's distance_towards_target from this point towards the other point. (with big caveats)
There was a problem hiding this comment.
Also, this line is suspiciously like the first one of the next function 😛
There was a problem hiding this comment.
Maybe: midpoint -> offset
| /// | ||
| /// If `distance_from_target` is bigger than the distance to the target, | ||
| /// this position is returned. | ||
| pub fn between<T>(self, target: &T, distance_from_target: i32) -> Position |
There was a problem hiding this comment.
Given that the other is toward, wouldn't this be more understandable as from?
| /// Calculates an approximate midpoint between this point and the target. | ||
| /// | ||
| /// In case of a tie, rounds towards the target. | ||
| pub fn midpoint_between<T>(self, target: &T) -> Position |
There was a problem hiding this comment.
suggestion: midpoint_between -> midpoint_to
It reads better.
| self + (new_offset_x, new_offset_y) | ||
| } | ||
|
|
||
| /// Calculates an approximate midpoint between this point and the target. |
| .map(|s| s.parse().unwrap()) | ||
| } | ||
|
|
||
| fn pos(room: RoomName, x: u32, y: u32) -> Position { |
There was a problem hiding this comment.
use super::Position::new as pos;? 😛
|
Sorry I've just kind of left this. Really glad to have your review, and once I have the time to be more active again, I'll be fixing the things! (and possibly just sending the code into a separate crate anyways...) |
|
Just for cleanup's sake. |
Just some methods which could be useful?
I'm on the edge for whether these do belong in this library, but if they're useful to people they shouldn't hurt too much.
If not, maybe now would be a good time to add a "position_utils" addon library?