Add correct behavior to all non-Mamook Incursion doors in Mamook - #11484
TiberonKalkaz wants to merge 1 commit into
Conversation
4bcb5e3 to
ffea185
Compare
| return xi.item.SKELETON_KEY | ||
| elseif npcUtil.tradeHasExactly(trade, xi.item.LIVING_KEY) then | ||
| return xi.item.LIVING_KEY | ||
| elseif npcUtil.tradeHasExactly(trade, xi.item.SET_OF_THIEFS_TOOLS) then |
There was a problem hiding this comment.
use tradeMatches function
There was a problem hiding this comment.
will do
is tradeHadExactly on the deprecation path? If so I can take a swing at wiping it out in a future PR
There was a problem hiding this comment.
Yes, the tl;dr is our old trade functions are really not good for the codebase as a whole for trading due to how item locking works. So we're moving away from locking immediately and now we check up front and lock/consume them later.
| local mamookGlobal = {} | ||
|
|
||
| local doorOpenTime = 15 -- Doors stay open for 15s | ||
| local noValidationFlag = 0x8000 |
There was a problem hiding this comment.
use the values directly
| end | ||
|
|
||
| ----------------------------------- | ||
| -- Helper fcn to find success rate based on level and thf tool |
There was a problem hiding this comment.
will be wiped out by the next comment
| return | ||
| end | ||
|
|
||
| local rate = getDoorSuccessRate(thfKeyType, player:getMainLvl()) |
There was a problem hiding this comment.
a helper function called inside a helper function which is also only called once in the whole file.
Something tells me all of this could be better organized
There was a problem hiding this comment.
will collapse helper fcns
| end | ||
|
|
||
| -- handle the Tanscale Key | ||
| if npcUtil.tradeHasExactly(trade, xi.item.MAMOOK_TANSCALE_KEY) then |
| end | ||
|
|
||
| entity.onEventUpdate = function(player, csid, option, npc) | ||
| end |
There was a problem hiding this comment.
Dont add empty functions to npc or mob scripts, please
There was a problem hiding this comment.
yeeted - was left over from testing
|
|
||
| local lockedSideOfDoor = function(player) | ||
| return player:getXPos() > -240.000 | ||
| end |
There was a problem hiding this comment.
Please no. No "1 line functions that consist of a single check" and no "local variables defined at the very top outside of any concrete function" specially not when its only used once.
There was a problem hiding this comment.
will update each npc file
i'll take a backlog to update the pattern in arrapago reef npcs as well (I think it was there)
|
|
||
| entity.onTrade = function(player, npc, trade) | ||
| if | ||
| npcUtil.tradeHasExactly(trade, { xi.item.MAMOOK_SILVERSCALE_KEY, xi.item.MAMOOK_TANSCALE_KEY, xi.item.MAMOOK_BLACKSCALE_KEY }) and |
ffea185 to
9b26bf1
Compare
| local entity = {} | ||
|
|
||
| entity.onTrade = function(player, npc, trade) | ||
| end |
There was a problem hiding this comment.
remove empty functions
I affirm:
What does this pull request do?
Steps to test these changes
For the Ebony Doors
For Red Bell Mahogany Door
For Galool Ja Ja's Door
For Galool Ja Ja's North Room Door
Out of Scope:
Captutes
Open Bugs:
Closes Bug 2712 reported against Pheonix