valence-rs / valence

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

Add utility function to ItemStack for creating custom PlayerHeads #621

Open NotThatRqd opened 4 months ago

NotThatRqd commented 4 months ago

Objective

Solution

Concerns

NotThatRqd commented 4 months ago

Hmm, not sure why the tests are failing because it doesn't seem like what I changed should cause this to happen

NotThatRqd commented 4 months ago

Some of the tests are failing but I don't think it's related to my changes.

JackCrumpLeys commented 4 months ago

Hmm, not sure why the tests are failing because it doesn't seem like what I changed should cause this to happen

cargo fmt --all

In the example you don't use the command system (a system that is near and dear to me), this encourages library users not to do so. See the command example. I recommend something like this:

#[derive(Command, Debug, Clone)]
#[paths("head")]
#[scopes("valence.command.head")]
enum HeadCommand {
    #[paths("{whos_head}")]
    Head { whos_head: String }, // see https://valence.rs/rustdoc/src/valence_command/parsers/strings.rs.html for more string options
}

fn command_handler(
     mut command_events: EventReader<CommandResultEvent<HeadCommand>>>,
     mut clients_query: Query<(&mut Client, &Username, &Properties, &mut Inventory)>,
 ) {
     for event in command_events.read() {
         let target = if let Some(target_username) = &event.result {
             clients_query
                 .iter()
                 .find(|(_, username, _, _)| username.0 == target_username) 
         } else {
             Some(clients_query.get(event.executor).unwrap())
         };

         if let Some(target) = target {
             let properties = target.2;
             let textures = properties.textures().unwrap();

             // Construct a PlayerHead using `with_playerhead_texture_value`
             let head = ItemStack::new(
                 ItemKind::PlayerHead,
                 1,
                 None,
             ).with_playerhead_texture_value(textures.value.clone()).unwrap();

             let (_, _, _, mut inventory) = clients_query.get_mut(event.executor).unwrap();
             inventory.set_slot(36, head);
         } else {
             let (mut client, _, _, _) = clients_query.get_mut(event.executor).unwrap();
             client.send_chat_message(
                 "No player with that username found on the server".color(Color::RED),
             );
             continue;
         }
     }
 }

This will allow the client to do syntax highlighting and error handling to be done upstream. NOTE: I did not test this code.

NotThatRqd commented 4 months ago

Wow, didn't know that existed until today 😅 Thanks for letting me know about the command system

One thing though, wouldn't it be like this?

mut command_events: EventReader<CommandResultEvent<HeadCommand>>>,
JackCrumpLeys commented 4 months ago

Wow, didn't know that existed until today 😅 Thanks for letting me know about the command system

One thing though, wouldn't it be like this?

mut command_events: EventReader<CommandResultEvent<HeadCommand>>>,

Haha 😂 silly me. I copied from the command example and forgot to change. Good catch. I will edit to improve clarity in the future.

NotThatRqd commented 4 months ago
#[derive(Command, Debug, Clone)]
#[paths("head")]
#[scopes("valence.command.head")]
enum HeadCommand {
    #[paths("{whos_head}")]
    Head { whos_head: String }, // see https://valence.rs/rustdoc/src/valence_command/parsers/strings.rs.html for more string options
}

Is there a way to specify Head must be a Player's username for auto complete? I vageuly remember doing something like that in Bukkit once..

Maybe EntitySelector, but it seems to allow you to select multiple Players or even entities which isn't quite what I'm talking about

JackCrumpLeys commented 4 months ago

I'm pretty sure that the client doesn't know players that are not in the server. Have a look at all the options in valance_command::parse if I remember correctly I implemented them all.

On Thu, 6 Jun 2024, 10:01 rad, @.***> wrote:

[derive(Command, Debug, Clone)]#[paths("head")]#[scopes("valence.command.head")]enum HeadCommand {

#[paths("{whos_head}")]
Head { whos_head: String }, // see https://valence.rs/rustdoc/src/valence_command/parsers/strings.rs.html for more string options}

Is there a way to specify Head must be a Player's username for auto complete? I vageuly remember doing something like that in Bukkit once..

— Reply to this email directly, view it on GitHub https://github.com/valence-rs/valence/pull/621#issuecomment-2151031224, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMLABFI2T3K4CKK5XUU3WPDZF6DDPAVCNFSM6AAAAABIXUJVOGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJRGAZTCMRSGQ . You are receiving this because you commented.Message ID: @.***>

JackCrumpLeys commented 4 months ago

Oh and autocomplete server-side does not exist yet. I want to wait until after the rewrite to do that.

On Thu, 6 Jun 2024, 10:04 jack crump-leys, @.***> wrote:

I'm pretty sure that the client doesn't know players that are not in the server. Have a look at all the options in valance_command::parse if I remember correctly I implemented them all.

On Thu, 6 Jun 2024, 10:01 rad, @.***> wrote:

[derive(Command, Debug, Clone)]#[paths("head")]#[scopes("valence.command.head")]enum HeadCommand {

#[paths("{whos_head}")]
Head { whos_head: String }, // see https://valence.rs/rustdoc/src/valence_command/parsers/strings.rs.html for more string options}

Is there a way to specify Head must be a Player's username for auto complete? I vageuly remember doing something like that in Bukkit once..

— Reply to this email directly, view it on GitHub https://github.com/valence-rs/valence/pull/621#issuecomment-2151031224, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMLABFI2T3K4CKK5XUU3WPDZF6DDPAVCNFSM6AAAAABIXUJVOGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJRGAZTCMRSGQ . You are receiving this because you commented.Message ID: @.***>

NotThatRqd commented 4 months ago

Yeah that's a good point, I always thought that the auto complete for online players was clientside but now that I think about it it's probably not

JackCrumpLeys commented 4 months ago

auto complete for online players was clientside

See PlayerSelector this will match players in the server.

NotThatRqd commented 4 months ago

Doesn't seem to come up when searching in the docs https://valence.rs/rustdoc/valence_command/parsers/index.html?search=PlayerSelector

NotThatRqd commented 4 months ago

I edited my comment above, that's what I was considering but it also allows you to select any entity or multiple entities which I don't want, so I think just manually parsing single usernames for online players like I do in the example is fine bc it's pretty simple to do.

JackCrumpLeys commented 4 months ago

Just use the command to get a string.

NotThatRqd commented 4 months ago

Alright idk why it doesn't like my formatting, I've ran the formatter on my code. I'm not sure what this means Diff in /home/runner/work/valence/valence/crates/valence_protocol/src/item.rs at line __

JackCrumpLeys commented 4 months ago

If you read the pipeline log you can see the executed command and the mentioned diff. You may have an old version of rust try rustup update.

NotThatRqd commented 4 months ago

Just ran rustup update and then cargo fmt --all -- --check but there weren't any changes :/

NotThatRqd commented 4 months ago

YES okay I just manually added in the changes it wanted because cargo fmt wouldn't for some reason.

rj00a commented 4 months ago

We're using nightly rustfmt settings here, so you probably need to do cargo +nightly fmt or switch to the nightly toochain.