valence icon indicating copy to clipboard operation
valence copied to clipboard

Add utility function to ItemStack for creating custom PlayerHeads

Open NotThatRqd opened this issue 1 year ago • 18 comments

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 avatar Jun 04 '24 03:06 NotThatRqd

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

NotThatRqd avatar Jun 04 '24 03:06 NotThatRqd

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

NotThatRqd avatar Jun 05 '24 00:06 NotThatRqd

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.

JackCrumpLeys avatar Jun 05 '24 21:06 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>>>,

NotThatRqd avatar Jun 05 '24 21:06 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>>>,

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

JackCrumpLeys avatar Jun 05 '24 21:06 JackCrumpLeys

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

NotThatRqd avatar Jun 05 '24 22:06 NotThatRqd

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 avatar Jun 05 '24 22:06 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 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 avatar Jun 05 '24 22:06 JackCrumpLeys

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

NotThatRqd avatar Jun 05 '24 22:06 NotThatRqd

auto complete for online players was clientside

See PlayerSelector this will match players in the server.

JackCrumpLeys avatar Jun 05 '24 22:06 JackCrumpLeys

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

NotThatRqd avatar Jun 05 '24 22:06 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.

NotThatRqd avatar Jun 05 '24 22:06 NotThatRqd

Just use the command to get a string.

JackCrumpLeys avatar Jun 05 '24 22:06 JackCrumpLeys

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 __

NotThatRqd avatar Jun 06 '24 02:06 NotThatRqd

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.

JackCrumpLeys avatar Jun 06 '24 04:06 JackCrumpLeys

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

NotThatRqd avatar Jun 06 '24 04:06 NotThatRqd

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

NotThatRqd avatar Jun 06 '24 04:06 NotThatRqd

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

rj00a avatar Jun 06 '24 08:06 rj00a