valence-rs / valence

A Rust framework for building Minecraft servers.
http://valence.rs/
MIT License
2.71k stars 143 forks source link

Merge `valence` and `valence_protocol` #308

Closed rj00a closed 1 year ago

rj00a commented 1 year ago

Describe the problem related to your feature request.

The current valence_protocol architecture has felt somewhat flawed and messy. I have identified a few reasons for this. It is for these reasons that I believe a general-purpose protocol crate that covers everyone's use case is not really feasible.

What solution would you like?

What alternative(s) have you considered?

There are some problems that may make us want to avoid doing this at all:

Additional context

misterghast commented 1 year ago

How exactly would a server that uses Valence even function without some kind of access to the protocol/packets? It just makes sense to merge them, honestly. I can't think of a use case that wants one but not the other.

AviiNL commented 1 year ago

How exactly would a server that uses Valence even function without some kind of access to the protocol/packets? It just makes sense to merge them, honestly. I can't think of a use case that wants one but not the other.

The packet inspector for one only uses protocol without the need for valence itself. So proxying would be a valid use case, Although I do agree that it would probably fit better in the main project. Probably making things a lot easier in the long run.

dyc3 commented 1 year ago

The reasons you've presented here are compelling to merge valence and valence_protocol. However, I do not think that that is a good idea given the current state of the rust compiler. I have personally noticed a fantastic boon to productivity since the separation of valence and valence protocol because of the caching tnat is now possible.

We should most certainly have a central crate to define the structure of all Minecraft protocol packets. It will save a ton of dev time compiling in general, because caching. There is a reason that bevy has their packages separated as they currently do, and I think there is merit in that. Compile times do matter, and I audibly groan whenever valence protocol shows up when I'm compiling.

Ultimately, what I think we should focus on is to define what exactly should valance protocol cover.

rj00a commented 1 year ago

I didn't really make this clear, but I'm not suggesting that we keep everything in a single crate. What I'm trying to do here is make valence's modules self-contained so we can draw clear divisions between them. This includes the relevant packets used in each module.

We could then make these modules separate crates if we wanted:

The fact that they're in separate crates would be completely transparent to anyone using the main valence crate. We could also put the modules behind feature flags.

Also note that the poor compile times from valence_protocol is mostly caused by the block state code generator creating way too much code. The situation won't be any worse when it's moved to a valence::block crate.