Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand item unique id control #4195

Closed

Conversation

sader1992
Copy link
Contributor

@sader1992 sader1992 commented Jun 1, 2019

  • Addressed Issue(s): #4175

  • Server Mode: both

  • Description of Pull Request:

there is already a command to get the equipment unique id

getequipuniqueid(<equipment slot>{,<char_id>})

with this PR
getinventorylist now assign a int variable the item unique id @inventorylist_unique_id$

OnSellItem now assign @sold_unique_id$ "idea from https://rathena.org/board/topic/119433-how-to-get-unique-id-from-onsellitem/#comment-362164"

countbound now returns @bound_unique_id$[] too for the item unique id

new commands

uniqueid_getiteminfo(<"string  unique id">{,<char_id>})

this command get the item information by it's unique id and assign it to the variables.
the variables are the same as the getinventorylist command , however it start with .@uid_ instead of @inventorylist_

uniqueid_find(<"item_unique_id">{,<char_id>})

check if the player have an item with the unique id provided.

uniqueid_delitem(<"string unique id">{,<char_id>})

this command delete an item from the inventory by it's unique id

test scripts

//@item hat 5
prontera,152,175,4	script	unique_id_control	444,{
	.@unique_id$ = getequipuniqueid(EQI_HEAD_TOP);
	mes .@unique_id$;
	next;
	uniqueid_delitem(.@unique_id$);
	end;
}

prontera,152,177,4	script	unique_id_control2	444,{
	.@unique_id$ = getequipuniqueid(EQI_HEAD_TOP);
	mes .@unique_id$;
	next;
	uniqueid_getiteminfo(.@unique_id$);
	
	mes "ID: " + .@uid_item_id;
	mes "Equip Place: " + .@uid_equip;
	mes "Refine: " + .@uid_refine;
	mes "Identify: " + .@uid_identify;
	mes "Attribute: " + .@uid_attribute;
	mes "Card 1: " + .@uid_card1;
	mes "Card 2: " + .@uid_card2;
	mes "Card 3: " + .@uid_card3;
	mes "Card 4: " + .@uid_card4;
	mes "Expire: " + .@uid_expire;
	mes "Bound: " + .@uid_bound;
	mes "Random Opions";
	mes "ID 1: " + .@uid_option_id1;
	mes "Value 1: " + .@uid_option_value1;
	mes "Param 1: " + .@uid_option_parameter1;
	mes "ID 2: " + .@uid_option_id2;
	mes "Value 2: " + .@uid_option_value2;
	mes "Param 2: " + .@uid_option_parameter2;
	mes "ID 3: " + .@uid_option_id3;
	mes "Value 3: " + .@uid_option_value3;
	mes "Param 3: " + .@uid_option_parameter3;
	mes "ID 4: " + .@uid_option_id4;
	mes "Value 4: " + .@uid_option_value4;
	mes "Param 4: " + .@uid_option_parameter4;
	mes "ID 5: " + .@uid_option_id5;
	mes "Value 5: " + .@uid_option_value5;
	mes "Param 5: " + .@uid_option_parameter5;
	mes "Tradable: " + .@uid_tradable;
end;
}

prontera,152,179,4	script	unique_id_control3	444,{
	getinventorylist;
	for(.@i = 0;.@i< @inventorylist_count;.@i++){
		mes @inventorylist_id[.@i] + " " + @inventorylist_unique_id$[.@i];
	}
end;
}

-	shop	dyn_shop1	-1,501:50.

prontera,152,181,4	script	unique_id_control4	444,{
	callshop "dyn_shop1",2;
	npcshopattach "dyn_shop1";
	end;

OnSellItem:
	mes @sold_unique_id$;
end;
}
Old Script/Idea ``` //@item hat 5 prontera,152,175,4 script unique_id_control 444,{ .@unique_id = getequipuniqueid(EQI_HEAD_TOP); mes .@unique_id; next; uniqueid_delitem .@unique_id; end; }

prontera,152,177,4 script unique_id_control2 444,{
.@unique_id = getequipuniqueid(EQI_HEAD_TOP);
mes .@unique_id;
next;
uniqueid_getiteminfo .@unique_id,.@item;
for(.@i=0;.@i<getarraysize(.s$);.@i++){
mes .s$[.@i] + " " + .@item[.@i];
}
for(.@i= getarraysize(.s$);.@i<getarraysize(.@item);.@i+=3){
mes "it->option[" + .@i + "].id" + .@item[.@i];
mes "it->option[" + .@i + "].value" + .@item[.@i];
mes "it->option[" + .@i + "].param" + .@item[.@i];
}
end;
OnInit:
setarray .s$,
"it->nameid"
,"it->refine"
,"it->identify"
,"it->attribute"
,"it->card[0]"
,"it->card[1]"
,"it->card[2]"
,"it->card[3]"
,"it->expire_time"
,"it->bound";
}

prontera,152,179,4 script unique_id_control3 444,{
getinventorylist;
for(.@i = 0;.@i< @inventorylist_count;.@i++){
mes @inventorylist_id[.@i] + " " + @inventorylist_unique_id[.@i];
}
end;
}

  • shop dyn_shop1 -1,501:50.

prontera,152,181,4 script unique_id_control4 444,{
callshop "dyn_shop1",2;
npcshopattach "dyn_shop1";
end;

OnSellItem:
mes @sold_uniqueid;
end;
}

Copy link

@anacondaq anacondaq left a comment

Also, recommend renaming uniqueid item commands to something like

uniqueid_COMMAND
or unique_id_COMMAND
to keep all unique_id with the same pre-fix or post-fix.

Also, you forgot to add documentation with examples to /doc/script_commands.txt

src/map/script.cpp Outdated Show resolved Hide resolved
src/map/script.cpp Outdated Show resolved Hide resolved
src/map/script.cpp Outdated Show resolved Hide resolved
src/map/script.cpp Outdated Show resolved Hide resolved
src/map/script.cpp Outdated Show resolved Hide resolved
sader1992 and others added 7 commits Jun 1, 2019
Co-Authored-By: anacondaq <anacondaq@users.noreply.github.com>
Co-Authored-By: anacondaq <anacondaq@users.noreply.github.com>
Co-Authored-By: anacondaq <anacondaq@users.noreply.github.com>
Co-Authored-By: anacondaq <anacondaq@users.noreply.github.com>
src/map/npc.cpp Outdated Show resolved Hide resolved
@sader1992
Copy link
Contributor Author

@sader1992 sader1992 commented Jun 13, 2019

tested it , work great
sorry @aleos89 look like i did a lot of things wrong xD
i will be more carefull next time

Copy link
Contributor

@cydh cydh left a comment

If you mind for these

src/map/script.cpp Outdated Show resolved Hide resolved
src/map/script.cpp Outdated Show resolved Hide resolved
src/map/script.cpp Outdated Show resolved Hide resolved
src/map/script.cpp Outdated Show resolved Hide resolved
src/map/script.cpp Outdated Show resolved Hide resolved
src/map/script.cpp Outdated Show resolved Hide resolved
thanks to @cydh

Co-Authored-By: Cydh Ramdh <cydh@users.noreply.github.com>
@aleos89
Copy link
Contributor

@aleos89 aleos89 commented Jun 13, 2019

No worries, you did nothing wrong. I figured it'd be quicker for me to manually make the changes than list tons of edits.

@sader1992
Copy link
Contributor Author

@sader1992 sader1992 commented Sep 15, 2019

if someone want to test the PR (like for a real life usage)
this script is ready to use , that uses the new commands
[UnOfficial] Skyfortress Drop/reroll enchantment Functions 1.0.0

@sader1992
Copy link
Contributor Author

@sader1992 sader1992 commented Jan 16, 2020

update to work as int64 instead of unsigned long long
to follow e72c736

Copy link
Contributor

@Atemo Atemo left a comment

My 2 cents!

src/map/script.cpp Outdated Show resolved Hide resolved
if (it->equip)
pc_unequipitem(sd, i, 3);

if (buildin_delitem_search(sd, it, 0, 0)) {
Copy link
Contributor

@Atemo Atemo Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildin_delitem_search doesn't check the unique id and it may delete the wrong item. Maybe pc_delitem would do the job

@sader1992
Copy link
Contributor Author

@sader1992 sader1992 commented Mar 23, 2021

@Atemo thanks for the feedback, will work on it

for the uniqueid_getiteminfo I think it's better idea to just remove it and relay on getinventorylist instead

getinventorylist;
if((.@ndx = inarray(@inventorylist_unique_id,<unique_id>)) !=-1){
	//@inventorylist_id[.@ndx]
	//@inventorylist_refine[.@ndx]
	//@inventorylist_card1[.@ndx]
	//etc
}else{
	//item not found
}

and it would be easier then just making the writer remember what index the cards are etc
so this pr would be just adding delete_item_unique_id/edit getinventorylist/also making getequipuniqueid return int instead of string

@Atemo
Copy link
Contributor

@Atemo Atemo commented Mar 23, 2021

Eventually the unique id could be also saved in

@lgtm-com
Copy link

@lgtm-com lgtm-com bot commented May 12, 2021

This pull request introduces 1 alert when merging c1ae466 into 58f2d21 - view on LGTM.com

new alerts:

  • 1 for Comparison result is always the same

sader1992 and others added 4 commits May 12, 2021
Co-Authored-By: Atemo <Atemo@users.noreply.github.com>
Co-Authored-By: Atemo <Atemo@users.noreply.github.com>
this I think better, this the same variable names as the getinventorylist , however it start with ".@uid_"

better then remembering the index of each value
.@uid_item_id - item ID
.@uid_equip - on which position the item is equipped (see EQP_* constants)
It will contain 0 if the item is not equipped.
.@uid_refine - refine
.@uid_identify - identify
.@uid_attribute - attribute
.@uid_card1 - card 1
.@uid_card2 - card 2
.@uid_card3 - card 3
.@uid_card4 - card 4
.@uid_expire - expire time
.@uid_bound - bound type
.@uid_option_id1 - option ID 1
.@uid_option_value1 - option value 1
.@uid_option_parameter1 - option parameter 1
.@uid_option_id2 - option ID 2
.@uid_option_value2 - option value 2
.@uid_option_parameter2 - option parameter 2
.@uid_option_id3 - option ID 3
.@uid_option_value3 - option value 3
.@uid_option_parameter3 - option parameter 3
.@uid_option_id4 - option ID 4
.@uid_option_value4 - option value 4
.@uid_option_parameter4 - option parameter 4
.@uid_option_id5 - option ID 5
.@uid_option_value5 - option value 5
.@uid_option_parameter5 - option parameter 5
.@uid_tradable - item tradability
Copy link
Contributor Author

@sader1992 sader1992 May 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is way better then just letting the scripter provide a .@array , the variables like this are the same as getinventorylist command but they start with ".@uid" instead of "@inventorylist_"
we can get the same result with something like this

getinventorylist;
if((.@ndx = inarray(@inventorylist_unique_id$,<"unique_id">)) !=-1){
	//@inventorylist_id[.@ndx]
	//@inventorylist_refine[.@ndx]
	//@inventorylist_card1[.@ndx]
	//etc
}else{
	//item not found
}

but i still think if we want the information of one item only , no need to use getinventory list command

@lgtm-com
Copy link

@lgtm-com lgtm-com bot commented May 12, 2021

This pull request introduces 2 alerts when merging efebd7e into 58f2d21 - view on LGTM.com

new alerts:

  • 2 for Comparison result is always the same

@@ -2689,6 +2689,82 @@ See 'getequipid' for a full list of valid equipment slots.

---------------------------------------

*uniqueid_getiteminfo(<"item_unique_id">{,<char_id>})

This function assigns the item information of <item_unique_id> to the variables mentioned down there.
Copy link

@Litro Litro May 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recommend to add "from the invoking character's inventory if found." for more detailed description

@sader1992
Copy link
Contributor Author

@sader1992 sader1992 commented Nov 18, 2021

I think after this update #6247
the inventory index update gives the intended functionality of this pull request in a different way.
so in think this pull request is not as important as before.
will close the pull request, if any dev wanted me to reopen it , I would.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants