-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add serialiable and flatbufferable representations of World and Tank #27
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,4 +17,3 @@ table WorldState { | |
player: Tank; | ||
others: [Tank]; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
#[derive(Clone, Copy, Debug)] | ||
pub struct Position { | ||
pub x: f32, | ||
pub y: f32, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,169 @@ | ||
use anyhow::{anyhow, Result}; | ||
use flatbuffers::{FlatBufferBuilder, ForwardsUOffset, Vector, WIPOffset}; | ||
use log::error; | ||
|
||
use schema::math_generated; | ||
use schema::messages_generated; | ||
use schema::world_generated; | ||
|
||
use crate::world::{Tank, World}; | ||
|
||
pub trait SerializableAsMessage { | ||
fn serialize(&self, builder: &mut FlatBufferBuilder, config: &Config) -> Result<Vec<u8>>; | ||
} | ||
|
||
trait Flatbufferable<'a> { | ||
type Buffer; | ||
fn add_to_fb( | ||
&self, | ||
builder: &mut FlatBufferBuilder<'a>, | ||
config: &Config, | ||
) -> WIPOffset<Self::Buffer>; | ||
} | ||
|
||
pub struct Config { | ||
pub player_id: u16, | ||
} | ||
|
||
impl Config { | ||
pub fn new(player_id: u16) -> Config { | ||
Config { player_id } | ||
} | ||
} | ||
|
||
impl<'a> Flatbufferable<'a> for Tank { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's replace the lifetime with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bump |
||
type Buffer = world_generated::Tank<'a>; | ||
#[allow(unused_variables)] | ||
fn add_to_fb( | ||
&self, | ||
builder: &mut FlatBufferBuilder<'a>, | ||
config: &Config, | ||
) -> WIPOffset<world_generated::Tank<'a>> { | ||
world_generated::Tank::create( | ||
builder, | ||
&world_generated::TankArgs { | ||
pos: Some(&math_generated::Vec2::new(self.pos().x, self.pos().y)), | ||
}, | ||
) | ||
} | ||
} | ||
|
||
// TODO: 1) see if it's possible to add a test for this. Currently it's hard because you can't | ||
// get_root for an array. Perhaps make a testing schema that just has the [Tank] field. | ||
// 2) See if we can simplify for all Vec<&T> where T is Flatbufferable. Currently difficult because | ||
// each T has its own associated Buffer type. | ||
impl<'a> Flatbufferable<'a> for Vec<&Tank> { | ||
type Buffer = Vector<'a, ForwardsUOffset<world_generated::Tank<'a>>>; | ||
fn add_to_fb( | ||
&self, | ||
builder: &mut FlatBufferBuilder<'a>, | ||
config: &Config, | ||
) -> WIPOffset<Vector<'a, ForwardsUOffset<world_generated::Tank<'a>>>> { | ||
let mut vec = Vec::new(); | ||
|
||
for tank in self.iter() { | ||
vec.push(tank.add_to_fb(builder, config)); | ||
} | ||
|
||
builder.create_vector(vec.as_slice()) | ||
} | ||
} | ||
|
||
impl SerializableAsMessage for World { | ||
fn serialize(&self, builder: &mut FlatBufferBuilder, config: &Config) -> Result<Vec<u8>> { | ||
builder.reset(); | ||
|
||
let (player, other_tanks): (Vec<&Tank>, Vec<&Tank>) = self | ||
.tanks() | ||
.iter() | ||
.partition(|tank| tank.player() == config.player_id); | ||
|
||
if player.len() != 1 { | ||
error!( | ||
"There are {} players with id {}. Can't serialize world.", | ||
player.len(), | ||
config.player_id | ||
); | ||
mluogh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return Err(anyhow!( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's add a TODO to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are using anyhow. Using thiserror contradicts this. https://old.reddit.com/r/rust/comments/dfs1zk/2019_q4_error_patterns_snafu_vs_errderive_anyhow/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like treating serialization module as own library because it directly interfaces with our game logic. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sire you misunderstand. anyhow is the preference for application code, and thiserror is the preference for library code. Here, you have written a nice serialization library. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can table this, it's interfering with velocity. But if the server ever needs different handling for each serialization error type, then we will need to define error types. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See second comment. The serialization module is not much of a library since it will have a lot of game logic (e.g. sorting between teammates and enemies when serializing tank). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No other application can use our serialization "library" other than ours. |
||
"Config does not specify unique player; can't serialize for schema." | ||
)); | ||
} | ||
|
||
let player = player.get(0).unwrap().add_to_fb(builder, config); | ||
mluogh marked this conversation as resolved.
Show resolved
Hide resolved
mluogh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let other_tanks = other_tanks.add_to_fb(builder, config); | ||
|
||
let world = world_generated::WorldState::create( | ||
builder, | ||
&world_generated::WorldStateArgs { | ||
player: Some(player), | ||
others: Some(other_tanks), | ||
}, | ||
); | ||
|
||
let message = messages_generated::MessageRoot::create( | ||
builder, | ||
&messages_generated::MessageRootArgs { | ||
message_type: messages_generated::Message::WorldState, | ||
message: Some(world.as_union_value()), | ||
}, | ||
); | ||
|
||
builder.finish(message, None); | ||
Ok(builder.finished_data().to_vec()) | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
use crate::math::Position; | ||
use flatbuffers::get_root; | ||
|
||
#[test] | ||
fn tank_can_be_flatbuffered() { | ||
let config = Config::new(0); | ||
let mut builder = flatbuffers::FlatBufferBuilder::new_with_capacity(1024); | ||
let tank = Tank::new(0, Position { x: 69.0, y: 420.0 }); | ||
let tank_buf = tank.add_to_fb(&mut builder, &config); | ||
builder.finish(tank_buf, None); | ||
|
||
let recovered_tank = get_root::<world_generated::Tank>(builder.finished_data()); | ||
|
||
assert_eq!(recovered_tank.pos().unwrap().x(), tank.pos().x); | ||
assert_eq!(recovered_tank.pos().unwrap().y(), tank.pos().y); | ||
} | ||
|
||
#[test] | ||
fn world_can_be_serialized() { | ||
let config = Config::new(0); | ||
let mut builder = flatbuffers::FlatBufferBuilder::new_with_capacity(1024); | ||
|
||
let mut world = World::new(); | ||
|
||
world.add_tank(Tank::new(0, Position { x: 69.0, y: 420.0 })); | ||
world.add_tank(Tank::new(1, Position { x: 23.0, y: 54.0 })); | ||
world.add_tank(Tank::new(2, Position { x: 84.0, y: 34.0 })); | ||
|
||
let world_as_bytes = world.serialize(&mut builder, &config).unwrap(); | ||
|
||
let message = get_root::<messages_generated::MessageRoot>(world_as_bytes.as_ref()); | ||
assert_eq!( | ||
message.message_type(), | ||
messages_generated::Message::WorldState | ||
); | ||
let recovered_world = message.message_as_world_state().unwrap(); | ||
|
||
let player = recovered_world.player().unwrap(); | ||
let others = recovered_world.others().unwrap(); | ||
let tanks = world.tanks(); | ||
|
||
assert_eq!(player.pos().unwrap().x(), tanks[0].pos().x); | ||
assert_eq!(player.pos().unwrap().y(), tanks[0].pos().y); | ||
|
||
assert_eq!(others.len(), 2); | ||
assert_eq!(others.get(0).pos().unwrap().x(), tanks[1].pos().x); | ||
assert_eq!(others.get(0).pos().unwrap().y(), tanks[1].pos().y); | ||
assert_eq!(others.get(1).pos().unwrap().x(), tanks[2].pos().x); | ||
assert_eq!(others.get(1).pos().unwrap().y(), tanks[2].pos().y); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
use crate::math::Position; | ||
|
||
pub struct World { | ||
tanks: Vec<Tank>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this not have dimensions There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not necessary for v0. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is quite necessary @yizzlez There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? There are no boundaries restricting movement in v0. |
||
} | ||
|
||
pub struct Tank { | ||
// TODO: discuss what player would look like and replace. | ||
player: u16, | ||
pos: Position, | ||
} | ||
|
||
impl Tank { | ||
pub fn player(&self) -> u16 { | ||
self.player | ||
} | ||
|
||
pub fn pos(&self) -> Position { | ||
self.pos | ||
} | ||
|
||
pub fn new(player: u16, pos: Position) -> Tank { | ||
Tank { player, pos } | ||
} | ||
} | ||
|
||
impl World { | ||
pub fn new() -> World { | ||
World { tanks: Vec::new() } | ||
} | ||
|
||
// TODO: replace with add_player after discussion on representing player. | ||
pub fn add_tank(&mut self, tank: Tank) { | ||
self.tanks.push(tank); | ||
} | ||
|
||
pub fn tanks(&self) -> &Vec<Tank> { | ||
&self.tanks | ||
} | ||
} |
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.
The type isn't exactly the Buffer, it's the object type rather
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.
Bump