-
Notifications
You must be signed in to change notification settings - Fork 4
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
Initial concepts of type safe API #2
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Класс 👍
src/player.rs
Outdated
pub struct Player(*mut sys::Player); | ||
|
||
impl Player { | ||
pub fn get_name(&self) -> BwString { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Зачем BwString?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Пока не могу внятно ответить, разбираюсь с интерфейсами и пытаюсь осознать, какие контракты нам будут нужны.
Если сходу конвертировать в String будет еще одна аллокация. Вообще можно использовать OsString который является промежуточным звеном между сырыми строками и строками Rust, и может быть дешево преобразован в обе стороны при условии, что внутри лежит UTF-8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Дим, наверно пофиг на доп аллокацию? get_name не должны спрашивать чаще 16 раз за такт
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не то что бы это было критично. Просто зачем?
Я реализовал Deref<Target=str>
и AsRef<str>
для этого типа, то есть его можно спокойно подставлять в те функции, где ожидается &str
. Если же потребуется владение, то или x.to_owned()
или String::from(x)
.
src/string.rs
Outdated
} | ||
} | ||
|
||
pub fn data(&self) -> &CStr { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Зачем тут такие кишки?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В тесте показаны примеры использования. А вообще все будет зависеть от того, как их потом использовать.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если строку придется отдавать обратно, например в Game::sendText
то лучше иметь возможность получить доступ к оригинальному буферу. Это преобразование дешевое. Семантически, CStr
это слайс сишной строки.
pub fn distance_to<T>(&self, t: &T) -> i32 | ||
where T: HasPosition | ||
{ | ||
unsafe { sys::Unit_getDistance_Position(self.raw, t.position()) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the difference in implementation:
int UnitInterface::getDistance(Position target) const
// retrieve left/top/right/bottom values for calculations
int left = target.x - 1;
int top = target.y - 1;
int right = target.x + 1;
int bottom = target.y + 1;
vs
int UnitInterface::getDistance(Unit target) const
// retrieve left/top/right/bottom values for calculations
int left = target->getLeft() - 1;
int top = target->getTop() - 1;
int right = target->getRight() + 1;
int bottom = target->getBottom() + 1
Where
int UnitInterface::getLeft() const
{
return this->getPosition().x - this->getType().dimensionLeft();
}
You can run `cargo test --target=i686-pc-windows-gnu --lib` for now
Are the |
Yes, it is a known problem that need to be solved. Regression appeared in between |
635df27
to
d452b00
Compare
No description provided.