fix: fishing#3981
Conversation
Fishing fixed just like real Tibia, including the mechanical fishing for the Isle of Evil Quest and the game achievements. You can merge this PR already, but keep on mind you need to change every water tiles in the map as shown in the image. Else, wait until Majesty upload the map update I sent to him and everything will work just fine. Everything has been debuged and tested at https://tibiatales.com using canary/tfs crystal server distribution.
📝 WalkthroughWalkthroughThis PR refactors the fishing system by introducing water-type classifications (fishable, non-fishable, dirty), transformation mappings, and specialized handlers. Both regular and mechanical fishing actions now delegate dirty-water and desert outcomes to dedicated functions, perform target transformations with decay, and apply shimmer-based cooldown logic. Yalahar Sewers nail-based fishing and elemental loot rolls are integrated into mechanical fishing. Item definitions for water and dirt items are expanded with explicit attributes, duration, and decay targets. ChangesFishing Mechanics System
Sequence Diagram(s)sequenceDiagram
participant Player
participant FishingAction
participant WaterCheck
participant DirtyWaterHandler
participant DesertHandler
participant TransformLogic
participant LootSelection
participant Storage as Storage (Shimmer Cooldown)
Player->>FishingAction: Use fishing item on water target
FishingAction->>WaterCheck: Check target water type
alt Dirty Water Spent
WaterCheck->>FishingAction: Return (magic effect)
else Dirty Water Fishable
WaterCheck->>DirtyWaterHandler: Delegate to handler
DirtyWaterHandler->>Storage: Check shimmer cooldown
DirtyWaterHandler->>TransformLogic: Transform if mapped
DirtyWaterHandler->>LootSelection: Award loot/achievement
DirtyWaterHandler->>FishingAction: Return (flow control)
else Desert Water
WaterCheck->>DesertHandler: Delegate to handler
DesertHandler->>TransformLogic: Transform and decay target
DesertHandler->>LootSelection: Award rare loot/progress
DesertHandler->>FishingAction: Return (flow control)
else Non-Fishable Water
WaterCheck->>FishingAction: Return (magic effect)
else Fishable Water
WaterCheck->>FishingAction: Continue to fishing attempt
FishingAction->>FishingAction: Check worm, compute success
alt Success with Worm
FishingAction->>TransformLogic: Apply transformToNonFishable[targetId]
TransformLogic->>LootSelection: Award transformed target reward
LootSelection->>Player: Deliver item, progress achievement
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the fishing and mechanical fishing scripts to support new mechanics, including dirty water fishing (for Shimmer Swimmers), desert fishing, and updated water ID classifications. The review feedback highlights several critical issues in mechanical_fishing.lua where the logic was copied from fishing.lua but incorrectly references worms (item ID 3492 or "worm") instead of nails (NAIL_ID) for bait checks and consumption. Additionally, a logical bug was identified in the elemental loot loop where the chance check is misplaced and the function fails to return early upon successfully finding an item, which would cause the script to continue executing regular fishing logic.
| if player:getItemCount(3492) > 0 then | ||
| player:addSkillTries(SKILL_FISHING, 1, true) | ||
| end |
There was a problem hiding this comment.
The mechanical fishing rod uses nails as bait instead of worms. Therefore, the skill tries check should verify if the player has nails (NAIL_ID) in their inventory, rather than worms (3492).
if player:getItemCount(NAIL_ID) > 0 then
player:addSkillTries(SKILL_FISHING, 1, true)
end
| if useWorms and not player:removeItem("worm", 1) then | ||
| return true | ||
| end |
There was a problem hiding this comment.
The mechanical fishing rod uses nails as bait instead of worms. This function should remove a nail (NAIL_ID) instead of a worm.
if useWorms and not player:removeItem(NAIL_ID, 1) then
return true
end
| if player:getItemCount(3492) > 0 then | ||
| player:addSkillTries(SKILL_FISHING, 1, true) | ||
| end |
There was a problem hiding this comment.
The mechanical fishing rod uses nails as bait instead of worms. The skill tries check should verify if the player has nails (NAIL_ID) in their inventory, rather than worms (3492).
if player:getItemCount(NAIL_ID) > 0 then
player:addSkillTries(SKILL_FISHING, 1, true)
end
| if useWorms and not player:removeItem("worm", 1) then | ||
| return true | ||
| end |
There was a problem hiding this comment.
The mechanical fishing rod uses nails as bait instead of worms. This function should remove a nail (NAIL_ID) instead of a worm.
if useWorms and not player:removeItem(NAIL_ID, 1) then
return true
end
| local chance = math.random(10000) | ||
| for i = 1, #elementals.chances do | ||
| local randomItem = elementals.chances[i] | ||
| if chance >= randomItem.from and chance <= randomItem.to then | ||
| player:addItem(randomItem.itemId, 1) | ||
| end | ||
| if chance > 1115 then | ||
| player:say("There was just rubbish in it.", TALKTYPE_MONSTER_SAY) | ||
| return true | ||
| end | ||
| end |
There was a problem hiding this comment.
There are two issues here:
- The check
chance > 1115is placed inside the loop, which is inefficient and logically misplaced since it executes on the first iteration and returns immediately if true. - If a player successfully fishes an elemental item (
chance <= 1115), the loop adds the item but does not returntrue. This causes the script to continue executing the rest of the regular fishing logic, which is a bug.
Moving the chance > 1115 check outside the loop and returning true immediately upon finding an item resolves both issues.
local chance = math.random(10000)
if chance > 1115 then
player:say("There was just rubbish in it.", TALKTYPE_MONSTER_SAY)
return true
end
for i = 1, #elementals.chances do
local randomItem = elementals.chances[i]
if chance >= randomItem.from and chance <= randomItem.to then
player:addItem(randomItem.itemId, 1)
return true
end
end
| if player:getItemCount(3492) > 0 and table.contains(fishableWaterIds, targetId) then | ||
| player:addSkillTries(SKILL_FISHING, 1, true) | ||
| end |
There was a problem hiding this comment.
The mechanical fishing rod uses nails as bait instead of worms. The skill tries check should verify if the player has nails (NAIL_ID) in their inventory, rather than worms (3492).
if player:getItemCount(NAIL_ID) > 0 and table.contains(fishableWaterIds, targetId) then
player:addSkillTries(SKILL_FISHING, 1, true)
end
| if useWorms and not player:removeItem("worm", 1) then | ||
| return true | ||
| end |
There was a problem hiding this comment.
The mechanical fishing rod uses nails as bait instead of worms. This should remove a nail (NAIL_ID) instead of a worm.
if useWorms and not player:removeItem(NAIL_ID, 1) then
return true
end
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@data-otservbr-global/scripts/actions/other/fishing.lua`:
- Line 44: The table dirtyWaterSpent currently contains 12560 which triggers the
early-return when targetId is in dirtyWaterSpent and prevents the later targetId
== 12560 loot branch (lootVeryRare1/lootRare1/lootCommon1) from ever running;
fix by either removing 12560 from dirtyWaterSpent or by changing the
early-return condition that checks targetId so it excludes 12560 (i.e., allow
targetId == 12560 to fall through to the loot branch), and ensure the same
change is applied consistently where similar early-return checks occur around
the other targetId handling blocks.
- Line 1: The top-level waterIds table is missing IDs produced by
transformToNonFishable, causing later checks in the fishing flow (see function
transformToNonFishable and the main fishing handler that returns false instead
of taking the non-fishable-water path) to fail; update the waterIds list (and
the equivalent ID lists around the other waterId declarations noted) to include
4809–4814 and 21314 so transformed tiles are recognized as water/non-fishable
and the code follows the correct branch (ensure all occurrences of the initial
waterIds arrays at the other mentioned blocks are updated accordingly).
In `@data-otservbr-global/scripts/actions/other/mechanical_fishing.lua`:
- Line 1: The waterIds table is missing transformed water tile IDs so
transformed non-fishable states are rejected; update the waterIds list (used in
the mechanical fishing action) to include the IDs produced by
transformToNonFishable (4809–4814 and 21314) and likewise add those IDs to the
other similar waterId checks present in the file (the blocks referenced at lines
roughly 27-41 and 56-70) so transformed sewer/water tiles follow the
non-fishable handling path.
- Line 44: The dirtyWaterSpent table includes 12560 which causes the generic
return path to fire before the specific loot branch for targetId == 12560,
making the new dirty-water loot table unreachable; fix by removing 12560 from
dirtyWaterSpent (or by moving the targetId == 12560 conditional to execute
before the generic dirtyWaterSpent return) so the specific loot branch in
mechanical_fishing.lua runs — update references to dirtyWaterSpent and the
targetId == 12560 check accordingly (also adjust the same pattern where similar
returns appear later in the file).
- Around line 259-273: The elemental loot branch can fall through to the generic
fishing flow; after awarding the elemental item inside the loop (where
player:addItem(randomItem.itemId, 1) is called) immediately return true to stop
further processing so the worm/fish/achievement flow is not executed; also
ensure the rubbish check (chance > 1115) is not placed so it executes after
successful elemental resolution (i.e., move or evaluate that check outside/after
the elementals.chances loop) so it only triggers when no elemental was granted.
In `@data/items/items.xml`:
- Around line 31964-31970: Add the missing type="trashholder" attribute to the
shallow water item with id="13988": locate the <item id="13988" name="shallow
water"> block and add an attribute entry with key/type equal to "type" and value
"trashholder" so its definition matches other fishable/transitional
shallow-water tiles and keeps fishing behavior consistent with fishing.lua.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 013235ef-ac4a-441e-8bfb-2964a92e99bf
📒 Files selected for processing (3)
data-otservbr-global/scripts/actions/other/fishing.luadata-otservbr-global/scripts/actions/other/mechanical_fishing.luadata/items/items.xml
| @@ -1,4 +1,5 @@ | |||
| local waterIds = { 622, 4597, 4598, 4599, 4600, 12561, 12563, 4601, 4602, 4609, 4610, 4611, 4612, 4613, 4614, 629, 630, 631, 632, 633, 634, 7236, 9582, 13988, 13989, 12560, 21414 } | |||
| local waterIds = { 622, 4597, 4598, 4599, 4600, 12561, 12563, 4601, 4602, 4609, 4610, 4611, 4612, 4613, 4614, 629, 630, 631, 632, 633, 634, 7236, 9582, 12560, 12561, 12562, 12563, 12558, 12559, 13988, 13989, 21414, 21312 } | |||
There was a problem hiding this comment.
Add the transformed water states to waterIds.
transformToNonFishable can turn tiles into 4809-4814 and 21314, but those IDs are not in the top-level waterIds gate. After the first successful catch, later uses hit return false at Line 181 instead of the non-fishable-water path.
Also applies to: 27-41, 56-70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@data-otservbr-global/scripts/actions/other/fishing.lua` at line 1, The
top-level waterIds table is missing IDs produced by transformToNonFishable,
causing later checks in the fishing flow (see function transformToNonFishable
and the main fishing handler that returns false instead of taking the
non-fishable-water path) to fail; update the waterIds list (and the equivalent
ID lists around the other waterId declarations noted) to include 4809–4814 and
21314 so transformed tiles are recognized as water/non-fishable and the code
follows the correct branch (ensure all occurrences of the initial waterIds
arrays at the other mentioned blocks are updated accordingly).
| } | ||
|
|
||
| local dirtyWaterFishable = { 12561, 12562, 12563 } | ||
| local dirtyWaterSpent = { 12558, 12559, 12560 } |
There was a problem hiding this comment.
12560 is blocked before its loot branch can run.
Because dirtyWaterSpent includes 12560, the early return at Line 186 makes the later targetId == 12560 block unreachable. That leaves the new lootVeryRare1/lootRare1/lootCommon1 path dead.
Possible fix
-local dirtyWaterSpent = { 12558, 12559, 12560 }
+local dirtyWaterSpent = { 12558, 12559 }Also applies to: 186-189, 232-245
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@data-otservbr-global/scripts/actions/other/fishing.lua` at line 44, The table
dirtyWaterSpent currently contains 12560 which triggers the early-return when
targetId is in dirtyWaterSpent and prevents the later targetId == 12560 loot
branch (lootVeryRare1/lootRare1/lootCommon1) from ever running; fix by either
removing 12560 from dirtyWaterSpent or by changing the early-return condition
that checks targetId so it excludes 12560 (i.e., allow targetId == 12560 to fall
through to the loot branch), and ensure the same change is applied consistently
where similar early-return checks occur around the other targetId handling
blocks.
| @@ -1,10 +1,193 @@ | |||
| local waterIds = { 622, 4597, 4598, 4599, 4600, 4601, 4602, 4609, 4610, 4611, 4612, 4613, 4614, 629, 630, 631, 632, 633, 634, 7236, 9582, 13988, 13989 } | |||
| local waterIds = { 622, 4597, 4598, 4599, 4600, 12561, 12563, 4601, 4602, 4609, 4610, 4611, 4612, 4613, 4614, 629, 630, 631, 632, 633, 634, 7236, 9582, 12560, 12561, 12562, 12563, 12558, 12559, 13988, 13989, 21414, 21312 } | |||
There was a problem hiding this comment.
Add the transformed water states to waterIds.
transformToNonFishable produces 4809-4814 and 21314, but the initial waterIds check still rejects them. That means transformed sewer/water tiles stop going through the non-fishable handling and just fall out of the action.
Also applies to: 27-41, 56-70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@data-otservbr-global/scripts/actions/other/mechanical_fishing.lua` at line 1,
The waterIds table is missing transformed water tile IDs so transformed
non-fishable states are rejected; update the waterIds list (used in the
mechanical fishing action) to include the IDs produced by transformToNonFishable
(4809–4814 and 21314) and likewise add those IDs to the other similar waterId
checks present in the file (the blocks referenced at lines roughly 27-41 and
56-70) so transformed sewer/water tiles follow the non-fishable handling path.
| } | ||
|
|
||
| local dirtyWaterFishable = { 12561, 12562, 12563 } | ||
| local dirtyWaterSpent = { 12558, 12559, 12560 } |
There was a problem hiding this comment.
12560 is unreachable here too.
The generic dirtyWaterSpent return at Line 229 fires before the later targetId == 12560 loot branch, so the new dirty-water loot table can never execute in mechanical fishing either.
Possible fix
-local dirtyWaterSpent = { 12558, 12559, 12560 }
+local dirtyWaterSpent = { 12558, 12559 }Also applies to: 229-232, 275-288
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@data-otservbr-global/scripts/actions/other/mechanical_fishing.lua` at line
44, The dirtyWaterSpent table includes 12560 which causes the generic return
path to fire before the specific loot branch for targetId == 12560, making the
new dirty-water loot table unreachable; fix by removing 12560 from
dirtyWaterSpent (or by moving the targetId == 12560 conditional to execute
before the generic dirtyWaterSpent return) so the specific loot branch in
mechanical_fishing.lua runs — update references to dirtyWaterSpent and the
targetId == 12560 check accordingly (also adjust the same pattern where similar
returns appear later in the file).
| toPosition:sendMagicEffect(CONST_ME_WATERSPLASH) | ||
| target:remove() | ||
| target:transform(target.itemid + 1) | ||
|
|
||
| local chance = math.random(10000) | ||
| for i = 1, #elementals.chances do | ||
| local randomItem = elementals.chances[i] | ||
| if chance >= randomItem.from and chance <= randomItem.to then | ||
| player:addItem(randomItem.itemId, 1) | ||
| end | ||
| if chance > 1115 then | ||
| player:say("There was just rubbish in it.", TALKTYPE_MONSTER_SAY) | ||
| return true | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Return after resolving the 9582 loot roll.
When the corpse roll awards an elemental item, this branch falls through into the generic fishing flow because only the rubbish case returns. That can still consume a worm and also grant the default fish/achievement afterward.
Possible fix
for i = 1, `#elementals.chances` do
local randomItem = elementals.chances[i]
if chance >= randomItem.from and chance <= randomItem.to then
player:addItem(randomItem.itemId, 1)
end
if chance > 1115 then
player:say("There was just rubbish in it.", TALKTYPE_MONSTER_SAY)
return true
end
end
+ return true
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| toPosition:sendMagicEffect(CONST_ME_WATERSPLASH) | |
| target:remove() | |
| target:transform(target.itemid + 1) | |
| local chance = math.random(10000) | |
| for i = 1, #elementals.chances do | |
| local randomItem = elementals.chances[i] | |
| if chance >= randomItem.from and chance <= randomItem.to then | |
| player:addItem(randomItem.itemId, 1) | |
| end | |
| if chance > 1115 then | |
| player:say("There was just rubbish in it.", TALKTYPE_MONSTER_SAY) | |
| return true | |
| end | |
| end | |
| end | |
| toPosition:sendMagicEffect(CONST_ME_WATERSPLASH) | |
| target:transform(target.itemid + 1) | |
| local chance = math.random(10000) | |
| for i = 1, `#elementals.chances` do | |
| local randomItem = elementals.chances[i] | |
| if chance >= randomItem.from and chance <= randomItem.to then | |
| player:addItem(randomItem.itemId, 1) | |
| end | |
| if chance > 1115 then | |
| player:say("There was just rubbish in it.", TALKTYPE_MONSTER_SAY) | |
| return true | |
| end | |
| end | |
| return true | |
| end |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@data-otservbr-global/scripts/actions/other/mechanical_fishing.lua` around
lines 259 - 273, The elemental loot branch can fall through to the generic
fishing flow; after awarding the elemental item inside the loop (where
player:addItem(randomItem.itemId, 1) is called) immediately return true to stop
further processing so the worm/fish/achievement flow is not executed; also
ensure the rubbish check (chance > 1115) is not placed so it executes after
successful elemental resolution (i.e., move or evaluate that check outside/after
the elementals.chances loop) so it only triggers when no elemental was granted.
| <item id="13988" name="shallow water"> | ||
| <attribute key="primarytype" value="natural tiles" /> | ||
| <attribute key="fluidsource" value="water" /> | ||
| <attribute key="effect" value="bluebubble" /> | ||
| <attribute key="duration" value="1200" /> | ||
| <attribute key="decayTo" value="13989" /> | ||
| </item> |
There was a problem hiding this comment.
Add type="trashholder" to item 13988 to keep fishing behavior consistent.
13988 is defined as a fishable/transitional shallow-water tile (duration + decayTo) but misses type="trashholder", which is present on equivalent fishable shallow-water entries in this PR. This can create inconsistent fishing outcomes for this tile family (see downstream water target usage in data-canary/scripts/actions/tools/fishing.lua, Line 1).
Suggested patch
<item id="13988" name="shallow water">
<attribute key="primarytype" value="natural tiles" />
<attribute key="fluidsource" value="water" />
+ <attribute key="type" value="trashholder" />
<attribute key="effect" value="bluebubble" />
<attribute key="duration" value="1200" />
<attribute key="decayTo" value="13989" />
</item> 📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <item id="13988" name="shallow water"> | |
| <attribute key="primarytype" value="natural tiles" /> | |
| <attribute key="fluidsource" value="water" /> | |
| <attribute key="effect" value="bluebubble" /> | |
| <attribute key="duration" value="1200" /> | |
| <attribute key="decayTo" value="13989" /> | |
| </item> | |
| <item id="13988" name="shallow water"> | |
| <attribute key="primarytype" value="natural tiles" /> | |
| <attribute key="fluidsource" value="water" /> | |
| <attribute key="type" value="trashholder" /> | |
| <attribute key="effect" value="bluebubble" /> | |
| <attribute key="duration" value="1200" /> | |
| <attribute key="decayTo" value="13989" /> | |
| </item> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@data/items/items.xml` around lines 31964 - 31970, Add the missing
type="trashholder" attribute to the shallow water item with id="13988": locate
the <item id="13988" name="shallow water"> block and add an attribute entry with
key/type equal to "type" and value "trashholder" so its definition matches other
fishable/transitional shallow-water tiles and keeps fishing behavior consistent
with fishing.lua.
There was a problem hiding this comment.
Hi, thanks for your contribution. I left the concrete fixes inline.
One behavior worth re-checking: the mechanical fishing rod now has worm-based fallback paths outside the Yalahar sewer branch. If this rod is meant to stay tied to the Isle of Evil mechanical-fish flow, those paths should either stay nail/mechanical-fish based or return early instead of behaving like regular fishing.
| @@ -1,4 +1,5 @@ | |||
| local waterIds = { 622, 4597, 4598, 4599, 4600, 12561, 12563, 4601, 4602, 4609, 4610, 4611, 4612, 4613, 4614, 629, 630, 631, 632, 633, 634, 7236, 9582, 13988, 13989, 12560, 21414 } | |||
| local waterIds = { 622, 4597, 4598, 4599, 4600, 12561, 12563, 4601, 4602, 4609, 4610, 4611, 4612, 4613, 4614, 629, 630, 631, 632, 633, 634, 7236, 9582, 12560, 12561, 12562, 12563, 12558, 12559, 13988, 13989, 21414, 21312 } | |||
There was a problem hiding this comment.
transformToNonFishable can turn fishable tiles into 4809-4814 and 21314, but this top-level gate rejects those ids before the action reaches nonFishableWaterIds. After the first successful catch on those source tiles, a later use returns false instead of taking the non-fishable-water path.
| local waterIds = { 622, 4597, 4598, 4599, 4600, 12561, 12563, 4601, 4602, 4609, 4610, 4611, 4612, 4613, 4614, 629, 630, 631, 632, 633, 634, 7236, 9582, 12560, 12561, 12562, 12563, 12558, 12559, 13988, 13989, 21414, 21312 } | |
| local waterIds = { 622, 4597, 4598, 4599, 4600, 12561, 12563, 4601, 4602, 4609, 4610, 4611, 4612, 4613, 4614, 4809, 4810, 4811, 4812, 4813, 4814, 629, 630, 631, 632, 633, 634, 7236, 9582, 12560, 12561, 12562, 12563, 12558, 12559, 13988, 13989, 21414, 21312, 21314 } |
| } | ||
|
|
||
| local dirtyWaterFishable = { 12561, 12562, 12563 } | ||
| local dirtyWaterSpent = { 12558, 12559, 12560 } |
There was a problem hiding this comment.
Keeping 12560 in the generic spent-water list makes the dedicated targetId == 12560 loot branch below unreachable, because the action returns at the early spent-water check first.
| local dirtyWaterSpent = { 12558, 12559, 12560 } | |
| local dirtyWaterSpent = { 12558, 12559 } |
| @@ -1,10 +1,193 @@ | |||
| local waterIds = { 622, 4597, 4598, 4599, 4600, 4601, 4602, 4609, 4610, 4611, 4612, 4613, 4614, 629, 630, 631, 632, 633, 634, 7236, 9582, 13988, 13989 } | |||
| local waterIds = { 622, 4597, 4598, 4599, 4600, 12561, 12563, 4601, 4602, 4609, 4610, 4611, 4612, 4613, 4614, 629, 630, 631, 632, 633, 634, 7236, 9582, 12560, 12561, 12562, 12563, 12558, 12559, 13988, 13989, 21414, 21312 } | |||
There was a problem hiding this comment.
transformToNonFishable can produce 4809-4814 and 21314, but this top-level waterIds check runs before nonFishableWaterIds. Transformed water tiles will be rejected by the action instead of showing the expected non-fishable-water effect.
| local waterIds = { 622, 4597, 4598, 4599, 4600, 12561, 12563, 4601, 4602, 4609, 4610, 4611, 4612, 4613, 4614, 629, 630, 631, 632, 633, 634, 7236, 9582, 12560, 12561, 12562, 12563, 12558, 12559, 13988, 13989, 21414, 21312 } | |
| local waterIds = { 622, 4597, 4598, 4599, 4600, 12561, 12563, 4601, 4602, 4609, 4610, 4611, 4612, 4613, 4614, 4809, 4810, 4811, 4812, 4813, 4814, 629, 630, 631, 632, 633, 634, 7236, 9582, 12560, 12561, 12562, 12563, 12558, 12559, 13988, 13989, 21414, 21312, 21314 } |
There was a problem hiding this comment.
Did you test it? Works fine for me as it is.
| } | ||
|
|
||
| local dirtyWaterFishable = { 12561, 12562, 12563 } | ||
| local dirtyWaterSpent = { 12558, 12559, 12560 } |
There was a problem hiding this comment.
12560 is handled by the generic spent-water return before it can reach the dedicated targetId == 12560 loot branch, so that branch cannot run.
| local dirtyWaterSpent = { 12558, 12559, 12560 } | |
| local dirtyWaterSpent = { 12558, 12559 } |
| local chance = math.random(10000) | ||
| for i = 1, #elementals.chances do | ||
| local randomItem = elementals.chances[i] | ||
| if chance >= randomItem.from and chance <= randomItem.to then | ||
| player:addItem(randomItem.itemId, 1) | ||
| end | ||
| if chance > 1115 then | ||
| player:say("There was just rubbish in it.", TALKTYPE_MONSTER_SAY) | ||
| return true | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
When the elemental roll awards an item, the function continues into the generic fishing flow because only the rubbish path returns. That can consume normal bait and grant the default fishing reward after the corpse roll has already been resolved.
| local chance = math.random(10000) | |
| for i = 1, #elementals.chances do | |
| local randomItem = elementals.chances[i] | |
| if chance >= randomItem.from and chance <= randomItem.to then | |
| player:addItem(randomItem.itemId, 1) | |
| end | |
| if chance > 1115 then | |
| player:say("There was just rubbish in it.", TALKTYPE_MONSTER_SAY) | |
| return true | |
| end | |
| end | |
| end | |
| local chance = math.random(10000) | |
| if chance > 1115 then | |
| player:say("There was just rubbish in it.", TALKTYPE_MONSTER_SAY) | |
| return true | |
| end | |
| for i = 1, #elementals.chances do | |
| local randomItem = elementals.chances[i] | |
| if chance >= randomItem.from and chance <= randomItem.to then | |
| player:addItem(randomItem.itemId, 1) | |
| return true | |
| end | |
| end | |
| return true | |
| end |
| <item id="13988" name="shallow water"> | ||
| <attribute key="primarytype" value="natural tiles" /> | ||
| <attribute key="fluidsource" value="water" /> | ||
| <attribute key="effect" value="bluebubble" /> | ||
| <attribute key="duration" value="1200" /> | ||
| <attribute key="decayTo" value="13989" /> | ||
| </item> |
There was a problem hiding this comment.
13988 is now a transitional shallow-water tile with duration and decayTo, but it is missing the trashholder type that the equivalent shallow-water entries in this diff have. Adding it keeps this tile family consistent.
| <item id="13988" name="shallow water"> | |
| <attribute key="primarytype" value="natural tiles" /> | |
| <attribute key="fluidsource" value="water" /> | |
| <attribute key="effect" value="bluebubble" /> | |
| <attribute key="duration" value="1200" /> | |
| <attribute key="decayTo" value="13989" /> | |
| </item> | |
| <item id="13988" name="shallow water"> | |
| <attribute key="primarytype" value="natural tiles" /> | |
| <attribute key="fluidsource" value="water" /> | |
| <attribute key="type" value="trashholder" /> | |
| <attribute key="effect" value="bluebubble" /> | |
| <attribute key="duration" value="1200" /> | |
| <attribute key="decayTo" value="13989" /> | |
| </item> |
I think mechanical fishing rod shouldn't be added yet. In real tibia you can't use it for regular fishing, only for the mechanical fish. |
|
Hi Paco. Could you please provide me with a list of all the IDs that need to be changed? It would really help me if it could be in the format below: id 4609 -> 4597 I will create a Lua script to perform this function. Thanks! |
Hello, sure! Into these: Hope it helps! :D Good Luck doing that, please don't fell into desperation, it may take you a while to update all tiles. |
3 participants
Fishing fixed just like real Tibia, including the mechanical fishing for the Isle of Evil Quest and the game achievements.
You can merge this PR already, but keep on mind you need to change every water tiles in the map as shown in the image.
Else, wait until Majesty upload the map update I sent to him and everything will work just fine.
Everything has been debuged and tested at https://tibiatales.com/