hematite_server icon indicating copy to clipboard operation
hematite_server copied to clipboard

First steps towards single player support

Open toqueteos opened this issue 9 years ago • 12 comments

I've been working on the single player support for hematite_server. I created the PR so we can comment on lines, there's stuff happening just not that visible.

  • [x] Rust v1.5
  • [ ] rustfmt
    • Unfortunately it causes a bit of a mess, we won't use it for now
  • [x] Documentation:
    • [ ] Whatever gets added has docs
    • [ ] NON-Goal. Docs for everything I can catch
  • [x] List of changes:
    • [x] Got rid of the BLOCK OF SHAME! :tada:
    • [x] Keep Alive based on channels as https://github.com/RichardlL/Tunnul does!
    • [x] First steps towards supporting single player #97
    • [x] serverbound::KeepAlive was i32 instead of Var<i32>.
    • [x] clientbound::PlayerAbilities being sent twice, with bad params
    • [x] Gamemode set to survival by default, instead of adventure.

/cc @fenhl @RichardlL

toqueteos avatar Dec 15 '15 00:12 toqueteos

What's with the randomly indented line continuations everywhere?

fenhl avatar Dec 15 '15 02:12 fenhl

Wow, Someones getting work done!

I've was pretty sick last week, but right now I'm working on stuff for nearly implicit type conversion and packet sending. Also a very cool way of having chunks use _Much_ less memory, and near none compression cost (cost less to compress than not) for sending chunks to the client. No promises, but stay tuned :)

I'll review more later, but my initial comments are:

  • Generating a random number for keep alive is useless, as we cant check what it is,(there is no point in having it random anyway) so I just use 0 to de-clutter the code/ dependencies
  • The way I'm planning on checking if the client is responding (keep alive resonse)
    • is having the timeout on the TcpStream's read in the general game packet handling loop to 30 seconds (Default keepalive timeout)
    • if the read fails (times out/ stream died, doing it this way handles all cases of disconnect)
      • Call shutdown method on the client's tcpstream to "notify" the keepalive thread, in case it was do to timout
      • handle player disconnect / saving / Saying "Player disconnected", etc

On another note, Why Single Player first? It seems that it would be much easier to implement Multi -> single than the other way around.

Also, was it @fenhl who wanted to avoid encoding/decoding during single player?

I haven't done much research into it, but it seems an easy way to do this is to:

  • Compile two server binaries, one single-player and one multiplayer
  • Have an environmental variable to switch which modules are used, single or multiplayer, but they are named the same;

E.G.

if mode == multi_player {
     mod multi_player::types::{string,chat, etc};
     use multi_player::types::{string,chat, etc};
} else {
     mod single_player::types::{string,chat, etc};
     use single_player::types::{string,chat, etc};
}
...
let string = string::convert_string();

This way nothing would have to be reimplemented

For most single player statically sized types, it would just look like this

fn convert_from_minecraft_format(input: Type) -> Type {
    return input;
}

And with Link time optimization enabled the entire function would just be removed

Abnormalities like Varints just be transmuting from a fixed size byte array of 4 (size of i32) , Strings could just be prefixed with i32 rather than a varint and transmuted, (which is also free!), etc.

RichardlL avatar Dec 15 '15 04:12 RichardlL

Single player it's just the name, this branch right now allows more than one player to log in and do its thing but because we lack proper entity handling they won't see each other (I'm not actually sure if they can see or not, I need to try this). There are no protocol differences between single and multiplayer, at least that I'm aware of.

Generating a random number for keep alive is useless, as we cant check what it is,(there is no point in having it random anyway) so I just use 0 to de-clutter the code/ dependencies

Right now it's outputting the number and we should check it because that's what the protocol spec states, that KeepAlive number ids should match. We can always hardcode the value or make it something incremental (this is actually a great idea which might be useful in the future for reproducing bugs).

toqueteos avatar Dec 15 '15 11:12 toqueteos

@fenhl

What's with the randomly indented line continuations everywhere?

That must be rustfmt. Can you give me a specific line?

toqueteos avatar Dec 15 '15 12:12 toqueteos

WIP commit is gone! Still lots of stuff left to do :tada:

I improved Server::keep_alive_loop so it checks multiple times for sends on the TcpStream channel and then sends all the KeepAlive packets in batch.

EDIT: Tests currently failing because of #[deny(unused_assignments)]. If I don't assign it a default value it also warns so... Maybe doing it in a more functional way will solve it... thinking about it

EDIT2: Duh, solved it. I was doing a stupid assigment which was totally unnecessary.

EDIT3: Fixed stats vec! indentation.

toqueteos avatar Dec 16 '15 21:12 toqueteos

@toqueteos line 67 in src/packet.rs, for example.

Edit: also the packet macros.

fenhl avatar Dec 16 '15 22:12 fenhl

@fenhl Yep, that's rustfmt. It can be solved by declaring packets first and then writing them instead of calling try!(Packet{ ... }).write(&mut stream)

toqueteos avatar Dec 17 '15 08:12 toqueteos

I'd rather avoid using rustfmt as long as it produces this kind of output.

fenhl avatar Dec 17 '15 13:12 fenhl

@fenhl Could you find a suitable rustfmt config so we don't ditch it entirely? The purpose of rustfmt is getting rid of styling discussions and having uniform code style all over a project is a good thing.

toqueteos avatar Dec 18 '15 17:12 toqueteos

@toqueteos I haven't used rustfmt yet, but I doubt this exists given the tool's notoriously bad macro handling. At this point, I think we can still manage style manually. In fact, I might open a style review PR soon.

fenhl avatar Dec 18 '15 21:12 fenhl

@fenhl If it doesn't exist, create one! It's simple. Crate a rustfmt.toml in the project root and tune it as you like. rustfmt --config-help shows you the current configurable options.

toqueteos avatar Dec 18 '15 21:12 toqueteos

I'm working on the un-rustfmt-ization and finishing #76

toqueteos avatar Dec 19 '15 01:12 toqueteos

It's been 6 years. I'm closing this. If anybody wants to continue this, feel free to do it.

toqueteos avatar Jul 11 '23 13:07 toqueteos