getmobdrops refactor

[jasonch35]

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

Avoid use of global variables that tend to get overwritten when command is called by 2 or more players simultaneously.
The command now requires an array as the second or third argument.
The third argument is optional and, if provided, will copy the drop rates into the array.
Updated documentation to reflect these changes.

Usage:

.@count = getmobdrops(.@mob_id, .@item, .@rate);
if (.@count) {
	mes(getmonsterinfo(.@mob_id, MOB_NAME) + " - " + .@count + " drops found:");
	for (.@i = 0; .@i < .@count; ++.@i) {
		mes(.@item[.@i] + " (" + getitemname(.@item[.@i]) + ") " + .@rate[.@i]/100 + ((.@rate[.@i]%100 < 10) ? ".0":".") + .@rate[.@i]%100 + "%");
	}
} else {
	mes("No drops found.");
}
close();

Issues addressed:
Wrong data when multiple players call the command.

[skyleo]

May want to consider to deprecate the command instead, to not screw over existing scripts too much. And instead use a new name getmonsterdrops, so that when the deprecated command is used, a warning is shown on map-server saying to use getmonsterdrops instead.

Alternatively one can go the hard-route but you really have to ensure and be aware of what is shown to people using older scripts then, it has to not silently fail 100%. And it definitely would be good if it fails at parsing already and not at runtime, otherwise people may start the server and think all is fine after the update containing this change.

Ideally this PR should already replace all existing scripts with the new command.

Good work though.

[jasonch35]

@skyleo Good point, I hadn’t considered that. However, both getpartymember and getguildmember underwent similar upgrades and were merged without deprecation. 😄 Should I still create a new command and deprecate getmobdrops?

[skyleo]

@skyleo Good point, I hadn’t considered that. However, both getpartymember and getguildmember underwent similar upgrades and were merged without deprecation. 😄 Should I still create a new command and deprecate getmobdrops?

Although I don't fully remember the behavior, the script engine may fail older scripts on parsing already due to the mandatory parameter amount being different between the older and your version of the script command.

So I guess you may be fine. May be worth testing though. ;)

[MishimaHaruna]

Although I don't fully remember the behavior, the script engine may fail older scripts on parsing already due to the mandatory parameter amount being different between the older and your version of the script command.

That's right, since the old command had only one argument and the new one has two mandatory ones and an optional one, it'll already error out when trying to load an old script, forcing the user to notice and update the script. There's no need to deprecate and renamed the command in this case

[MishimaHaruna]

Thank you, looks good to me now, I'll merge it