Add utility function to ItemStack for creating custom PlayerHeads

[NotThatRqd]

Objective

• Make it easier to create PlayerHeads with a custom skin.

Solution

• Quick utility function on the ItemStack struct that will add the necessary NBT for you.

Concerns

• Not sure if my explanation of how you're supposed to use the function (in the rustdoc) is good enough.
• Not sure if returning a Result on a function meant to be chained is okay.
• Not sure if I've chosen a good name for the function.

[NotThatRqd]

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

[NotThatRqd]

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

[JackCrumpLeys]

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]

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]

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]

#[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]

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 <#621 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMLABFI2T3K4CKK5XUU3WPDZF6DDPAVCNFSM6AAAAABIXUJVOGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJRGAZTCMRSGQ> . You are receiving this because you commented.Message ID: ***@***.***>

[JackCrumpLeys]

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 > <#621 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AMLABFI2T3K4CKK5XUU3WPDZF6DDPAVCNFSM6AAAAABIXUJVOGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJRGAZTCMRSGQ> > . > You are receiving this because you commented.Message ID: > ***@***.***> >

[NotThatRqd]

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]

auto complete for online players was clientside

See PlayerSelector this will match players in the server.

[NotThatRqd]

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

[NotThatRqd]

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]

Just use the command to get a string.

[NotThatRqd]

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]

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]

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

[NotThatRqd]

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

[rj00a]

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