Umbala Script Update:

[dastgirp]

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 will be closed if the above-mentioned criteria are not fulfilled.

Changes Proposed

Moved delitem before getitem.
So even if somebody manages to do some kind of manipulation to items, delitem would stop the script execution

Affected Branches:

• master
  Issues addressed:

• Quest Umbala - Ore Downgrading - delitem() after getitem() (Dupe Bug) #1934

Known Issues and TODO List

[HerculesWSAPI]

This change is 

[Asheraf]

I believe adding a if (countitem() < x) close(); would be better, making the delitem() stop script execution would not show close button to the user which is not very nice.

[dastgirp]

1. we should always prefer delitem before getitem
2. countitem checks are there, script will stop execution if the player tries to cheat the npc through some ways.

Anyways, open to suggestions, if everyone's ok with countitem, then I would do like that

[Asheraf]

@dastgir i agree about delitem() being first however the issue in this script is not that, the problem is there is a pause between the check for item and the delete of item (multiple next() in this example) so the proper fix for such an issue is to either make a double item check if you want to keep the script on the official structure or move the check to exactly before the item delete, stopping the script execution would require the player to restart his client which is not a good thing assuming that someone may unintentionally lose the items before delete as this is now exploitable via rodex but would be the same issue with any future official/custom system that allows moving inventory items.

[4144]

Yes main issue is in script. First should be item deleted and and without pauses after it should be given new items. If need second check can be added.

[dastgirp]

@Asheraf please check