Add field for Citizens' new --url option

[Andre601]

Is your feature request related to a problem? Please describe. No

Describe the solution you'd like Citizens added a new --url parameter to the skin command, which allows to directly set the Skin of a NPC through the provided URL. I want to suggest to implement a new field, which would provide the command with the new --url field (/npc skin --url :url)

Something that came into my mind was to perhaps also have the above generated command with a custom (smaller) url which acts as a redirect to the textures-url of minecraft. (Something like https://mineskin.org/:id/texture) This would give an easy to use command, which would also be small in sice and therefore 100% usable by the player ingame.

Describe alternatives you've considered The only alternative is to either grab the texture URL from Mineskin, or to use the much longer skin command using the -t argument.

Additional context Update mentioning the new command option: https://www.spigotmc.org/resources/citizens.13811/update?update=324588

I tested it with this texture URL and it worked without issues: http://textures.minecraft.net/texture/184e745d4a037f2a8572c8947b06411f63181346932f14608eac34f339dac71f

[InventivetalentDev]

Looks like they're using mineskin for url texture generation aswell, so that might be a bit problematic. (https://github.com/CitizensDev/Citizens2/commit/5f6026f50bef28e37f9f2a67f6ae18edbc2d0edc)
I'd kinda want to implement this a bit more elegantly.

[Andre601]

I see. So from the comment you posted on the above mentioned commit, would you appreciate the Citizens-dev to implement a --mineskin option to directly utilize the Mineskin API? Just to understand this correctly.

[InventivetalentDev]

It'd certainly make more sense than browsing mineskin to get a texture url, just to have citizens use mineskin to generate just that texture again.

[Andre601]

Yeah. Alternatively could Mineskin perhaps check if the texture is a valid MC texture URL to then just return the info it may have?

Or Citizens could perhaps try this... idk

[InventivetalentDev]

Yeah, good point. I'll add a check for that.

[mcmonkey4eva]

I'd suggest just making the server recognize when any mineskin URL is input as an image link, and just performing the automatic lookup-and-return-relevant-details instead of regenerating. Eg if you get a request at https://api.mineskin.org/generate/url with data url=https://mineskin.org/216543087 or similar internal URL, just automatically recognize that it's a link to your own service and return pre-existing data.

For what it's worth: it's fairly standard for 'generate' pathways to not actually generate anything when they don't need to, as standard deduplicating cache logic. Usually powered by a hash of the data compared to the existing dataset (hash table lookup), to find if something has previously been uploaded - which, if you're not already doing something to that effect, I highly recommend getting that set up as well. Users can and will constantly upload things that have previously been uploaded, either by themselves or others.

---

Citizens added a new --url parameter to the skin command --> This isn't a new feature btw, we've had it for a few years, it just wasn't in the core plugin files previously (previously was managed by this script and other versions thereof).

In actual usage we've never had anybody report trouble related to the ratelimit you described in your commit comment, because, naturally... it's a user input command. It looks like your preferred rate slowdown is about 1 second, which is a lot faster than anybody can select a new NPC, find the next skin file, and type the command to put that skin on that NPC. It's not triggered via API, it's not triggered internally (we just keep the texture/sig in save data, not the route by which it was generated), and it's generally advised against to spam lookups anyway (Mojang's direct lookup rate limit for skin-by-name is way harsher)... users who need to duplicate one skin to many NPCs are advised to use the skin save/load commands.

BTW I should note that your /npc skin -t sample command is never going to work and we've had to explicitly tell people to not try to use it and instead just use the script and a link (minecraft client has an input length limit, so a / command with that much data doesn't work... the -t is only meant to work from server console (ie non-/), and even then is only meant for dev/test usage moreso than any endusers ever touching it).

[InventivetalentDev]

it's fairly standard for 'generate' pathways to not actually generate anything when they don't need to

yup, already doing that. Only stuff that's not already there is actually generated. (See https://github.com/MineSkin/api.mineskin.org)

---

we've never had anybody report trouble related to the ratelimit you described in your commit comment, because, naturally... it's a user input command.

Fair point, hadn't really considered only users running the requests. Though there's still a chance of running into the api's limitations when a lot of users happen to run requests at similar times. There's only so many accounts mineskin manages.

---

I should note that your /npc skin -t sample command is never going to work

Yeah I realised that myself pretty early on (iirc that originated from a PR, so I never really put a second thought to it); mainly kept it for console usage.

we've had to explicitly tell people to not try to use it

To be fair, another PR would've been an option :P

---

I'd suggest just making the server recognize when any mineskin URL is input as an image link, and just performing the automatic lookup-and-return-relevant-details instead of regenerating.

I'll definitely get to work on that, really seems like the easiest option.

[Andre601]

I'd suggest just making the server recognize when any mineskin URL is input as an image link, and just performing the automatic lookup-and-return-relevant-details instead of regenerating.

You're aware that I literally suggested this before, right?

Yeah. Alternatively could Mineskin perhaps check if the texture is a valid MC texture URL to then just return the info it may have?

---

This isn't a new feature btw, we've had it for a few years, it just wasn't in the core plugin files previously (previously was managed by this script and other versions thereof).

Considering that not everyone wants to use scripts and that this is worth enough to be mentioned in a update (And was actually added to the plugin itself, therefore wasn't a core part of it) would I say that this is indeed something new. It wasn't something new for that script stuff, but from the perspective of Citizens itself is it something new.

[InventivetalentDev]

I though you were talking about textures.minecraft.net urls (which I'm now checking as of https://github.com/MineSkin/api.mineskin.org/commit/439ac0f4a2e0c919d8bf8241b14ec53976f02838).
Though I did just see the /texture path you suggested, sorry I missed that!

Anyway, just doing both now :)

[Andre601]

In addition does the /npc -t command work. I used it myself many times. You just need to remove the /, and select an NPC first through the console with npc select <id>

But again: It worked for me without any issues and iirc did I even recommend altering the command to not include a / by default as it is used in the console anyway. It's perhaps also worth to mention (in bold maybe?) that the command would only be usable in console and that it requires an NPC to be selected first using npc select <id>

[mcmonkey4eva]

I'd suggest adding a /npc skin --url {generated_mineskin_url} in place of the existing -t sample, instead of actively recommending the dev/test feature and explaining the complexities of using it.

Considering that not everyone wants to use scripts

If you don't run Denizen, you're missing half of the features of Citizens. I don't really see any valid reason to explicitly refuse to run it.

[Andre601]

If you don't run Denizen, you're missing half of the features of Citizens. I don't really see any valid reason to explicitly refuse to run it.

I don't see any proper reason to actually run it in the first place. All I need does Citizens already provide, so there is no use for this.

[InventivetalentDev]

I'd suggest adding a /npc skin --url {generated_mineskin_url} in place of the existing -t sample, instead of actively recommending the dev/test feature and explaining the complexities of using it.

Was planning to do just that!