Skip to content
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

Merged
merged 5 commits into from
May 15, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion schema/world.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,3 @@ table WorldState {
player: Tank;
others: [Tank];
}

32 changes: 15 additions & 17 deletions server/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
mod math;
mod serialization;
mod world;
mluogh marked this conversation as resolved.
Show resolved Hide resolved

use std::collections::{HashSet, VecDeque};
use std::net::SocketAddr;
use std::time;
Expand All @@ -10,7 +14,10 @@ use log::{debug, info, warn};
use tungstenite::Message;

use schema::actions_generated::get_root_as_action_root;
use schema::{messages_generated, world_generated};

use math::Position;
use serialization::{Config, Serializable};
use world::{Tank, World};

type Peers = Arc<Mutex<HashSet<SocketAddr>>>;
type Buffer = Vec<u8>;
Expand Down Expand Up @@ -130,22 +137,13 @@ async fn run() -> anyhow::Result<()> {
});

let mut builder = flatbuffers::FlatBufferBuilder::new_with_capacity(1024);
let world = world_generated::WorldState::create(
&mut builder,
&world_generated::WorldStateArgs {
player: None,
others: None,
},
);
let message = messages_generated::MessageRoot::create(
&mut builder,
&messages_generated::MessageRootArgs {
message_type: messages_generated::Message::WorldState,
message: Some(world.as_union_value()),
},
);
builder.finish(message, None);
let world = Arc::new(builder.finished_data().to_vec());
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 = Arc::new(world.serialize(&mut builder, &Config::new(0)));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The maintained state should be the rust WOrld struct, not the Buffer itself. We modify the World struct during processing and only serialize into message when we broadcast to clients

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this to interoperate into the existing code as easily as possible. We can start using a real World in a future pr.


// Listen for new WebSocket connections.
while let Ok((stream, address)) = tcp_listener.accept().await {
Expand Down
5 changes: 5 additions & 0 deletions server/src/math.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#[derive(Debug)]
pub struct Position {
pub x: f32,
pub y: f32,
}
166 changes: 166 additions & 0 deletions server/src/serialization.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
use flatbuffers::{FlatBufferBuilder, ForwardsUOffset, Vector, WIPOffset};

use schema::math_generated;
use schema::messages_generated;
use schema::world_generated;

use log::error;
mluogh marked this conversation as resolved.
Show resolved Hide resolved

use crate::world::{Tank, World};

pub trait Serializable {
mluogh marked this conversation as resolved.
Show resolved Hide resolved
fn serialize(&self, builder: &mut FlatBufferBuilder, config: &Config) -> Vec<u8>;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Serializable takes a flatbuffer because we allocating a new FlatbufferBuilder every time we call this function is extremely inefficient.

I know this is not the API we discussed, but I think this is a valid price to pay.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not take in a builder. You clear the builder anyway, and the caller of the API should not need to construct a builder

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reset() does not clear owned buffer from FlatBufferBuilder. It will reuse this buffer.

google/flatbuffers#5135

}

trait Flatbufferable<'a> {
type Buffer;
Copy link
Owner

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

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bump

fn add_to_fb(
&self,
builder: &mut FlatBufferBuilder<'a>,
config: &Config,
) -> WIPOffset<Self::Buffer>;
}

pub struct Config {
pub player: u16,
mluogh marked this conversation as resolved.
Show resolved Hide resolved
}

impl Config {
pub fn new(player: u16) -> Config {
Config { player }
}
}

impl<'a> Flatbufferable<'a> for Tank {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's replace the lifetime with 'buffer

Copy link
Owner

Choose a reason for hiding this comment

The 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_ref().x,
self.pos_ref().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 Serializable for World {
fn serialize(&self, builder: &mut FlatBufferBuilder, config: &Config) -> Vec<u8> {
mluogh marked this conversation as resolved.
Show resolved Hide resolved
builder.reset();

let (player, other_tanks): (Vec<&Tank>, Vec<&Tank>) = self
.tanks_ref()
.iter()
.partition(|tank| tank.player() == config.player);

if player.len() != 1 {
error!("Could not find player in World.");
return Vec::new();
}

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);
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_ref().x);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use results instead of unwrap

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pos() returns an Option, not Result.

At any rate, an unwrap() in a test is not a big deal, crashing will cause an error which is what we want.

assert_eq!(recovered_tank.pos().unwrap().y(), tank.pos_ref().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);

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_ref();

assert_eq!(player.pos().unwrap().x(), tanks[0].pos_ref().x);
assert_eq!(player.pos().unwrap().y(), tanks[0].pos_ref().y);

assert_eq!(others.len(), 2);
assert_eq!(others.get(0).pos().unwrap().x(), tanks[1].pos_ref().x);
assert_eq!(others.get(0).pos().unwrap().y(), tanks[1].pos_ref().y);
assert_eq!(others.get(1).pos().unwrap().x(), tanks[2].pos_ref().x);
assert_eq!(others.get(1).pos().unwrap().y(), tanks[2].pos_ref().y);
}
}
40 changes: 40 additions & 0 deletions server/src/world.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
use crate::math::Position;

pub struct World {
tanks: Vec<Tank>,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this not have dimensions

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary for v0.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite necessary @yizzlez

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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_ref(&self) -> &Position {
mluogh marked this conversation as resolved.
Show resolved Hide resolved
&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_ref(&self) -> &Vec<Tank> {
mluogh marked this conversation as resolved.
Show resolved Hide resolved
&self.tanks
}
}