Add network link to ue-server implementation #13

Open
dkanus wants to merge 23 commits from feature_link into master
Showing only changes of commit ba3ac088dd - Show all commits

View File

@ -4,11 +4,21 @@ use std::str;
extern crate custom_error;
use custom_error::custom_error;
const RECEIVED_FIELD_SIZE: usize = 4;
const LENGTH_FIELD_SIZE: usize = 4;
const HEAD_RECEIVED: u8 = 85;
const HEAD_MESSAGE: u8 = 42;
const MAX_MESSAGE_LENGTH: usize = 0xA00000;
// Defines how many bytes is used to encode "AMOUNT" field in the response from ue-server about
// amount of bytes it received since the last update
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])) }
const UE_RECEIVED_FIELD_SIZE: usize = 4;
// Defines how many bytes is used to encode "LENGTH" field, describing length of
dkanus marked this conversation as resolved Outdated

Wtf

Wtf
// next JSON message from ue-server
dkanus marked this conversation as resolved Outdated

Wtf

Wtf
const UE_LENGTH_FIELD_SIZE: usize = 4;
// Value indicating that next byte sequence from ue-server reports amount of bytes received by
// that server so far. Value itself is arbitrary.
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
const HEAD_UE_RECEIVED: u8 = 85;
// Value indicating that next byte sequence from ue-server contains JSON message.
// Value itself is arbitrary.
const HEAD_UE_MESSAGE: u8 = 42;
// Maximum allowed size of JSON message sent from ue-server.
const MAX_UE_MESSAGE_LENGTH: usize = 25 * 1024 * 1024;
custom_error! { pub ReadingStreamError
InvalidHead{input: u8} = "Invalid byte used as a HEAD: {input}",
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.
@ -37,8 +47,8 @@ pub struct MessageReader {
/// For converting byte stream expected to be generated by Acedia mod from the game server into
/// actual messages. Expected format is a sequence of either:
/// [HEAD_RECEIVED: 1 byte] [amount of bytes received by game server since last update: 4 bytes]
/// [HEAD_MESSAGE: 1 byte] [length of the message: 4 bytes] [utf8-encoded string: ??? bytes]
/// [HEAD_UE_RECEIVED: 1 byte] [amount of bytes received by game server since last update: 4 bytes]
/// [HEAD_UE_MESSAGE: 1 byte] [length of the message: 4 bytes] [utf8-encoded string: ??? bytes]
/// On any invalid input enters a failure state (can be checked by `is_broken()`) and
Review

Maybe remove continue?

Maybe remove continue?
/// never recovers from it.
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()
/// Use either `push_byte()` or `push()` to input byte stream from game server and `pop()` to
dkanus marked this conversation as resolved Outdated

with_capactiy?

with_capactiy?
@ -63,9 +73,9 @@ impl MessageReader {
}
match &self.reading_state {
ReadingState::Head => {
if input == HEAD_RECEIVED {
if input == HEAD_UE_RECEIVED {
Ggg_123 marked this conversation as resolved Outdated

<< 8
wtf

> << 8 wtf
self.reading_state = ReadingState::ReceivedBytes;
} else if input == HEAD_MESSAGE {
} else if input == HEAD_UE_MESSAGE {
self.reading_state = ReadingState::Length;
} else {
Ggg_123 marked this conversation as resolved Outdated

as u64;
wtf

> as u64; wtf
self.is_broken = true;
@ -76,7 +86,7 @@ impl MessageReader {
self.next_received_bytes = self.next_received_bytes << 8;
self.next_received_bytes += input as u32;
self.read_bytes += 1;
if self.read_bytes >= RECEIVED_FIELD_SIZE {
if self.read_bytes >= UE_RECEIVED_FIELD_SIZE {
self.received_bytes += self.next_received_bytes as u64;
self.next_received_bytes = 0;
Review

Why is it a separate function, and not inside send/flush?
When should I call it?

Why is it a separate function, and not inside send/flush? When should I call it?
self.read_bytes = 0;
@ -87,10 +97,10 @@ impl MessageReader {
self.current_message_length = self.current_message_length << 8;
self.current_message_length += input as usize;
self.read_bytes += 1;
if self.read_bytes >= LENGTH_FIELD_SIZE {
if self.read_bytes >= UE_LENGTH_FIELD_SIZE {
self.read_bytes = 0;
self.reading_state = ReadingState::Payload;
if self.current_message_length > MAX_MESSAGE_LENGTH {
if self.current_message_length > MAX_UE_MESSAGE_LENGTH {
Review

// todo - add IP support, since 0.0.0.0 is subjective (make it default, but changeable)

// todo - add IP support, since 0.0.0.0 is subjective (make it default, but changeable)
self.is_broken = true;
dkanus marked this conversation as resolved Outdated

"1 as usize" => "1usize"
Or just remove.

"1 as usize" => "1usize" Or just remove.
return Err(ReadingStreamError::MessageTooLong {
dkanus marked this conversation as resolved Outdated

Move this line below error checking, since it contains early exit

Move this line below error checking, since it contains early exit

Agreed.

Agreed.
length: self.current_message_length,
@ -143,7 +153,7 @@ impl MessageReader {
#[test]
fn message_push_byte() {
let mut reader = MessageReader::new();
reader.push_byte(HEAD_MESSAGE).unwrap();
reader.push_byte(HEAD_UE_MESSAGE).unwrap();
reader.push_byte(0).unwrap();
reader.push_byte(0).unwrap();
reader.push_byte(0).unwrap();
@ -161,7 +171,7 @@ fn message_push_byte() {
reader.push_byte(0x6c).unwrap(); // l
reader.push_byte(0x64).unwrap(); // d
reader.push_byte(0x21).unwrap(); // <exclamation mark>
reader.push_byte(HEAD_MESSAGE).unwrap();
reader.push_byte(HEAD_UE_MESSAGE).unwrap();
reader.push_byte(0).unwrap();
reader.push_byte(0).unwrap();
reader.push_byte(0).unwrap();
@ -177,19 +187,19 @@ fn message_push_byte() {
#[test]
fn received_push_byte() {
let mut reader = MessageReader::new();
reader.push_byte(HEAD_RECEIVED).unwrap();
reader.push_byte(HEAD_UE_RECEIVED).unwrap();
reader.push_byte(0).unwrap();
Ggg_123 marked this conversation as resolved Outdated

1092203255 bytes received?! wtf

1092203255 bytes received?! wtf
reader.push_byte(0).unwrap();
reader.push_byte(0).unwrap();
reader.push_byte(243).unwrap();
assert_eq!(reader.received_bytes(), 243);
reader.push_byte(HEAD_RECEIVED).unwrap();
reader.push_byte(HEAD_UE_RECEIVED).unwrap();
reader.push_byte(65).unwrap();
reader.push_byte(25).unwrap();
reader.push_byte(178).unwrap();
reader.push_byte(4).unwrap();
assert_eq!(reader.received_bytes(), 1092203255);
reader.push_byte(HEAD_RECEIVED).unwrap();
reader.push_byte(HEAD_UE_RECEIVED).unwrap();
reader.push_byte(231).unwrap();
reader.push_byte(34).unwrap();
reader.push_byte(154).unwrap();
@ -199,12 +209,12 @@ fn received_push_byte() {
#[test]
fn mixed_push_byte() {
let mut reader = MessageReader::new();
reader.push_byte(HEAD_RECEIVED).unwrap();
reader.push_byte(HEAD_UE_RECEIVED).unwrap();
reader.push_byte(0).unwrap();
reader.push_byte(0).unwrap();
reader.push_byte(0).unwrap();
dkanus marked this conversation as resolved Outdated

Replace with:
reader.push_byte(0xf3).unwrap();
...
reader.push_byte(0x41).unwrap();
reader.push_byte(0x19).unwrap();
reader.push_byte(0xb2).unwrap();
reader.push_byte(0x04).unwrap();
assert_eq!(reader.ue_received_bytes(), 0x41_19_b2_f7); // 0xf7 = 0x04 + 0xf3
Because otherwise this is "magic numbers wtf".

Replace with: reader.push_byte(0xf3).unwrap(); ... reader.push_byte(0x41).unwrap(); reader.push_byte(0x19).unwrap(); reader.push_byte(0xb2).unwrap(); reader.push_byte(0x04).unwrap(); assert_eq!(reader.ue_received_bytes(), 0x41_19_b2_f7); // 0xf7 = 0x04 + 0xf3 Because otherwise this is "magic numbers wtf".

Woa, that's neat. Agreed.

Woa, that's neat. Agreed.
reader.push_byte(243).unwrap();
reader.push_byte(HEAD_MESSAGE).unwrap();
reader.push_byte(HEAD_UE_MESSAGE).unwrap();
reader.push_byte(0).unwrap();
reader.push_byte(0).unwrap();
reader.push_byte(0).unwrap();
@ -212,7 +222,7 @@ fn mixed_push_byte() {
reader.push_byte(0x59).unwrap(); // Y
reader.push_byte(0x6f).unwrap(); // o
reader.push_byte(0x21).unwrap(); // <exclamation mark>
reader.push_byte(HEAD_RECEIVED).unwrap();
reader.push_byte(HEAD_UE_RECEIVED).unwrap();
reader.push_byte(65).unwrap();
reader.push_byte(25).unwrap();
reader.push_byte(178).unwrap();
@ -227,12 +237,12 @@ fn pushing_many_bytes_at_once() {
let mut reader = MessageReader::new();
reader
.push(&[
HEAD_RECEIVED,
HEAD_UE_RECEIVED,
0,
0,
0,
243,
HEAD_MESSAGE,
HEAD_UE_MESSAGE,
0,
0,
0,
@ -240,7 +250,7 @@ fn pushing_many_bytes_at_once() {
0x59, // Y
0x6f, // o
0x21, // <exclamation mark>
HEAD_RECEIVED,
HEAD_UE_RECEIVED,
65,
25,
178,
@ -255,7 +265,7 @@ fn pushing_many_bytes_at_once() {
#[test]
fn generates_error_invalid_head() {
let mut reader = MessageReader::new();
reader.push_byte(HEAD_RECEIVED).unwrap();
reader.push_byte(HEAD_UE_RECEIVED).unwrap();
reader.push_byte(0).unwrap();
reader.push_byte(0).unwrap();
reader.push_byte(0).unwrap();
@ -270,14 +280,16 @@ fn generates_error_invalid_head() {
#[test]
fn generates_error_message_too_long() {
let mut reader = MessageReader::new();
reader.push_byte(HEAD_MESSAGE).unwrap();
// Max length right now is `0xA00000`
reader.push_byte(0).unwrap();
reader.push_byte(0xA0).unwrap();
reader.push_byte(0).unwrap();
let huge_length = MAX_UE_MESSAGE_LENGTH + 1;
let bytes = (huge_length as u32).to_be_bytes();
reader.push_byte(HEAD_UE_MESSAGE).unwrap();
reader.push_byte(bytes[0]).unwrap();
reader.push_byte(bytes[1]).unwrap();
reader.push_byte(bytes[2]).unwrap();
assert!(!reader.is_broken());
reader
.push_byte(1)
.push_byte(bytes[3])
.expect_err("Testing failing on exceeding allowed message length");
assert!(reader.is_broken());
}
@ -285,7 +297,7 @@ fn generates_error_message_too_long() {
#[test]
fn generates_error_invalid_unicode() {
let mut reader = MessageReader::new();
reader.push_byte(HEAD_MESSAGE).unwrap();
reader.push_byte(HEAD_UE_MESSAGE).unwrap();
reader.push_byte(0).unwrap();
reader.push_byte(0).unwrap();
reader.push_byte(0).unwrap();