Skip to content

Conversation

@kisuka
Copy link
Contributor

@kisuka kisuka commented Oct 23, 2018

Pull Request Prelude

Changes Proposed

Adds a script command to return the number of signed items a character has in their inventory.

Currently you have to do the following for this same functionality:

.@slot3 = getcharid(0) & 65535;
.@slot4 = getcharid(0) >> 16;
countitem2(<item ID>, 1, 0, 0, 254, 0, .@slot3, .@slot4)

This pull request changes that so you can simply do this:

countnameditem(<item ID>, {<character id/name>});

@HerculesWSAPI
Copy link
Contributor

This change is Reviewable

@Emistry
Copy link
Member

Emistry commented Oct 24, 2018

could we have extra parameters for count named item of other specific char id that exists in player inventory too?
example: count how many named Apple that exists in PlayerA inventory are named PlayerB's Apple?

countnameditem(<item ID>, {<character id/name>, <target_char_id/name>});

use-case may not be wide, but seem useful if we could have it to count item that named using other player name in different player inventory.

@kisuka
Copy link
Contributor Author

kisuka commented Oct 24, 2018

could we have extra parameters for count named item of other specific char id that exists in player inventory too?
example: count how many named Apple that exists in PlayerA inventory are named PlayerB's Apple?

countnameditem(<item ID>, {<character id/name>, <target_char_id/name>});

use-case may not be wide, but seem useful if we could have it to count item that named using other player name in different player inventory.

Sure. I'll need to tweak the code a bit but should be possible.

@kisuka
Copy link
Contributor Author

kisuka commented Oct 24, 2018

Hmmm might be a bit more complex than I previously thought. So... even in the current state, the inscribed character has to be online for the count to be successful.

I'm not seeing any existing methods in script.c that allows the look up of offline character data. Should we possibly add a new function within pc.c that allows searching for a character's basic information even if they're offline? Or possibly a direct sql command within script.c on the function for countnameditem? Essentially we'd need to look up a character by either their name or id and use their char id within the slot 3 and 4 checks.

The other option is leaving it as self-named items only, which is how the aegis command works.

}
next;
mes("[Item Checker]");
mes("You have "+countnameditem(Apple, "John")+" apples with John's name on it!");
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want support for translations, you cant split sentenses.
Need use sprintf or other formatting functions.

Example: mes(sprintf(_$("You have %d apples with John's name on it!"), countnameditem(Apple, "John"))

But this is docs, because this i not sure is it really need here.

Copy link
Member

@dastgirp dastgirp Oct 27, 2018

Choose a reason for hiding this comment

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

Or

Suggested change
mes("You have "+countnameditem(Apple, "John")+" apples with John's name on it!");
mesf("You have %d apples with John's name on it!", countnameditem(Apple, "John"));

Copy link
Member

Choose a reason for hiding this comment

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

I agree that docs should show the recommended code style, since this is where people learn/copy from. The suggested mesf() variant is the cleanest/most appropriate

@Helianthella Helianthella added type:enhancement Issue describes an enhancement or feature that should be implemented component:core:scriptengine Affecting the script engine or the script commands component:documentation Affecting the documentation in the doc/ folder labels Nov 8, 2018
@Helianthella Helianthella added this to the Release v2018.11.18 milestone Nov 8, 2018
}
next;
mes("[Item Checker]");
mes("You have "+countnameditem(Apple, "John")+" apples with John's name on it!");
Copy link
Member

Choose a reason for hiding this comment

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

I agree that docs should show the recommended code style, since this is where people learn/copy from. The suggested mesf() variant is the cleanest/most appropriate

@MishimaHaruna MishimaHaruna changed the base branch from stable to master February 1, 2019 19:20
@MishimaHaruna MishimaHaruna merged commit 5182ecd into HerculesWS:master Feb 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:core:scriptengine Affecting the script engine or the script commands component:documentation Affecting the documentation in the doc/ folder type:enhancement Issue describes an enhancement or feature that should be implemented

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants