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

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

We can just reexport those types from valence::inventory

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

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.

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.

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 ?

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.