weapons icon indicating copy to clipboard operation
weapons copied to clipboard

Implement skins search

Open samyycX opened this issue 1 year ago • 8 comments

For #273, @crashzk This code can enable player to search skins.

Usage

  1. Type !ws <skin name> to search like !ws asi and it will show a menu like this: QQ截图20230527082354
  2. Click the skin you want then it will show a menu like this: QQ截图20230527082549

Options

  • Apply All: Apply to all weapons that have this skin (with their own skin id)
  • Apply current: Apply to the weapon in player hand with its skin id, if the weapon in player hand doesn't have this skin, it will return SearchApplyCurrentFailed phrase.
  • Specified weapon: this option IS NOT designed for applying the skin to specified weapon some skins, like Asiimov, has different skin id for different weapons ( 801 for AK47, 551 for P250) this option can apply the skin id you selected to the weapon in your hand (even if the weapon doesn't originally have this skin) the reason i designed it like this because i think this will be more fun and useful ( because player can just hold the specified weapon in their hand and click 'Apply current' option instead of setting with this option)

Translation Phrase

  • SearchDisabled
  • SearchApplyAll
  • SearchApplyCurrent
  • SearchApplyCurrentFailed

TODO

since #271 add a cvar sm_weapons_enable_all_skins, we need to do a check before player use the specified weapon options ( or just disable all the specified weapon options )

samyycX avatar May 27 '23 08:05 samyycX

@samyycX thank you very much, I will be testing this version of yours in the branch I use. Any problems report here, thanks again.

[UPDATE] I believe you need to make some changes to merge with #271, my version already has it included and gives an error when merging. At least that's what GItHub warns.

https://github.com/ZK-Servidores/weapons/pull/7

crashzk avatar May 28 '23 19:05 crashzk

@samyycX thanks, I'll be testing everything now and come back here. Thanks.

crashzk avatar May 29 '23 12:05 crashzk

@crashzk,

@samyycX thank you very much, I will be testing this version of yours in the branch I use. Any problems report here, thanks again.

[UPDATE] I believe you need to make some changes to merge with #271, my version already has it included and gives an error when merging. At least that's what GItHub warns.

ZK-Servidores#7

Sorry but im not very familiar with git.. Since you have code for PR 271 in your repository, should I commit the code in your repository from now on? Because if I continue to commit the code in my repository, it will be a problem for you to merge

samyycX avatar May 29 '23 12:05 samyycX

@samyycX my version is now going, I took the liberty of adding the Portuguese-BR translation in my branch.

I believe so, must confirm in my repository. So both your changes and the one in PR #271 take effect if @kgns merges after checking everything.

The only difference between my branch and yours is that I added access to the sm_weapons_enable_all_skins option in the menu for players with VIP. I can change this here easily if needed to merge here.

[UPDATE] I tested your changes, everything is fine here. I believe I can merge the changes from my branch into yours just to add PT-BR translation and wait for a reply now from the author.

Thanks again, if I get any errors let me know here.

crashzk avatar May 29 '23 12:05 crashzk

@samyycX my version is now going, I took the liberty of adding the Portuguese-BR translation in my branch.

I believe so, must confirm in my repository. So both your changes and the one in PR #271 take effect if @kgns merges after checking everything.

The only difference between my branch and yours is that I added access to the sm_weapons_enable_all_skins option in the menu for players with VIP. I can change this here easily if needed to merge here.

[UPDATE] I tested your changes, everything is fine here. I believe I can merge the changes from my branch into yours just to add PT-BR translation and wait for a reply now from the author.

Thanks again, if I get any errors let me know here.

Sure, my code is basically complete, the only problem is that I haven't make sm_weapons_enable_all_skins cvar control the specified weapons options. since you add the access to it to player, you may modify menus.sp, add a check in SkinsMenuHandler switch case MenuAction_DisplayItem, line 1265 in your repository. You can always modify it and merge.

samyycX avatar May 29 '23 12:05 samyycX

@samyycX I understand, thank you.

In any case, I saw your PR #274 and #271, both can be merged later without problems, as you mentioned before too, individually.

From what I've seen, they won't generate conflicts if they merge first with 271 and then with 274. My version in this case would be the same as them, but as I mentioned, with VIP access in the sm_weapons_enable_all_skinscvar option. Anyway, that's it for now, after merging both I create a direct PR just to translate the language I use.

And again, thank you!

crashzk avatar May 29 '23 13:05 crashzk

Ok, I will wait for PR #271 to be merged and then make some compatibility changes in this PR afterwards.

samyycX avatar May 29 '23 13:05 samyycX

@kgns sorry for the ping, could this PR #274 and #271 be merged?

#271 should come first in the case so we can see that there won't be compatibility errors with this PR later.

crashzk avatar Jun 09 '23 21:06 crashzk