feat: the order of the lion quest#3992
Open
Paco161315 wants to merge 10 commits into
Open
Conversation
Complete Quest+ Achievements + Questlog + Bosses. Removed the storage reseter when talking to Kesar with the keywords because I think it's stupid. Everything has been debuged and tested at https://tibiatales.com using canary/tfs crystal server distribution.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds "The Order of the Lion" quest: quest catalog, storages, many NPCs (day/night), creature and move events, arena/drume encounter and actions, teleports/elevators, chest rewards, world spawn and door/table updates. ChangesThe Order of the Lion Quest System
Sequence DiagramsequenceDiagram
participant Player as Player
participant NPC as NPCHandler
participant Quest as QuestCatalog
participant World as WorldEngine
Player->>NPC: talk / use action
NPC->>Quest: update storages / trigger quest events
Quest->>World: spawn monsters / teleport / grant rewards
World->>Player: teleport / effects / rewards
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 implements 'The Order of the Lion' quest line, introducing a new quest catalog, numerous day/night NPCs, and various quest-related action, creaturescript, and moveevent scripts. The code review identified several critical bugs and improvement opportunities, including a storage key mismatch (AccessEasternSide vs AccessEastSide) in door_quest.lua, a duplicate key [6] in the quest catalog, and invalid table arguments passed to the Position constructor in action-bounac_entrance.lua. Additionally, feedback was provided regarding performance inefficiencies in moveevent_fugue.lua, undefined global callbacks in several NPC files, a lack of capacity checks in action_evrard_quest.lua that could lock players out of items, a missing event registration for Kesar's immortality, a mismatched cooldown message, and a lack of player feedback on lever failure.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| itemPos = { { x = 33793, y = 31388, z = 6 } }, | ||
| }, | ||
| [Storage.Quest.U12_40.TheOrderOfTheLion.AccessEastSide] = { | ||
| [Storage.Quest.U12_40.TheOrderOfTheLion.AccessEasternSide] = { |
There was a problem hiding this comment.
The storage key AccessEasternSide is used here, but all other files (such as bounac_guard.lua, evrard.lua, and moveevent_castle.lua) use AccessEastSide. This mismatch will prevent players from opening the quest doors even after being granted access by the guard or Evrard.
[Storage.Quest.U12_40.TheOrderOfTheLion.AccessEastSide] = {
| end, | ||
| }, | ||
| }, | ||
| [6] = { |
There was a problem hiding this comment.
There is a duplicate key [6] in the missions table. The mission "Mission: Provisions for the Butcher" overwrites "Mission: The Hunt for Dal", which will break the quest log and progress tracking for the latter. It should be changed to [7].
[7] = {
Comment on lines
+8
to
+15
| player:teleportTo({ x = 32423, y = 32448, z = 7 }) | ||
| toPosition:sendMagicEffect(CONST_ME_WATERSPLASH) | ||
| Position({ x = 32423, y = 32448, z = 7 }):sendMagicEffect(CONST_ME_WATERSPLASH) | ||
| end | ||
| elseif item:getActionId() == 59603 then | ||
| player:teleportTo({ x = 33183, y = 31756, z = 7 }) | ||
| Position({ x = 33183, y = 31756, z = 7 }):sendMagicEffect(CONST_ME_WATERSPLASH) | ||
| toPosition:sendMagicEffect(CONST_ME_WATERSPLASH) |
There was a problem hiding this comment.
Passing a table { x = ..., y = ..., z = ... } to the Position constructor is invalid and will cause a runtime error when calling :sendMagicEffect() on it. The constructor expects individual coordinates as arguments, e.g., Position(32423, 32448, 7).
player:teleportTo(Position(32423, 32448, 7))
toPosition:sendMagicEffect(CONST_ME_WATERSPLASH)
Position(32423, 32448, 7):sendMagicEffect(CONST_ME_WATERSPLASH)
end
elseif item:getActionId() == 59603 then
player:teleportTo(Position(33183, 31756, 7))
Position(33183, 31756, 7):sendMagicEffect(CONST_ME_WATERSPLASH)
toPosition:sendMagicEffect(CONST_ME_WATERSPLASH)
Comment on lines
+13
to
+28
| local function fugueAlreadyExists() | ||
| for x = spawnAreaMin.x, spawnAreaMax.x do | ||
| for y = spawnAreaMin.y, spawnAreaMax.y do | ||
| local pos = Position(x, y, spawnAreaMin.z) | ||
| local tile = Tile(pos) | ||
| if tile then | ||
| for _, creature in ipairs(tile:getCreatures()) do | ||
| if creature:getName():lower() == "fugue" then | ||
| return true | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| return false | ||
| end |
There was a problem hiding this comment.
The fugueAlreadyExists function uses nested loops to scan 861 tiles (41 * 21) on every step-in event. This is highly inefficient and can cause severe lag spikes. Instead, use Game.getSpectators to retrieve all creatures in the area in a single, highly optimized engine call.
local function fugueAlreadyExists()
local center = Position(32378, 32475, 2)
local spectators = Game.getSpectators(center, false, false, 20, 20, 10, 10)
for _, spectator in ipairs(spectators) do
if spectator:isMonster() and spectator:getName():lower() == "fugue" then
return true
end
end
return false
end
| npcHandler:onCloseChannel(npc, creature) | ||
| end | ||
|
|
||
| npcHandler:setCallback(CALLBACK_MESSAGE_DEFAULT, creatureSayCallback) |
There was a problem hiding this comment.
The line npcHandler:setCallback(CALLBACK_MESSAGE_DEFAULT, creatureSayCallback) is present, but creatureSayCallback is never defined in this file. This references an undefined global variable, which is bad practice and can lead to issues. Since this NPC does not have custom dialogue callbacks, this line should be removed. Note that the same issue applies to innkeeper_alphonse.lua, kesars_valet_day.lua, and kesars_valet_night.lua.
Comment on lines
+21
to
+33
| function evrardItems.onUse(player, item, fromPosition, target, toPosition, isHotkey) | ||
| if player:getStorageValue(item.uid) ~= 1 then | ||
| local newItem = player:addItem(reward[item.uid], 1) | ||
| if newItem and parchmentText[reward[item.uid]] then | ||
| newItem:setAttribute(ITEM_ATTRIBUTE_TEXT, parchmentText[reward[item.uid]]) | ||
| end | ||
| player:setStorageValue(item.uid, 1) | ||
| player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "You have found a " .. ItemType(reward[item.uid]):getName() .. ".") | ||
| else | ||
| player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "The chest is empty.") | ||
| end | ||
| return true | ||
| end |
There was a problem hiding this comment.
If the player does not have enough capacity or container slots, player:addItem might fail or return nil. However, the script still sets the storage value to 1 and tells the player they found the item, permanently locking them out of obtaining it. You should check if the player has enough capacity and if the item was successfully added before setting the storage.
function evrardItems.onUse(player, item, fromPosition, target, toPosition, isHotkey)
if player:getStorageValue(item.uid) ~= 1 then
local itemId = reward[item.uid]
if player:getFreeCapacity() < ItemType(itemId):getWeight() then
player:sendCancelMessage("You do not have enough capacity.")
return true
end
local newItem = player:addItem(itemId, 1)
if not newItem then
player:sendCancelMessage("You do not have enough room in your inventory.")
return true
end
if parchmentText[itemId] then
newItem:setAttribute(ITEM_ATTRIBUTE_TEXT, parchmentText[itemId])
end
player:setStorageValue(item.uid, 1)
player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "You have found a " .. ItemType(itemId):getName() .. ".")
else
player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "The chest is empty.")
end
return true
end
Comment on lines
+35
to
+38
| if totalCommanders == 1 then | ||
| Game.createMonster("Kesar", Position(32444, 32515, 7), false, true) | ||
| Game.createMonster("Drume", Position(32444, 32516, 7), false, true) | ||
| end |
There was a problem hiding this comment.
The KesarImmortal event is registered globally, but it is never registered to the spawned "Kesar" monster instance. Without registering the event on the creature, Kesar will not be immortal during the fight.
if totalCommanders == 1 then
local kesar = Game.createMonster("Kesar", Position(32444, 32515, 7), false, true)
if kesar then
kesar:registerEvent("KesarImmortal")
end
Game.createMonster("Drume", Position(32444, 32516, 7), false, true)
end
Comment on lines
+3
to
+6
| if creature:isPlayer() and not creature:canFightBoss("Drume") then | ||
| creature:teleportTo(fromPosition, true) | ||
| creature:sendTextMessage(MESSAGE_EVENT_ADVANCE, "You've been into the skirmish in the last 10 hours.") | ||
| end |
There was a problem hiding this comment.
The message says "You've been into the skirmish in the last 10 hours.", but the actual boss cooldown set in action_drume.lua is typically 20 hours (or BOSS_DEFAULT_TIME_TO_FIGHT_AGAIN). This hardcoded text should be updated to "20 hours" to match the actual cooldown.
if creature:isPlayer() and not creature:canFightBoss("Drume") then
creature:teleportTo(fromPosition, true)
creature:sendTextMessage(MESSAGE_EVENT_ADVANCE, "You've been into the skirmish in the last 20 hours.")
end
Comment on lines
+70
to
+74
| for _, pi in pairs(players) do | ||
| if pi:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.KesarMission) < 4 then | ||
| return true | ||
| end | ||
| end |
There was a problem hiding this comment.
If a player in the team does not have the required quest progress (KesarMission < 4), the lever script returns true and does nothing, but it does not send any feedback or cancel message to the player. This will confuse players. You should send a cancel message explaining why the lever failed.
for _, pi in pairs(players) do
if pi:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.KesarMission) < 4 then
player:sendCancelMessage("Someone in your team does not have the required quest progress.")
player:getPosition():sendMagicEffect(CONST_ME_POFF)
return true
end
end
…/Paco161315/canary into feat-the-order-of-the-lion-quest
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
data-otservbr-global/npc/wes_the_blacksmith_day.lua (1)
81-86: 💤 Low valueHand-over prompt missing the item-count guard.
Unlike Fral/Jehan (which check
getItemCount(...) >= Nbefore prompting), this branch only checksWesTask == 1, so Wes claims "You have about 20 broken longbows there" even when the player has none; the subsequentyesthen silently no-ops atremoveItem(34161, 20). Consider gating the prompt on the item count for consistent UX.💡 Add item-count guard
- elseif MsgContains(message, "yselda") and player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.WesTask) == 1 then + elseif MsgContains(message, "yselda") and player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.WesTask) == 1 and player:getItemCount(34161) >= 20 then🤖 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/npc/wes_the_blacksmith_day.lua` around lines 81 - 86, The prompt is shown unconditionally when WesTask == 1, causing a misleading request even if the player lacks 20 broken longbows; update the branch that checks MsgContains(message, "yselda") and player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.WesTask) == 1 to first check the player's item count (e.g., player:getItemCount(34161) >= 20) and only call npcHandler:say(...) and npcHandler:setTopic(playerId, 1) if the count is sufficient; otherwise, reply with a different npcHandler:say(...) telling the player they don't have enough items (so the later yes/removeItem(34161, 20) logic is only reachable when the player actually has 20).data-otservbr-global/npc/innkeeper_alphonse.lua (1)
60-60: 💤 Low valueRemove stray
setCallbackreferencing an undefinedcreatureSayCallback.This file defines no
creatureSayCallback, so the reference resolves toniland the call is a no-op copy-paste leftover. Drop it for a pure shop NPC.♻️ Proposed cleanup
-npcHandler:setCallback(CALLBACK_MESSAGE_DEFAULT, creatureSayCallback) npcHandler:addModule(FocusModule:new(), npcConfig.name, true, true, true)🤖 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/npc/innkeeper_alphonse.lua` at line 60, The file contains a stray call to npcHandler:setCallback(CALLBACK_MESSAGE_DEFAULT, creatureSayCallback) referencing an undefined creatureSayCallback; remove that line so the shop NPC does not register a nil callback and remains a pure shop NPC (search for the exact npcHandler:setCallback(..., creatureSayCallback) invocation and delete it).data-otservbr-global/npc/kesars_valet_day.lua (1)
58-58: 💤 Low valueStray
setCallbackwith undefinedcreatureSayCallback.Same copy-paste artifact as the innkeeper script —
creatureSayCallbackis undefined here (resolves tonil). Remove this line since the valet has no message callback.♻️ Proposed cleanup
-npcHandler:setCallback(CALLBACK_MESSAGE_DEFAULT, creatureSayCallback) npcHandler:addModule(FocusModule:new(), npcConfig.name, true, true, true)🤖 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/npc/kesars_valet_day.lua` at line 58, The line calling npcHandler:setCallback(CALLBACK_MESSAGE_DEFAULT, creatureSayCallback) references an undefined function creatureSayCallback and is a stray copy-paste; remove that setCallback invocation from the Kesars valet script so the npcHandler is not assigned a nil message callback (search for npcHandler:setCallback and creatureSayCallback in this file to locate and delete this specific line).data-otservbr-global/npc/evrard.lua (1)
1-117: ⚖️ Poor tradeoffDay/night variants duplicate the entire quest dialogue verbatim.
evrard.luaandevrard_the_miller.luaare byte-identical except forinternalNpcNameandrespawnType.period. Any logic fix (e.g. the item-loss bug above) must be applied in both places, which is error-prone. Consider extracting the sharedcreatureSayCallbackinto a common lib module that both NPC scripts require.🤖 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/npc/evrard.lua` around lines 1 - 117, The two NPC scripts duplicate all quest dialogue and logic (only internalNpcName and respawnType.period differ), so extract the shared behavior into a reusable module: move creatureSayCallback (and any shared npcHandler setup like setMessage, setCallback, and addModule) into a common Lua file (e.g., a module that exports a function to attach the callback and messages) and have both evrard.lua and evrard_the_miller.lua require that module and only define their unique bits (internalNpcName, npcConfig.respawnType.period, npcConfig.outfit, then call the shared attach function), ensuring npcType:register still uses the combined npcConfig after requiring the shared logic.data-otservbr-global/scripts/quests/the_order_of_lion_quest/creaturescripts_drume_kill.lua (1)
3-4: 💤 Low valueUnused
areaMin/areaMax— remove or wire up the area check.These positions are declared but never used. They look like a leftover from an intended "Drume died inside the arena" guard. Since Drume is only spawned in the arena this is currently harmless, but the dead variables should either be removed or used to scope the mission advance.
Want me to add an area-bounds check around the
KesarMissionadvance, or just drop the unused locals?🤖 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/quests/the_order_of_lion_quest/creaturescripts_drume_kill.lua` around lines 3 - 4, The variables areaMin/areaMax are declared but unused; wire them into the Drume kill creaturescript by guarding the KesarMission advance with an arena-bounds check. In the Drume death handler (e.g., onKill), get the death position (target/getPosition) and only advance KesarMission if that position is within areaMin..areaMax (pos:isInRange(areaMin, areaMax)); otherwise do nothing. If after adding this guard there are no other references, remove any remaining unused locals.
🤖 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/lib/core/quests/catalog/052_the_order_of_the_lion_quest.lua`:
- Around line 77-86: The missions table contains a duplicated numeric key ([6])
which overrides the earlier mission "Mission: The Hunt for Dal"; update the
second occurrence (the entry with name "Mission: Provisions for the Butcher" and
storageId Storage.Quest.U12_40.TheOrderOfTheLion.FralTask) to use the correct
next index (e.g., [7]) so each mission entry in quest.missions has a unique key
and the Dal mission remains present.
In `@data-otservbr-global/npc/bounac_guard.lua`:
- Around line 63-65: In creatureSayCallback, guard against Player(creature)
returning nil before calling player:getId(); assign local player =
Player(creature), then if not player then return false (or simply exit the
callback) to avoid dereferencing a non-player; update any subsequent logic in
creatureSayCallback that uses playerId or player methods to only run after this
nil-check so non-player creatures won't trigger runtime errors.
- Around line 74-104: The confirmation branch checking message:lower() == "yes"
and npcHandler:getTopic(playerId) == 1 is unreachable because you never set
npcHandler:setTopic(playerId, 1) when prompting the player; update the code path
that says the guard's challenge (the npcHandler:say block that tells the player
to "Earn our trust and I may let you pass.") to call
npcHandler:setTopic(playerId, 1) after speaking (and keep the existing
npcHandler:setTopic(playerId, 0) reset after handling "yes"); this targets the
npcHandler:setTopic / npcHandler:getTopic flow and the grantAccess function so
the yes-confirmation becomes reachable.
In `@data-otservbr-global/npc/evrard.lua`:
- Around line 60-62: In creatureSayCallback, guard against Player(creature)
being nil before calling player:getId(); check that player is non-nil (and
optionally isPlayer()) and return/exit the handler early if not, so calls to
player:getId() (and any subsequent player methods) are only executed for valid
player objects.
- Line 84: The condition currently calls player:removeItem(635,1) and
player:removeItem(2822,1) inside an && chain which can short-circuit and consume
one item if the second removal fails; change the logic so you first check that
the player possesses both items (e.g., with player:getItemCount or an explicit
possession check for item IDs 635 and 2822 and the existing storage checks via
player:getStorageValue) and only then call player:removeItem for each item
sequentially; ensure both removals are performed only after successful
possession checks and handle removal failure by aborting and returning the
original failure message.
In `@data-otservbr-global/npc/wes_the_blacksmith_day.lua`:
- Around line 64-80: The longbow-instruction branch is unreachable because
within the MsgContains(message, "yselda") and WesTask < 1 block, the topic == 0
path sets WesTask to 1 without advancing the topic, so topic == 1 never
executes. In the npcHandler:getTopic(playerId) == 0 branch, do not set
Storage.Quest.U12_40.TheOrderOfTheLion.WesTask yet; instead advance dialogue by
calling npcHandler:setTopic(playerId, 1). Then, in the
npcHandler:getTopic(playerId) == 1 branch, present the “broken longbows”
instruction, set Storage.Quest.U12_40.TheOrderOfTheLion.WesTask to 1, and reset
npcHandler:setTopic(playerId, 0).
In
`@data-otservbr-global/scripts/quests/the_order_of_lion_quest/moveevent_fugue.lua`:
- Around line 13-28: The fugueAlreadyExists function calls ipairs on
tile:getCreatures() without checking if it returns nil, risking a crash if the
creature list is absent. To fix this, first assign the result of
tile:getCreatures() to a local variable and check if it is not nil before
iterating with ipairs. This guards against nil values and prevents runtime
errors during the scan loop.
In `@data-otservbr-global/world/otservbr-monster.xml`:
- Around line 42947-42949: The outer monster spawn has a centerz="6" while the
inner <monster name="Ancient Lion Knight" ... z="7"> uses z="7" causing a height
mismatch; update the outer <monster centerx="32439" centery="32470" centerz="6"
radius="1"> to use centerz="7" so the centerz matches the inner monster z and
the spawn height is correct.
---
Nitpick comments:
In `@data-otservbr-global/npc/evrard.lua`:
- Around line 1-117: The two NPC scripts duplicate all quest dialogue and logic
(only internalNpcName and respawnType.period differ), so extract the shared
behavior into a reusable module: move creatureSayCallback (and any shared
npcHandler setup like setMessage, setCallback, and addModule) into a common Lua
file (e.g., a module that exports a function to attach the callback and
messages) and have both evrard.lua and evrard_the_miller.lua require that module
and only define their unique bits (internalNpcName,
npcConfig.respawnType.period, npcConfig.outfit, then call the shared attach
function), ensuring npcType:register still uses the combined npcConfig after
requiring the shared logic.
In `@data-otservbr-global/npc/innkeeper_alphonse.lua`:
- Line 60: The file contains a stray call to
npcHandler:setCallback(CALLBACK_MESSAGE_DEFAULT, creatureSayCallback)
referencing an undefined creatureSayCallback; remove that line so the shop NPC
does not register a nil callback and remains a pure shop NPC (search for the
exact npcHandler:setCallback(..., creatureSayCallback) invocation and delete
it).
In `@data-otservbr-global/npc/kesars_valet_day.lua`:
- Line 58: The line calling npcHandler:setCallback(CALLBACK_MESSAGE_DEFAULT,
creatureSayCallback) references an undefined function creatureSayCallback and is
a stray copy-paste; remove that setCallback invocation from the Kesars valet
script so the npcHandler is not assigned a nil message callback (search for
npcHandler:setCallback and creatureSayCallback in this file to locate and delete
this specific line).
In `@data-otservbr-global/npc/wes_the_blacksmith_day.lua`:
- Around line 81-86: The prompt is shown unconditionally when WesTask == 1,
causing a misleading request even if the player lacks 20 broken longbows; update
the branch that checks MsgContains(message, "yselda") and
player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.WesTask) == 1 to
first check the player's item count (e.g., player:getItemCount(34161) >= 20) and
only call npcHandler:say(...) and npcHandler:setTopic(playerId, 1) if the count
is sufficient; otherwise, reply with a different npcHandler:say(...) telling the
player they don't have enough items (so the later yes/removeItem(34161, 20)
logic is only reachable when the player actually has 20).
In
`@data-otservbr-global/scripts/quests/the_order_of_lion_quest/creaturescripts_drume_kill.lua`:
- Around line 3-4: The variables areaMin/areaMax are declared but unused; wire
them into the Drume kill creaturescript by guarding the KesarMission advance
with an arena-bounds check. In the Drume death handler (e.g., onKill), get the
death position (target/getPosition) and only advance KesarMission if that
position is within areaMin..areaMax (pos:isInRange(areaMin, areaMax)); otherwise
do nothing. If after adding this guard there are no other references, remove any
remaining unused locals.
🪄 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: 7ac2f6ef-810c-4439-a7c0-5bf5a587b0e0
📒 Files selected for processing (42)
data-otservbr-global/lib/core/quests/catalog/047_grave_danger.luadata-otservbr-global/lib/core/quests/catalog/052_the_order_of_the_lion_quest.luadata-otservbr-global/lib/core/quests/catalog/init.luadata-otservbr-global/npc/bounac_guard.luadata-otservbr-global/npc/dal_the_huntress_day.luadata-otservbr-global/npc/dal_the_huntress_night.luadata-otservbr-global/npc/evrard.luadata-otservbr-global/npc/evrard_the_miller.luadata-otservbr-global/npc/fitzmaurice.luadata-otservbr-global/npc/fral_the_butcher_day.luadata-otservbr-global/npc/fral_the_butcher_night.luadata-otservbr-global/npc/innkeeper_alphonse.luadata-otservbr-global/npc/jehan_the_baker_day.luadata-otservbr-global/npc/jehan_the_baker_night.luadata-otservbr-global/npc/kesar_the_younger_day.luadata-otservbr-global/npc/kesar_the_younger_night.luadata-otservbr-global/npc/kesars_valet_day.luadata-otservbr-global/npc/kesars_valet_night.luadata-otservbr-global/npc/onfroi_day.luadata-otservbr-global/npc/onfroi_night.luadata-otservbr-global/npc/wes_the_blacksmith_day.luadata-otservbr-global/npc/wes_the_blacksmith_night.luadata-otservbr-global/scripts/quests/grave_danger_quest/cobra_bastion/movements_access_storage.luadata-otservbr-global/scripts/quests/the_order_of_lion_quest/action-bounac_entrance.luadata-otservbr-global/scripts/quests/the_order_of_lion_quest/action_drume.luadata-otservbr-global/scripts/quests/the_order_of_lion_quest/action_elevator.luadata-otservbr-global/scripts/quests/the_order_of_lion_quest/action_evrard_quest.luadata-otservbr-global/scripts/quests/the_order_of_lion_quest/creaturescripts_commander_kills.luadata-otservbr-global/scripts/quests/the_order_of_lion_quest/creaturescripts_crypt_warrior_kill.luadata-otservbr-global/scripts/quests/the_order_of_lion_quest/creaturescripts_deer_kill.luadata-otservbr-global/scripts/quests/the_order_of_lion_quest/creaturescripts_drume_kill.luadata-otservbr-global/scripts/quests/the_order_of_lion_quest/creaturescripts_fugue_kill.luadata-otservbr-global/scripts/quests/the_order_of_lion_quest/moveevent_castle.luadata-otservbr-global/scripts/quests/the_order_of_lion_quest/moveevent_drume_entrance.luadata-otservbr-global/scripts/quests/the_order_of_lion_quest/moveevent_fugue.luadata-otservbr-global/scripts/quests/the_secret_library_quest/the_order_of_the_falcon/movements_access_storage.luadata-otservbr-global/scripts/world_changes/spawns_npc_by_time.luadata-otservbr-global/startup/tables/door_quest.luadata-otservbr-global/startup/tables/item.luadata-otservbr-global/world/otservbr-monster.xmldata-otservbr-global/world/otservbr-npc.xmldata/libs/tables/doors.lua
Comment on lines
+63
to
+65
| local function creatureSayCallback(npc, creature, type, message) | ||
| local player = Player(creature) | ||
| local playerId = player:getId() |
There was a problem hiding this comment.
Guard against a nil player before dereferencing.
CALLBACK_MESSAGE_DEFAULT can fire for any creature that speaks near the NPC. If creature is not a player, Player(creature) returns nil and player:getId() on Line 65 raises a runtime error.
🛡️ Proposed guard
local function creatureSayCallback(npc, creature, type, message)
local player = Player(creature)
+ if not player then
+ return false
+ end
local playerId = player:getId()🤖 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/npc/bounac_guard.lua` around lines 63 - 65, In
creatureSayCallback, guard against Player(creature) returning nil before calling
player:getId(); assign local player = Player(creature), then if not player then
return false (or simply exit the callback) to avoid dereferencing a non-player;
update any subsequent logic in creatureSayCallback that uses playerId or player
methods to only run after this nil-check so non-player creatures won't trigger
runtime errors.
Comment on lines
+74
to
+104
| else | ||
| npcHandler:say({ | ||
| "Pass? Not so fast, outsider. ...", | ||
| "The people of Bounac are cautious during these troubled times. Earn our trust and I may let you pass.", | ||
| }, npc, creature) | ||
| end | ||
| return true | ||
| end | ||
|
|
||
| if MsgContains(message, "out") then | ||
| if player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.AccessEastSide) >= 1 then | ||
| player:teleportTo(destinationOut) | ||
| player:getPosition():sendMagicEffect(CONST_ME_TELEPORT) | ||
| npcHandler:say("Safe travels, traveller.", npc, creature) | ||
| else | ||
| npcHandler:say("You are not even supposed to be in here.", npc, creature) | ||
| end | ||
| return true | ||
| end | ||
|
|
||
| if message:lower() == "yes" and npcHandler:getTopic(playerId) == 1 then | ||
| if player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.BounacTrust) >= 5 then | ||
| grantAccess(player, npc, creature) | ||
| else | ||
| npcHandler:say({ | ||
| "I'm afraid not. The citizens haven't vouched for you yet. ...", | ||
| "Earn their trust and come back.", | ||
| }, npc, creature) | ||
| end | ||
| npcHandler:setTopic(playerId, 0) | ||
| return true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for any setTopic(...,1) tied to the Bounac Guard flow
rg -nP -C2 'setTopic\([^,]+,\s*1\)' --type=lua -g '*bounac*'
rg -nP -C2 'Bounac\s*Guard|BounacTrust' --type=luaRepository: opentibiabr/canary
Length of output: 13233
🏁 Script executed:
#!/bin/bash
# Search repo-wide for setting topic=1
rg -nP 'npcHandler\s*:\s*setTopic\(\s*[^,]+,\s*1\s*\)' --type=lua
# Narrow to anything related to Bounac Guard / TheOrderOfTheLion and topic=1
rg -nP 'setTopic\(\s*[^,]+,\s*1\s*\)' --type=lua -g '*.lua' | rg -n 'bounac|Bounac Guard|TheOrderOfTheLion' || true
# Look for getTopic(...)=1 checks in bounac_guard.lua
rg -nP 'getTopic\(\s*playerId\s*\)\s*==\s*1' data-otservbr-global/npc/bounac_guard.lua --type=luaRepository: opentibiabr/canary
Length of output: 34318
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="data-otservbr-global/npc/bounac_guard.lua"
# 1) What topic values does this specific file set?
rg -nP 'npcHandler\s*:\s*setTopic\(\s*playerId\s*,\s*([0-9]+)\s*\)' "$FILE" --type=lua -o
echo "----"
rg -nP 'setTopic\(\s*playerId\s*,\s*' "$FILE" --type=lua
echo "----"
# 2) Confirm the getTopic condition and what sets it (context)
rg -nP -C3 'getTopic\(\s*playerId\s*\)\s*==\s*1' "$FILE" --type=lua
echo "----"
# 3) Inspect the surrounding flow for "pass"/"in"/"yes"
rg -nP -C3 'MsgContains\(\s*message\s*,\s*"out"\s*\)' "$FILE" --type=lua
rg -nP -C3 'MsgContains\(\s*message\s*,\s*"in"\s*\)' "$FILE" --type=lua || true
rg -nP -C3 'MsgContains\(\s*message\s*,\s*"pass"\s*\)' "$FILE" --type=lua || true
rg -nP -C3 'message\s*:\s*lower\(\)\s*==\s*"yes"' "$FILE" --type=lua
echo "----"
# 4) Check if any shared helper calls setTopic(playerId,1) indirectly for this npc script.
# (Repo-wide, but constrained to the Bounac Guard file name references.)
rg -nP 'bounac_guard|Bounac Guard' --type=luaRepository: opentibiabr/canary
Length of output: 1928
🏁 Script executed:
#!/bin/bash
FILE="data-otservbr-global/npc/bounac_guard.lua"
rg -nP 'setTopic\(' "$FILE" --type=luaRepository: opentibiabr/canary
Length of output: 101
🏁 Script executed:
#!/bin/bash
FILE="data-otservbr-global/npc/bounac_guard.lua"
wc -l "$FILE"
sed -n '1,200p' "$FILE" | cat -nRepository: opentibiabr/canary
Length of output: 4398
🏁 Script executed:
#!/bin/bash
# Locate NpcHandler implementation (getTopic/setTopic defaults)
fd -a -i 'npchandler*.lua' . || true
fd -a -i '*npcHandler*.lua' . || true
# Search for getTopic/setTopic in NPC system libs
rg -nP 'function\s+NpcHandler|getTopic\(|setTopic\(' data -S || true
rg -nP 'getTopic\(' data -S --glob='*npc*' || true
rg -nP 'setTopic\(' data -S --glob='*npc*' || trueRepository: opentibiabr/canary
Length of output: 13734
🏁 Script executed:
#!/bin/bash
# Inspect FocusModule for topic side-effects
rg -nP 'class\s+FocusModule|FocusModule' --type=lua data -S
rg -nP 'setTopic\(' --type=lua --glob='*focus*' data -S || true
# Find usage of the deny message text
rg -nF 'Pass? Not so fast, outsider' --type=lua data -SRepository: opentibiabr/canary
Length of output: 2432
🏁 Script executed:
#!/bin/bash
FILE="data/npclib/npc_system/modules.lua"
sed -n '240,430p' "$FILE" | cat -nRepository: opentibiabr/canary
Length of output: 6379
🏁 Script executed:
#!/bin/bash
FILE="data/npclib/npc_system/npc_handler.lua"
# show topic handling + interaction setup
sed -n '150,270p' "$FILE" | cat -n
echo "----"
# show message processing path for onSay/onGreet
rg -nP 'function\s+NpcHandler:onSay|function\s+NpcHandler:greet|function\s+NpcHandler:onGreet|function\s+NpcHandler:checkInteraction' "$FILE" --type=lua
echo "----"
rg -nP 'getTopic\(|setTopic\(|setInteraction\(|resetNpc\(' "$FILE" --type=lua | head -n 50Repository: opentibiabr/canary
Length of output: 5547
🏁 Script executed:
#!/bin/bash
FILE="data/npclib/npc_system/npc_handler.lua"
# Show greet and onSay implementations
sed -n '360,620p' "$FILE" | cat -nRepository: opentibiabr/canary
Length of output: 11260
Fix unreachable "yes" / topic == 1 confirmation in bounac_guard.lua
- The
"yes"branch checksnpcHandler:getTopic(playerId) == 1, butdata-otservbr-global/npc/bounac_guard.luanever setsnpcHandler:setTopic(playerId, 1)(topic is initialized to0on interaction, andFocusModuledoesn’t change it), so the confirmation code can’t be reached.
Likely fix
else
npcHandler:say({
"Pass? Not so fast, outsider. ...",
"The people of Bounac are cautious during these troubled times. Earn our trust and I may let you pass.",
}, npc, creature)
+ npcHandler:setTopic(playerId, 1)
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.
Suggested change
| else | |
| npcHandler:say({ | |
| "Pass? Not so fast, outsider. ...", | |
| "The people of Bounac are cautious during these troubled times. Earn our trust and I may let you pass.", | |
| }, npc, creature) | |
| end | |
| return true | |
| end | |
| if MsgContains(message, "out") then | |
| if player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.AccessEastSide) >= 1 then | |
| player:teleportTo(destinationOut) | |
| player:getPosition():sendMagicEffect(CONST_ME_TELEPORT) | |
| npcHandler:say("Safe travels, traveller.", npc, creature) | |
| else | |
| npcHandler:say("You are not even supposed to be in here.", npc, creature) | |
| end | |
| return true | |
| end | |
| if message:lower() == "yes" and npcHandler:getTopic(playerId) == 1 then | |
| if player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.BounacTrust) >= 5 then | |
| grantAccess(player, npc, creature) | |
| else | |
| npcHandler:say({ | |
| "I'm afraid not. The citizens haven't vouched for you yet. ...", | |
| "Earn their trust and come back.", | |
| }, npc, creature) | |
| end | |
| npcHandler:setTopic(playerId, 0) | |
| return true | |
| else | |
| npcHandler:say({ | |
| "Pass? Not so fast, outsider. ...", | |
| "The people of Bounac are cautious during these troubled times. Earn our trust and I may let you pass.", | |
| }, npc, creature) | |
| npcHandler:setTopic(playerId, 1) | |
| end | |
| return true | |
| end | |
| if MsgContains(message, "out") then | |
| if player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.AccessEastSide) >= 1 then | |
| player:teleportTo(destinationOut) | |
| player:getPosition():sendMagicEffect(CONST_ME_TELEPORT) | |
| npcHandler:say("Safe travels, traveller.", npc, creature) | |
| else | |
| npcHandler:say("You are not even supposed to be in here.", npc, creature) | |
| end | |
| return true | |
| end | |
| if message:lower() == "yes" and npcHandler:getTopic(playerId) == 1 then | |
| if player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.BounacTrust) >= 5 then | |
| grantAccess(player, npc, creature) | |
| else | |
| npcHandler:say({ | |
| "I'm afraid not. The citizens haven't vouched for you yet. ...", | |
| "Earn their trust and come back.", | |
| }, npc, creature) | |
| end | |
| npcHandler:setTopic(playerId, 0) | |
| return true |
🤖 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/npc/bounac_guard.lua` around lines 74 - 104, The
confirmation branch checking message:lower() == "yes" and
npcHandler:getTopic(playerId) == 1 is unreachable because you never set
npcHandler:setTopic(playerId, 1) when prompting the player; update the code path
that says the guard's challenge (the npcHandler:say block that tells the player
to "Earn our trust and I may let you pass.") to call
npcHandler:setTopic(playerId, 1) after speaking (and keep the existing
npcHandler:setTopic(playerId, 0) reset after handling "yes"); this targets the
npcHandler:setTopic / npcHandler:getTopic flow and the grantAccess function so
the yes-confirmation becomes reachable.
Comment on lines
+60
to
+62
| local function creatureSayCallback(npc, creature, type, message) | ||
| local player = Player(creature) | ||
| local playerId = player:getId() |
There was a problem hiding this comment.
Guard against a nil player before player:getId().
As with the Bounac Guard, the default callback may fire for a non-player creature, making Player(creature) nil and crashing on Line 62.
🛡️ Proposed guard
local function creatureSayCallback(npc, creature, type, message)
local player = Player(creature)
+ if not player then
+ return false
+ end
local playerId = player:getId()🤖 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/npc/evrard.lua` around lines 60 - 62, In
creatureSayCallback, guard against Player(creature) being nil before calling
player:getId(); check that player is non-nil (and optionally isPlayer()) and
return/exit the handler early if not, so calls to player:getId() (and any
subsequent player methods) are only executed for valid player objects.
| npcHandler:setTopic(playerId, 2) | ||
| elseif message:lower() == "yes" and player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.Evrard) == 1 then | ||
| if npcHandler:getTopic(playerId) == 2 then | ||
| if player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.Parchment) >= 1 and player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.MapStorage) >= 1 and player:removeItem(635, 1) and player:removeItem(2822, 1) then |
There was a problem hiding this comment.
Item loss: short-circuit removeItem calls can consume one item without advancing.
The condition removes items inside a short-circuited and chain. If the player has item 635 but not 2822, removeItem(635, 1) succeeds and permanently consumes the ledger, then removeItem(2822, 1) returns false, the whole branch fails, and the player is told they "haven't brought both items" — having already lost one. Validate possession first, then remove.
🐛 Proposed fix — check counts before removing
- if player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.Parchment) >= 1 and player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.MapStorage) >= 1 and player:removeItem(635, 1) and player:removeItem(2822, 1) then
+ if player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.Parchment) >= 1 and player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.MapStorage) >= 1 and player:getItemCount(635) >= 1 and player:getItemCount(2822) >= 1 then
+ player:removeItem(635, 1)
+ player:removeItem(2822, 1)
player:setStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.Evrard, 2)Same defect exists in evrard_the_miller.lua Line 84.
🤖 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/npc/evrard.lua` at line 84, The condition currently
calls player:removeItem(635,1) and player:removeItem(2822,1) inside an && chain
which can short-circuit and consume one item if the second removal fails; change
the logic so you first check that the player possesses both items (e.g., with
player:getItemCount or an explicit possession check for item IDs 635 and 2822
and the existing storage checks via player:getStorageValue) and only then call
player:removeItem for each item sequentially; ensure both removals are performed
only after successful possession checks and handle removal failure by aborting
and returning the original failure message.
Comment on lines
+64
to
+80
| if MsgContains(message, "yselda") and player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.AccessSouthernSide) >= 1 and player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.WesTask) < 1 then | ||
| if npcHandler:getTopic(playerId) == 0 then | ||
| npcHandler:say({ | ||
| "Ah yes, right. You came to the right place, ahem. ...", | ||
| "Well, for quite some time we are planning to help our king during this awful siege. As you know the passphrase, one of us already trusts you. ...", | ||
| "Outsiders are not welcome here during the siege. If you want to get into the castle, you will have to earn all our trust. ...", | ||
| "If you can do that, a member of the lion guard who is in contact with us, will let you in. ...", | ||
| "Just ask any of us the passphrase and we will see what we can do for you.", | ||
| }, npc, creature) | ||
| player:setStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.WesTask, 1) | ||
| elseif npcHandler:getTopic(playerId) == 1 then | ||
| npcHandler:say({ | ||
| "Alright then, let's see if you are really trustworthy. I need more material to forge better equipment for our men. ...", | ||
| "Bring me about 20 broken longbows from fallen usurpers and we will see.", | ||
| }, npc, creature) | ||
| npcHandler:setTopic(playerId, 0) | ||
| end |
There was a problem hiding this comment.
Unreachable longbow-instruction branch.
Inside the WesTask < 1 block, the topic == 0 path only sets WesTask = 1 and never sets the topic to 1. Since the topic enters this block at its default 0, the elseif npcHandler:getTopic(playerId) == 1 branch (lines 74-80) carrying the "Bring me about 20 broken longbows" instruction is never reachable. After the intro, the next yselda matches WesTask == 1 (line 81) instead. The collection instruction is therefore only surfaced via the quest log, not by Wes's dialogue. Confirm whether the intro path should advance the topic so the instruction is shown.
🔍 Possible flow fix (verify intended dialogue order)
if npcHandler:getTopic(playerId) == 0 then
npcHandler:say({
"Ah yes, right. You came to the right place, ahem. ...",
"Well, for quite some time we are planning to help our king during this awful siege. As you know the passphrase, one of us already trusts you. ...",
"Outsiders are not welcome here during the siege. If you want to get into the castle, you will have to earn all our trust. ...",
"If you can do that, a member of the lion guard who is in contact with us, will let you in. ...",
"Just ask any of us the passphrase and we will see what we can do for you.",
}, npc, creature)
player:setStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.WesTask, 1)
+ npcHandler:setTopic(playerId, 1)
elseif npcHandler:getTopic(playerId) == 1 then🤖 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/npc/wes_the_blacksmith_day.lua` around lines 64 - 80,
The longbow-instruction branch is unreachable because within the
MsgContains(message, "yselda") and WesTask < 1 block, the topic == 0 path sets
WesTask to 1 without advancing the topic, so topic == 1 never executes. In the
npcHandler:getTopic(playerId) == 0 branch, do not set
Storage.Quest.U12_40.TheOrderOfTheLion.WesTask yet; instead advance dialogue by
calling npcHandler:setTopic(playerId, 1). Then, in the
npcHandler:getTopic(playerId) == 1 branch, present the “broken longbows”
instruction, set Storage.Quest.U12_40.TheOrderOfTheLion.WesTask to 1, and reset
npcHandler:setTopic(playerId, 0).
Comment on lines
+42947
to
42949
| <monster centerx="32439" centery="32470" centerz="6" radius="1"> | ||
| <monster name="Ancient Lion Knight" x="0" y="0" z="7" spawntime="180" /> | ||
| </monster> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check z-coordinate patterns in monster spawn definitions to identify if centerz and inner z mismatches are common or anomalies
# Search for monster spawn blocks and extract centerz and inner z values
rg -A 2 '<monster centerx=' data-otservbr-global/world/otservbr-monster.xml | rg -P '(centerz="\d+"|z="\d+")' | head -50Repository: opentibiabr/canary
Length of output: 3389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="data-otservbr-global/world/otservbr-monster.xml"
echo "=== Context around lines 42940-42960 ==="
nl -ba "$FILE" | sed -n '42935,42960p'
echo
echo "=== Find all <monster centerz> blocks where the first inner <monster ... z=> differs ==="
python3 - <<'PY'
import re, sys
path="data-otservbr-global/world/otservbr-monster.xml"
txt=open(path,'r',encoding='utf-8').read()
# Capture each outer monster opening with centerz
outer_re=re.compile(r'<monster\b([^>]*?)\bcenterz="(-?\d+)"([^>]*?)>', re.M)
# For each outer opening, look at the first inner monster element's z="..."
# This is a heuristic but matches the common two-level structure in this file.
mismatches=[]
pos=0
for mo in outer_re.finditer(txt):
centerz=int(mo.group(2))
start=mo.end()
# Extract up to the end of this outer monster block (naively until the first "</monster>" after start)
end=txt.find("</monster>", start)
if end==-1:
continue
block=txt[start:end]
inner=re.search(r'<monster\b[^>]*?\bz="(-?\d+)"', block)
if not inner:
continue
innerz=int(inner.group(1))
if innerz!=centerz:
mismatches.append((centerz, innerz, mo.start()))
# Print up to 50 mismatches with nearby line numbers
def line_of(idx):
return txt.count("\n",0,idx)+1
print("Total mismatches found (outer centerz != first inner z):", len(mismatches))
for centerz, innerz, idx in mismatches[:50]:
ln=line_of(idx)
print(f"- at line ~{ln}: centerz={centerz}, inner z={innerz}")
PY
echo
echo "=== Specifically search for the Ancient Lion Knight block ==="
rg -n --fixed-string 'Ancient Lion Knight' "$FILE" | headRepository: opentibiabr/canary
Length of output: 164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="data-otservbr-global/world/otservbr-monster.xml"
python3 - <<'PY'
path="data-otservbr-global/world/otservbr-monster.xml"
lines=open(path,'r',encoding='utf-8').read().splitlines()
start=42935
end=42960
for i in range(start, min(end, len(lines)-1)+1):
print(f"{i+1}\t{lines[i]}")
PY
echo
echo "=== Extract the exact 'Ancient Lion Knight' outer <monster> block (centerz + inner z) ==="
python3 - <<'PY'
import re
path="data-otservbr-global/world/otservbr-monster.xml"
txt=open(path,'r',encoding='utf-8').read()
m=re.search(r'(<monster\b[^>]*\bcenterz="(-?\d+)"[^>]*>\\s*\\n\\s*<monster\\b[^>]*\\bname="Ancient Lion Knight"[^>]*>)', txt)
print("outer block match:", bool(m))
if m:
outer_snip=m.group(1)
# find inner z within that snippet
outer_centerz=m.group(2)
innerz=re.search(r'<monster\b[^>]*\bz="(-?\d+)"', outer_snip)
innerz_val=innerz.group(1) if innerz else None
print(f"centerz={outer_centerz}, inner z={innerz_val}")
print("\n--- Outer snippet ---")
print(outer_snip)
PY
echo
echo "=== Scan: outer centerz != first inner monster z (report count + first few line numbers) ==="
python3 - <<'PY'
import re
path="data-otservbr-global/world/otservbr-monster.xml"
txt=open(path,'r',encoding='utf-8').read()
lines=txt.splitlines(True) # keep newlines
def line_of_charpos(pos):
# convert char index to 1-based line number
return txt.count("\n",0,pos)+1
outer_re=re.compile(r'<monster\b([^>]*?)\bcenterz="(-?\d+)"([^>]*?)>', re.M)
mismatches=[]
for mo in outer_re.finditer(txt):
centerz=int(mo.group(2))
# naive: take substring until the next </monster> after this opening
start=mo.end()
end=txt.find("</monster>", start)
if end==-1:
continue
block=txt[start:end]
inner=re.search(r'<monster\b[^>]*?\bz="(-?\d+)"', block)
if not inner:
continue
innerz=int(inner.group(1))
if innerz!=centerz:
mismatches.append((line_of_charpos(mo.start()), centerz, innerz))
print("Total mismatches found:", len(mismatches))
for ln, centerz, innerz in mismatches[:30]:
print(f"- line ~{ln}: centerz={centerz}, inner z={innerz}")
# check whether there is a mismatch near the reported lines
near=[m for m in mismatches if 42940<=m[0]<=42960]
print("\nMismatches near 42940-42960:", near)
PYRepository: opentibiabr/canary
Length of output: 1824
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="data-otservbr-global/world/otservbr-monster.xml"
python3 - <<'PY'
import xml.etree.ElementTree as ET
path="data-otservbr-global/world/otservbr-monster.xml"
try:
tree=ET.parse(path)
except Exception as e:
print("XML parse failed:", e)
raise
root=tree.getroot()
mismatches=[]
# Heuristic: outer monster elements have `@centerz`; inner monster elements usually have `@z`.
# We'll compare all direct descendant <monster> elements inside an outer monster with centerz.
def iter_monsters(elem):
for child in elem.iter():
if child.tag == 'monster':
yield child
for outer in root.iter('monster'):
if 'centerz' not in outer.attrib:
continue
centerz=outer.attrib.get('centerz')
# collect all inner monsters' z under this outer monster
inner_z=set()
for inner in outer.iter('monster'):
if inner is outer:
continue
if 'z' in inner.attrib:
inner_z.add(inner.attrib['z'])
if not inner_z:
continue
if len(inner_z)==1 and next(iter(inner_z))==centerz:
continue
# if there are multiple z values, include them
mismatches.append((centerz, sorted(inner_z)))
print("Total mismatches where outer centerz != inner z set:", len(mismatches))
for i,(centerz, innerz_list) in enumerate(mismatches[:20], start=1):
print(f"{i}. outer centerz={centerz}, inner z values={innerz_list}")
PYRepository: opentibiabr/canary
Length of output: 158
Fix centerz mismatch for “Ancient Lion Knight” spawn
- The outer spawn uses
centerz="6"while the inner monster position usesz="7", and this is the onlycenterz/inner-zmismatch indata-otservbr-global/world/otservbr-monster.xml(likely a typo causing incorrect spawn height).
🔧 Proposed fix
- <monster centerx="32439" centery="32470" centerz="6" radius="1">
+ <monster centerx="32439" centery="32470" centerz="7" radius="1">
<monster name="Ancient Lion Knight" x="0" y="0" z="7" spawntime="180" />
</monster>📝 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.
Suggested change
| <monster centerx="32439" centery="32470" centerz="6" radius="1"> | |
| <monster name="Ancient Lion Knight" x="0" y="0" z="7" spawntime="180" /> | |
| </monster> | |
| <monster centerx="32439" centery="32470" centerz="7" radius="1"> | |
| <monster name="Ancient Lion Knight" x="0" y="0" z="7" spawntime="180" /> | |
| </monster> |
🤖 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/world/otservbr-monster.xml` around lines 42947 - 42949,
The outer monster spawn has a centerz="6" while the inner <monster name="Ancient
Lion Knight" ... z="7"> uses z="7" causing a height mismatch; update the outer
<monster centerx="32439" centery="32470" centerz="6" radius="1"> to use
centerz="7" so the centerz matches the inner monster z and the spawn height is
correct.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/lib/core/storages.lua`:
- Around line 2807-2812: The GraveDanger table currently defines CobraBastion
twice causing the second definition (CobraBastion = { Access = 46927 }) to
overwrite the first and drop Questline; merge these into a single CobraBastion
table containing both keys (Questline = 46891 and Access = 46927) and remove the
duplicate block so Storage.Quest.U12_20.GraveDanger.CobraBastion has both
Questline and Access available for scripts like actions_scarlett.lua and
creaturescripts_scarlett.lua to use.
🪄 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: 9956b08d-e176-4987-9265-9663664a03db
📒 Files selected for processing (1)
data-otservbr-global/lib/core/storages.lua
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
data-otservbr-global/scripts/quests/the_order_of_lion_quest/moveevent_castle.lua (1)
17-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix:
Positionequality is implemented correctly; only UX feedback is missing
data.position == positionwill match as expected:Positionregisters__eqin Lua (PositionFunctions::luaPositionCompare), which returnsPosition::operator==comparingx/y/z(not reference identity).- Add an
elsebranch/feedback when stepping on anenteringtile withoutAccessEasternSide(currently the player gets no effect/message).🤖 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/quests/the_order_of_lion_quest/moveevent_castle.lua` around lines 17 - 22, Keep the Position equality check as-is (data.position == position) since Position defines __eq; instead add user feedback when a matching teleport tile of type "entering" is found but the player lacks the required AccessEasternSide. Locate the loop over setting and the matched telemetry variable teleport, and in the branch that handles data.type == "entering" (or where teleport is used) add an else/denial branch that notifies the player (e.g., a send message or effect) when AccessEasternSide is false so the player receives clear UX feedback instead of nothing.
🧹 Nitpick comments (1)
data-otservbr-global/scripts/quests/the_order_of_lion_quest/moveevent_castle.lua (1)
28-33: ⚡ Quick winConsider adding feedback when access is denied.
When a player steps on an "entering" checkpoint without the required storage (
AccessEasternSide >= 1), nothing happens—no teleport, no message. This could confuse players who don't understand why they can't pass through.📢 Proposed improvement: send feedback message
if teleport.entering then if player:getStorageValue(Storage.Quest.U12_40.TheOrderOfTheLion.AccessEasternSide) >= 1 then player:teleportTo(teleport.newPosition) fromPosition:sendMagicEffect(CONST_ME_TELEPORT) teleport.newPosition:sendMagicEffect(CONST_ME_TELEPORT) + else + player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "You need permission to enter this area.") + fromPosition:sendMagicEffect(CONST_ME_POFF) end else🤖 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/quests/the_order_of_lion_quest/moveevent_castle.lua` around lines 28 - 33, The entering teleport currently checks teleport.entering and Storage.Quest.U12_40.TheOrderOfTheLion.AccessEasternSide via player:getStorageValue and silently does nothing if access < 1; update the branch handling the denied case to send player feedback (e.g., player:sendTextMessage or player:sendCancelMessage) explaining access is denied so players understand why they weren't teleported; keep the existing successful path (player:teleportTo and sendMagicEffect on fromPosition and teleport.newPosition) unchanged and only add the message in the else/denied branch.
🤖 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.
Outside diff comments:
In
`@data-otservbr-global/scripts/quests/the_order_of_lion_quest/moveevent_castle.lua`:
- Around line 17-22: Keep the Position equality check as-is (data.position ==
position) since Position defines __eq; instead add user feedback when a matching
teleport tile of type "entering" is found but the player lacks the required
AccessEasternSide. Locate the loop over setting and the matched telemetry
variable teleport, and in the branch that handles data.type == "entering" (or
where teleport is used) add an else/denial branch that notifies the player
(e.g., a send message or effect) when AccessEasternSide is false so the player
receives clear UX feedback instead of nothing.
---
Nitpick comments:
In
`@data-otservbr-global/scripts/quests/the_order_of_lion_quest/moveevent_castle.lua`:
- Around line 28-33: The entering teleport currently checks teleport.entering
and Storage.Quest.U12_40.TheOrderOfTheLion.AccessEasternSide via
player:getStorageValue and silently does nothing if access < 1; update the
branch handling the denied case to send player feedback (e.g.,
player:sendTextMessage or player:sendCancelMessage) explaining access is denied
so players understand why they weren't teleported; keep the existing successful
path (player:teleportTo and sendMagicEffect on fromPosition and
teleport.newPosition) unchanged and only add the message in the else/denied
branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3770eaea-7940-4a44-a1d9-791e8d633c24
📒 Files selected for processing (6)
data-otservbr-global/lib/core/storages.luadata-otservbr-global/npc/bounac_guard.luadata-otservbr-global/npc/evrard.luadata-otservbr-global/npc/evrard_the_miller.luadata-otservbr-global/scripts/creaturescripts/customs/freequests.luadata-otservbr-global/scripts/quests/the_order_of_lion_quest/moveevent_castle.lua
🚧 Files skipped from review as they are similar to previous changes (4)
- data-otservbr-global/npc/evrard.lua
- data-otservbr-global/npc/evrard_the_miller.lua
- data-otservbr-global/lib/core/storages.lua
- data-otservbr-global/npc/bounac_guard.lua
|
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
1 participant
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.
WARNING: Don't merge this pull request Until Majesty upload the map Update I sent to him or Quest may be broken.
Complete Quest+ Achievements + Questlog + Bosses.
Removed the storage reseter when talking to Kesar with the keywords because I think it's stupid.
Everything has been debuged and tested at https://tibiatales.com