Add network link to ue-server implementation #13

Open
dkanus wants to merge 23 commits from feature_link into master
2 changed files with 61 additions and 9 deletions
Showing only changes of commit 26b7d1dd91 - Show all commits

View File

@ -1,4 +1,60 @@
use std::error::Error;
use std::io::{Read, Write};
use std::net::{SocketAddr, TcpListener};
use std::sync::Arc;
use std::{str, thread};
mod reader;
mod writer;
dkanus marked this conversation as resolved Outdated

Wtf

Wtf

replace with array[u8;4] and
pub fn array_of_u8_to_u32(bytes: [u8; 4]) -> u32 {
(u32::from(bytes[0]) << 24)
+ (u32::from(bytes[1]) << 16)
+ (u32::from(bytes[2]) << 8)
+ (u32::from(bytes[3]))
}

replace with array[u8;4] and pub fn array_of_u8_to_u32(bytes: [u8; 4]) -> u32 { (u32::from(bytes[0]) << 24) + (u32::from(bytes[1]) << 16) + (u32::from(bytes[2]) << 8) + (u32::from(bytes[3])) }
pub use reader::MessageReader;
pub use writer::MessageWriter;
dkanus marked this conversation as resolved Outdated

Wtf

Wtf
dkanus marked this conversation as resolved Outdated

Wtf

Wtf
pub struct Link {
reader: MessageReader,
writer: MessageWriter,
dkanus marked this conversation as resolved Outdated

Remove " Value itself is arbitrary", change to "Arbitrary value indicating that ..."?

Remove " Value itself is arbitrary", change to "Arbitrary value indicating that ..."?
}
dkanus marked this conversation as resolved Outdated

Weird empty line

Weird empty line
impl Link {
pub fn run<F>(port: u16, handler: F) -> Result<(), Box<dyn Error>>
where
F: Fn(&mut Link, &str) -> () + Send + Sync + 'static,
{
let address = SocketAddr::from(([0, 0, 0, 0], port));
let listener = TcpListener::bind(address)?;
let handler = Arc::new(handler);
dkanus marked this conversation as resolved Outdated

with_capacity != limit.
Should it really be a constant?
I'd just inline it.

with_capacity != limit. Should it really be a constant? I'd just inline it.

It is expected limit that we use as a capacity.

It is expected limit that we use as a capacity.

But upon further consideration I agree that there is no sense in defining this as a constant.

But upon further consideration I agree that there is no sense in defining this as a constant.
loop {
// Listen to new (multiple) connection

Can be simplified => // Listen to new connections

Can be simplified => // Listen to new connections
let mut reading_stream = listener.accept()?.0;
Ggg_123 marked this conversation as resolved
Review

Probably needs a timeout, to avoid waiting forever:
stream
.set_read_timeout(Some(Duration::from_secs(5)))
.unwrap();

Probably needs a timeout, to avoid waiting forever: stream .set_read_timeout(Some(Duration::from_secs(5))) .unwrap();
let mut writing_stream = reading_stream.try_clone()?;
let mut avarice_link = Link {
reader: MessageReader::new(),
writer: MessageWriter::new(),
};
let handler_clone = handler.clone();
// On connection - spawn a new thread
dkanus marked this conversation as resolved Outdated

Unclear meaning.

Unclear meaning.

rename to ue_received_bytes

rename to ue_received_bytes
thread::spawn(move || loop {
Review

sent next message => sent this message?

sent next message => sent this message?
let mut buffer = [0; 1024];

1024? Not 4096?

1024? Not 4096?

4096 is limitation on how much we can write, it's unrelated to reading and this is an arbitrary constant. Honestly I don't know what to use, but one option is to add BufReader and read byte-by-byte.

4096 is limitation on how much we can write, it's unrelated to reading and this is an arbitrary constant. Honestly I don't know what to use, but one option is to add `BufReader` and read byte-by-byte.

Did some tests with ue-server. As of now, buffer size does not make a difference in speed, since bottleneck is ue-server's side by far. And it's unlikely that situation will change even with multiple ue-servers connecting to Avarice.

Did some tests with ue-server. As of now, buffer size does not make a difference in speed, since bottleneck is ue-server's side by far. And it's unlikely that situation will change even with multiple ue-servers connecting to Avarice.
// Reading cycle
Review

Confusing name, since it's not related to https://doc.rust-lang.org/std/iter/trait.Iterator.html#tymethod.next, which is very common.

Confusing name, since it's not related to https://doc.rust-lang.org/std/iter/trait.Iterator.html#tymethod.next, which is very common.
match reading_stream.read(&mut buffer) {
dkanus marked this conversation as resolved Outdated

"For converting byte stream expected to be generated"?

"For converting byte stream expected to be generated"?
Ok(n) => avarice_link.reader.push(&buffer[..n]).unwrap(),
_ => panic!("Connection issue!"),
dkanus marked this conversation as resolved Outdated

4 bytes
u32? i32? BEi32? LEi32?

> 4 bytes u32? i32? BEi32? LEi32?
};
// Handling cycle
while let Some(message) = avarice_link.reader.pop() {
handler_clone(&mut avarice_link, &message);
dkanus marked this conversation as resolved Outdated

buffer => length_buffer, since it's used only for storing length bytes

buffer => length_buffer, since it's used only for storing length bytes

Agreed

Agreed
}
// Writing
avarice_link
.writer
.update_ue_received_bytes(avarice_link.reader.ue_received_bytes());
if let Some(bytes) = avarice_link.writer.try_pop() {
writing_stream.write_all(&bytes).unwrap();
}
Review

Maybe remove continue?

Maybe remove continue?
});
dkanus marked this conversation as resolved Outdated

with_capactiy?

with_capactiy?
  • // will be recreated with with_capacity in push()
+ // will be recreated with with_capacity in push()
}
dkanus marked this conversation as resolved Outdated

with_capactiy?

with_capactiy?
}
pub fn write(&mut self, message: &str) {
self.writer.push(message);
}
}

View File

@ -1,13 +1,9 @@
use std::env;
use std::path::Path;
mod link;
mod unreal_config;
use link::Link;
fn main() {
let args: Vec<String> = env::args().collect();
let filename = &args[1];
let config = unreal_config::load_file(Path::new(filename));
if let Ok(config) = config {
print!("{}", config);
}
match Link::run(1234, |link, message| { link.write(message);}) {
Ok(_) => print!("Connect!"),
_ => print!("Connection error!"),
};
}