Adds BUILDIN(equipidx); BUILDIN(equip) now returns 0 or 1. #2355

bWolfie · 2019-01-22T14:00:28Z

Pull Request Prelude

I have followed proper Hercules code styling.
I have read and understood the contribution guidelines before making this PR.
I am aware that this PR may be closed if the above-mentioned criteria are not fulfilled.

Changes Proposed

Adds new buildin equipidx().
i.e. equipidx(<index>{, <item id>})

The reason for this change is it allows you the option to choose the specific inventory index to equip. Of course, you need to know the inventory index.

Currently I am using this in a mass refiner script. Since you need to equip an item to refine it, I was having trouble due to the equip() command always equipping the lowest index (I believe that's what it was doing).

Here is a sample in which all the different versions of 'Knife' are cycled through and equipped every 1 second.

.@id = Knife;
getinventorylist();
for (.@i = 0; .@i < @inventorylist_count; .@i++) {
	if (@inventorylist_id[.@i] == .@id) {
		.@Index[.@j] = .@i;
		.@j++;
		.@count++;
	}
}

for (.@i = 0; .@i < .@count; .@i++) {
	equipidx(.@Index[.@i], .@id);
	sleep2(1000);
}
end;

equip() now returns 0 or 1 depending on whether the item successfully equipped.

Issues addressed:
N/A

HerculesWSAPI · 2019-01-22T14:00:39Z

This change is

AnnieRuru · 2019-01-22T17:52:25Z

I think don't need, *equip2 already seems enough

Currently I am using this in a mass refiner script`

... this is not an official feature ...

Since you need to equip an item to refine it, I was having trouble due to the equip() command always equipping the lowest index (I believe that's what it was doing)

I am sure *successrefitem is meant to upgrade the equipped item ...

well, just my 2 cents

PS: when you say mass refiner script, I was thinking about using delitem2/getitem2
hmmm ... we don't have getitem3 ??

4144 · 2019-01-22T18:46:40Z

i not sure probably better create command equipIdx? Because if you know index, id is useless

AnnieRuru · 2019-01-22T18:54:00Z

erm ... but if the item already refined, it went from knife +0 into knife +10
isn't the index changed ? because its entirely different item

4144 · 2019-01-22T19:50:15Z

if get list non refined items before start refining, item with this it cant be refined in this loop two times.

I mean one item refine do remove/add item, and it reusing existing item index or adding item in new index, it not affect other items in one loop iteration

4144 · 2019-01-22T19:53:58Z

Another option for refine without add/remove item is abusing packet ZC_ENCHANT_EQUIPMENT script command enchantitem.
Normally it only add/remove cards in equipped item. But i can abuse it for change almost any items properties. Is this feature should be added?

AnnieRuru · 2019-01-22T20:47:18Z

that sounds AWESOME ...

but wait, can it change the ItemID as well ?
like change from knife into Spear ...? without deleting existing item ?
seems like it can cause weight issue or something

struct PACKET_ZC_ENCHANT_EQUIPMENT {
	int16 packetType;
	int16 wearState;
	int16 cardSlot;
#if PACKETVER_MAIN_NUM >= 20181121 || PACKETVER_RE_NUM >= 20180704 || PACKETVER_ZERO_NUM >= 20181114
	int32 itemId;
#else
	int16 itemId;
#endif
	int8 equipFlag;
} __attribute__((packed));

enlighten me, but I didn't see the refine flag there, nor the random option ...

4144 · 2019-01-22T20:54:59Z

item id i think can be changed too. But i not sure is client update all things if change id.
As i said before, for change other fields than cards need abuse client features.
With item options bit hard, because need send many packets for change them.
All magic happend in client if use cardSlot smaller or bigger than 0 to 3.

bWolfie · 2019-01-23T01:34:56Z

After successrefitem(), it seems the item doesn't change index and I can continue to loop through all them in my inventory.

Here's a video of me using the mass refiner with this new equip function: https://www.youtube.com/watch?v=J8C3DxtlhN4

equipidx() is a good suggestion (@4144). Maybe that is a better way to go.

AnnieRuru

The idea of having *equipidx looks fine, but your refiner script still can be improve (based on the video)
can just loop the percentage with *getequiprefinerycnt, then just successrefitem EQI_HAND_R, N;
where N is the number of times it refines

doc/script_commands.txt

src/map/script.c

bWolfie · 2019-01-23T10:32:45Z

I added a check so index isn't out of range. Is this correct or unnecessary?

	if (i < 0 || i >= sd->status.inventorySize) {
		ShowError("buildin_equipidx: Index (%d) should be from 0-%d.\n", i, sd->status.inventorySize);
		script_pushint(st, 0);
		return false;
	}

4144 · 2019-01-23T14:01:30Z

check is correct

src/map/script.c

AnnieRuru

I was just about to review, 4144 is faster than me XD
anyway I have tested this in-game and made some changes

/**
 * equipidx(<index>)
 */
static BUILDIN(equipidx)
{
	int nameid = 0, i = script_getnum(st, 2);
	struct item_data *item_data;
	struct map_session_data *sd = script->rid2sd(st);

	if (sd == NULL) {
		script_pushint(st, 0);
		return true;
	}

	if (i < 0 || i >= sd->status.inventorySize) {
		ShowError("buildin_equipidx: Index (%d) should be from 0-%d.\n", i, sd->status.inventorySize - 1);
		script_pushint(st, 0);
		return false;
	}

	if (sd->status.inventory[i].equip != 0) { // item already equipped, run silently
		script_pushint(st, 1);
		return true;
	}

	nameid = sd->status.inventory[i].nameid;
	if ((item_data = itemdb->exists(nameid)) == NULL) {
		ShowError("buildin_equipidx: Invalid Item ID (%d).\n", nameid);
		script_pushint(st, 0);
		return false;
	}

	if (pc->equipitem(sd, i, item_data->equip) == 0) {
		ShowWarning("buildin_equipidx: Item ID (%d) at index (%d) cannot be equipped.\n", nameid, i);
		script_pushint(st, 0);
		return false;
	}

	script_pushint(st, 1);
	return true;
}

honestly I don't see the point having {, } optional field there
after all, this is run by *getinventorylist,
and it already can be check by @inventorylist_id[.@i] already isn't it ?

src/map/script.c

AnnieRuru · 2019-01-23T14:26:09Z

another things concern me is assassin class can equip weapons in both hands,
accessory can equip in EQI_ACC_L or EQI_ACC_R

hmm ... if you couldn't do it, can leave at it is, after all nobody actually complain about this issue

*equip(<item id>{, <equip position>})

bWolfie · 2019-01-24T14:26:00Z

Thanks for the input guys. I am busy for a few days I will update once I'm free. Also, thanks for your tips on the refiner script, AnnieRuru. Was able to streamline it so script is cleaner and effects are only displaying once for better feel for the player.

bWolfie · 2019-01-29T12:52:43Z

Updated based on comments from Annie and 4144.

Though I made this bit if (pc->equipitem(sd, i, item_data->equip) == 0) { return true instead of false, since I don't see reason to stop script execution if it failed to equip (we're already returning 0 in that instance).

src/map/script.c

bWolfie · 2019-01-30T15:16:56Z

Okay done. Let me know if anything else is needed.

doc/script_commands.txt

AnnieRuru

ok wait 4144 again

4144 · 2019-02-28T17:44:46Z

src/map/script.c

+		return true;
+	}
+
+	nameid = script_getnum(st, 2);


nameid can be defined here
int nameid = script_getnum(st, 2);

AnnieRuru · 2019-02-28T17:56:31Z

/*=================================================
 * Checks if the player can equip the item at index n in inventory.
 * Returns 0 (no) or 1 (yes).
 *------------------------------------------------*/
static int pc_isequip(struct map_session_data *sd, int n)

try introduce a new script command *canequip to check if that item can be equip ?

AnnieRuru · 2019-03-09T18:11:47Z

doc/script_commands.txt

+	getinventorylist();
+	for (.@i = 0; .@i < @inventorylist_count; ++.@i) {
+		if (@inventorylist_id[.@i] == Knife && @inventorylist_expire[.@i]) {
+			equipidx(.@i);


thanks to dastgir idea, that just push another array of the index,
that means this example has to change accordingly

Suggested change

equipidx(.@i);

equipidx(@inventorylist_idx[.@i]);

Emistry · 2019-09-12T10:46:15Z

doc/script_commands.txt

@@ -5622,6 +5625,30 @@ Examples:

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

+*equipidx(<index>)
+
+The equipidx function will attempt to equip the item at the given inventory


Suggested change

The equipidx function will attempt to equip the item at the given inventory

This function will attempt to equip the item at the given

Emistry · 2019-09-12T10:47:26Z

doc/script_commands.txt

+inventory index. This feature is only really useful when multiple instances
+of an Item ID exist in the player's inventory.
+
+Returns 1 if the item was successfully equipped.


Suggested change

Returns 1 if the item was successfully equipped.

Return true if successful, otherwise it will return false.

Emistry · 2019-09-12T10:53:08Z

doc/script_commands.txt

@@ -5603,6 +5603,9 @@ item in his/her inventory, while the autoequip function will equip the
 given item ID when this is looted. The option parameter of the autoequip
 is 1 or 0, 1 to turn it on, and 0 to turn it off.

+equip() returns 1 if the item was successfully equipped and 0 if the


Suggested change

equip() returns 1 if the item was successfully equipped and 0 if the

Return equipment's position if successful, otherwise it will return false.

Emistry · 2019-09-12T10:58:03Z

src/map/script.c

@@ -25491,6 +25540,7 @@ static void script_parse_builtin(void)
 		BUILDIN_DEF(equip,"i"),
 		BUILDIN_DEF(autoequip,"ii"),
 		BUILDIN_DEF(equip2,"iiiiiii"),
+		BUILDIN_DEF(equipidx, "i?"),


Suggested change

BUILDIN_DEF(equipidx, "i?"),

BUILDIN_DEF(equipidx, "i"),

didnt have optional parameter.

bWolfie force-pushed the buildin_equip branch 2 times, most recently from 1e0462d to 06bcc68 Compare January 22, 2019 15:05

bWolfie force-pushed the buildin_equip branch 2 times, most recently from a525f09 to f03ab26 Compare January 23, 2019 06:19

bWolfie changed the title ~~Allows BUILDIN(equip) to specify an inventory index to equip.~~ Adds BUILDIN(equipidx) and equip() returns 0 or 1. Jan 23, 2019

bWolfie changed the title ~~Adds BUILDIN(equipidx) and equip() returns 0 or 1.~~ Adds BUILDIN(equipidx) and BUILDIN(equip) returns 0 or 1. Jan 23, 2019

bWolfie changed the title ~~Adds BUILDIN(equipidx) and BUILDIN(equip) returns 0 or 1.~~ Adds BUILDIN(equipidx); BUILDIN(equip) now returns 0 or 1. Jan 23, 2019

bWolfie force-pushed the buildin_equip branch from f03ab26 to 2fb233e Compare January 23, 2019 07:15

AnnieRuru requested changes Jan 23, 2019

View reviewed changes

doc/script_commands.txt Outdated Show resolved Hide resolved

src/map/script.c Outdated Show resolved Hide resolved

src/map/script.c Outdated Show resolved Hide resolved

src/map/script.c Outdated Show resolved Hide resolved

bWolfie force-pushed the buildin_equip branch from 2fb233e to e84f52c Compare January 23, 2019 10:29

bWolfie force-pushed the buildin_equip branch from e84f52c to 00d5314 Compare January 23, 2019 10:51

4144 requested changes Jan 23, 2019

View reviewed changes

src/map/script.c Outdated Show resolved Hide resolved

src/map/script.c Outdated Show resolved Hide resolved

src/map/script.c Outdated Show resolved Hide resolved

src/map/script.c Outdated Show resolved Hide resolved

AnnieRuru requested changes Jan 23, 2019

View reviewed changes

src/map/script.c Outdated Show resolved Hide resolved

bWolfie force-pushed the buildin_equip branch 3 times, most recently from dca185b to 698dd34 Compare January 29, 2019 12:50

bWolfie force-pushed the buildin_equip branch from 698dd34 to 63c2de9 Compare January 29, 2019 12:57

AnnieRuru requested changes Jan 30, 2019

View reviewed changes

src/map/script.c Outdated Show resolved Hide resolved

src/map/script.c Outdated Show resolved Hide resolved

Changes equip() to return 0 or 1 if item successfully equipped.

6ff691f

bWolfie force-pushed the buildin_equip branch from 63c2de9 to 273dd94 Compare January 30, 2019 14:57

AnnieRuru approved these changes Jan 30, 2019

View reviewed changes

Emistry added component:core:scriptengine Affecting the script engine or the script commands component:documentation Affecting the documentation in the doc/ folder labels Feb 26, 2019

AnnieRuru requested changes Feb 28, 2019

View reviewed changes

doc/script_commands.txt Outdated Show resolved Hide resolved

bWolfie force-pushed the buildin_equip branch from 273dd94 to 221191e Compare February 28, 2019 11:13

Adds BUILDIN(equipidx), which attempts to equip an item at given index.

29af737

bWolfie force-pushed the buildin_equip branch from 221191e to 29af737 Compare February 28, 2019 13:43

AnnieRuru approved these changes Feb 28, 2019

View reviewed changes

4144 requested changes Feb 28, 2019

View reviewed changes

src/map/script.c

return true;

}

nameid = script_getnum(st, 2);

Copy link

Contributor

4144 Feb 28, 2019

Choose a reason for hiding this comment

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

nameid can be defined here
int nameid = script_getnum(st, 2);

AnnieRuru requested changes Mar 9, 2019

View reviewed changes

AnnieRuru mentioned this pull request Apr 13, 2019

Add *setfavoriteitemidx & *autofavoriteitem script commands - alter item favorite state #2427

Merged

3 tasks

Emistry reviewed Sep 12, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds BUILDIN(equipidx); BUILDIN(equip) now returns 0 or 1. #2355

Adds BUILDIN(equipidx); BUILDIN(equip) now returns 0 or 1. #2355

bWolfie commented Jan 22, 2019 •

edited

HerculesWSAPI commented Jan 22, 2019

AnnieRuru commented Jan 22, 2019

4144 commented Jan 22, 2019

AnnieRuru commented Jan 22, 2019

4144 commented Jan 22, 2019

4144 commented Jan 22, 2019

AnnieRuru commented Jan 22, 2019

4144 commented Jan 22, 2019 •

edited

bWolfie commented Jan 23, 2019 •

edited

AnnieRuru left a comment

bWolfie commented Jan 23, 2019

4144 commented Jan 23, 2019

AnnieRuru left a comment

AnnieRuru commented Jan 23, 2019 •

edited

bWolfie commented Jan 24, 2019 •

edited

bWolfie commented Jan 29, 2019 •

edited

bWolfie commented Jan 30, 2019

AnnieRuru left a comment •

edited

4144 Feb 28, 2019

AnnieRuru commented Feb 28, 2019

AnnieRuru Mar 9, 2019

Emistry Sep 12, 2019

Emistry Sep 12, 2019

Emistry Sep 12, 2019

Emistry Sep 12, 2019

	The equipidx function will attempt to equip the item at the given inventory
	This function will attempt to equip the item at the given

	Returns 1 if the item was successfully equipped.
	Return true if successful, otherwise it will return false.

	equip() returns 1 if the item was successfully equipped and 0 if the
	Return equipment's position if successful, otherwise it will return false.

Adds BUILDIN(equipidx); BUILDIN(equip) now returns 0 or 1. #2355

Are you sure you want to change the base?

Adds BUILDIN(equipidx); BUILDIN(equip) now returns 0 or 1. #2355

Conversation

bWolfie commented Jan 22, 2019 • edited

Pull Request Prelude

Changes Proposed

HerculesWSAPI commented Jan 22, 2019

AnnieRuru commented Jan 22, 2019

4144 commented Jan 22, 2019

AnnieRuru commented Jan 22, 2019

4144 commented Jan 22, 2019

4144 commented Jan 22, 2019

AnnieRuru commented Jan 22, 2019

4144 commented Jan 22, 2019 • edited

bWolfie commented Jan 23, 2019 • edited

AnnieRuru left a comment

Choose a reason for hiding this comment

bWolfie commented Jan 23, 2019

4144 commented Jan 23, 2019

AnnieRuru left a comment

Choose a reason for hiding this comment

AnnieRuru commented Jan 23, 2019 • edited

bWolfie commented Jan 24, 2019 • edited

bWolfie commented Jan 29, 2019 • edited

bWolfie commented Jan 30, 2019

AnnieRuru left a comment • edited

Choose a reason for hiding this comment

4144 Feb 28, 2019

Choose a reason for hiding this comment

AnnieRuru commented Feb 28, 2019

AnnieRuru Mar 9, 2019

Choose a reason for hiding this comment

Emistry Sep 12, 2019

Choose a reason for hiding this comment

Emistry Sep 12, 2019

Choose a reason for hiding this comment

Emistry Sep 12, 2019

Choose a reason for hiding this comment

Emistry Sep 12, 2019

Choose a reason for hiding this comment

bWolfie commented Jan 22, 2019 •

edited

4144 commented Jan 22, 2019 •

edited

bWolfie commented Jan 23, 2019 •

edited

AnnieRuru commented Jan 23, 2019 •

edited

bWolfie commented Jan 24, 2019 •

edited

bWolfie commented Jan 29, 2019 •

edited

AnnieRuru left a comment •

edited