getpartymember upgrade#3305
Merged
MishimaHaruna merged 4 commits intoHerculesWS:getpartymemberfrom Aug 31, 2024
Merged
Conversation
MishimaHaruna
requested changes
Jul 30, 2024
Member
MishimaHaruna
left a comment
There was a problem hiding this comment.
Hello, and thank you for sharing! Please do update the scripts when you have time to do so (in a separate commit).
I think it looks good. Even if still in draft, I went alread and added some review comments. Please have a look at them.
Co-authored-by: Haru <[email protected]>
Co-authored-by: Haru <[email protected]>
Member
MishimaHaruna
left a comment
There was a problem hiding this comment.
Looks good! Will you update the scripts (luckily there aren't a lot of calls to this command, I only see 16) and the script_commands.txt documentation?
This was referenced Aug 2, 2024
MishimaHaruna
approved these changes
Aug 31, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Prelude
Changes Proposed
Hi, I'd like to share my version of the
getpartymemberscript command, similar togetunits.This updated version no longer uses global temporary variables, which are prone to being overwritten, especially when multiple players call the function simultaneously. Instead, it requires the scripter to declare an array as an argument in the 3rd parameter of the command.
Additionally, this change introduces the following constants:
PT_MEMBER_NAME,PT_MEMBER_CHARID, andPT_MEMBER_ACCID. The function now also returns the size of the party.Sample usage:
The existing script will be deprecated, but I will gladly update all current NPC scripts to use this new version if approved. I can apply similar updates to
getguildmemberandgetmobdropsas well.I think this approach aligns more closely with Herc's style.
Issues addressed:
Wrong data when multiple players call the command.