ReCentralize Packets

[Bafbi]

Objective

As discuss in #400

Solution

I centralize all packets and there associates structs from all crates in valence_packet.
I did not move any of the packet trait and stuff from valence_core cause I do not see the need, I see this crate only as way to get all packets in one place, for me the macro and trait should stay in valence_core. (can be discuss)
I did rework the valence_boss_bar crate because it used his ecs component in the packet struct, which was not good for moving the packets.

Upgrade

Before it get merge I'd like to :

• Create all packets (if it's not already the case)
• Document all packets (mostly by copying wiki.vg)
• Created a registry (how do that ?)

Question

I personally think that we should put all 'really minecraft related' struct in the valence_core, I explaining myself with an example :

#[derive(Copy, Clone, Debug, PartialEq, Eq, Encode, Decode)]
pub enum ClickMode {
    Click,
    ShiftClick,
    Hotbar,
    CreativeMiddleClick,
    DropKey,
    Drag,
    DoubleClick,
}

#[derive(Clone, Debug, Encode, Decode)]
pub struct SlotChange {
    pub idx: i16,
    pub item: Option<ItemStack>,
}

This two struct are currently in valence_packet::inventory:: but like I said, I personally think that this crate should only contain the packets definition, so I would like to move all this related to packets struct in there associate module in valence_core. Just like your through on this.

[dyc3]

ClickMode and SlotChange should not be moved away from the inventory packets. They are a part of the packet definitions, not just helper types.

[Bafbi]

ClickMode and SlotChange should not be moved away from the inventory packets. They are a part of the packet definitions, not just helper types.

Yes this is the thing they are not just for the packet definitions they are also helper type ;)
What I mean is that, for example :
If I want to create this method in the valence_inventory -> Inventory::click(slot: i16, click_mode: ClickMode)
The enduser will need to import valence_packet::inventory::ClickMode to use the method

But for the enduser he was not messing with any packets but just using the simple api, so for him he just import an helper type that as nothing to do with patkets (for him) from the valence_packet crate.

Hope you see what I mean

[dyc3]

We can just reexport those types from valence::inventory

[rj00a]

In addition to what dyc3 said,

Every packet should have its own file and should be organized according to its state. Like this. So we could have something like...

packets/
    handshaking/
        ...
    status/
        ...
    login/
        ...
    play/
        game_join_s2c.rs
        health_update_s2c.rs
        book_update_c2s.rs
        player_action_c2s.rs
        ...

Create all packets (if it's not already the case)

All the packets are defined, but they're just scattered all over the place.

Document all packets (mostly by copying wiki.vg)

I'm a bit hesitant to do that since the info on wiki.vg can become outdated and wrong very quickly. Incorrect documentation is arguably worse than no documentation at all.

Created a registry (how do that ?)

The packet inspector needs to know how to decode and display a packet given some information like packet ID, side, and state. So I think something like this will work:

pub struct PacketRegistry {
    packets: Vec<ErasedPacket>,
}

pub struct ErasedPacket {
    name: String,
    side: PacketSide,
    state: PacketState,
    id: i32,
    /// Decodes this packet and returns its `Debug` output.
    decode_and_debug: Box<dyn Fn(&mut &[u8]) -> anyhow::Result<String>>
}

But this work could be postponed until later.

---

Recently I had some bigger ideas for improving the project's structure. Right now I'm not really happy with the way things are organized, so here's my rough new idea:

graph TD
        subgraph valence
            schem --> server
	    player_list --> server
	    inventory --> server
	    anvil --> server
            command --> server
  	    network --> server
            weather --> server
	    advancement --> server
	    world_border --> server
	    server --> layer
	    boss_bar --> server
	    layer --> biome
	    layer --> dimension
            biome --> registry
            dimension --> registry
	    layer --> entity
            layer --> core
            entity --> protocol
            protocol --> text
            protocol --> block
            protocol --> item
            protocol --> nbt
        end

Loading

I can't tell if this is completely overengineered, but basically,

• valence_protocol now exists (again). Depends on blocks, items, text, nbt, etc. and contains all the packets, packet encoders/decoders, Packet and Encode/Decode traits and other protocol types. Not afraid to depend on bevy_ecs for #[derive(Component)] (but behind a feature flag). Can be used by other projects. Items from this can be re-exported by valence at the root when necessary.
• valence_client becomes valence_server. Its items are glob re-exported by valence and now encompasses more than just things relevant to Client when necessary. I'm also considering combining it with valence_layer since there's a ton of coupling between valence_client and valence_layer in Visibility Layers #424, but I'm trying to be careful not to let the crate size explode.
• valence_core still exists, but it's higher up the tree and has fewer items.
• Might make sense if some types from valence_entity moved into valence_protocol (perhaps valence_entity & valence_entities?). But all the generated entity structs are kept out of valence_protocol.
• Not exactly sure what the relationship of valence_registry and valence_protocol should be.

There are a lot of constraints to work with here. For instance, valence_block and valence_entity need to be in their own crate because of all the generated code.

So honestly, considering that #424 is around the corner and given the above idea, we might want to re-evaluate this pull request.

[Bafbi]

We should probably add the sound crate at the same level as block, item... cause it is needed by the packet.

Ok, so I think that I will just organize all the packets then I leave it that way.

[Bafbi]

Ok so I'v just created the modules and didn't bother changing any import if this will not be merge, you will just have to copy the folder from this branch.
Tell me if I should do anything else here ?

[rj00a]

This is good, thanks for working on this. If you could finish it up that would be appreciated. I'll worry about organizing more after this is merged.

Also the packet modules should re-export the packet structs, e.g.

pub mod handshake_c2s;

pub use handshake_c2s::HandshakeC2s;

Then you can glob import the packets from the packet inspector build script.

[Bafbi]

We have so many thing that depend on the protocol module, the only thing I can move is the Packet trait, the thing is that even crate like valence_block depend on it, so we either don't impl Encode / Decode trait on thought crate but in valence_protocol or valence_protocol is the lowerst crate.

[rj00a]

Looks good, just need to update the branch.