Skip to content

update: threatened dreams quest#3996

Open
Paco161315 wants to merge 8 commits into
opentibiabr:mainfrom
Paco161315:update-threatened-dreams-quest
Open

update: threatened dreams quest#3996
Paco161315 wants to merge 8 commits into
opentibiabr:mainfrom
Paco161315:update-threatened-dreams-quest

Conversation

@Paco161315

@Paco161315 Paco161315 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

1.- Storages Updated
2.- Questlog Updated
3.- All NPCs/Items storage references now match the storages.lua
4.- All questlog shortcut references are now called with the entire storage directly
5.- Added Questog entries for Butterly Ring Quest

Note: Delete grass with your map editor on top of the big red mushroom near carlin, pos: 32319, 31701, 7

Everything has been debuged and tested at https://tibiatales.com

Summary by CodeRabbit

  • New Features

    • Major expansion of the "Threatened Dreams" quest: five new missions, refined mission text formatting, new rewards and progression steps.
    • Many new in-world interactables and actions (map assembly, treasure, swan-feather spots, present placement, starlight/sun-catcher flows, whelp/poacher actions).
    • Added nightmare-monster kill tracking; numerous NPC dialogue/voice updates and shop inventory changes.
  • Bug Fixes

    • Corrected and clarified various NPC dialogue and message texts.

1.- Storages Updated
2.- Questlog Updated
3.- All NPCs/Items storage references now match the storages.lua
4.- All questlog shortcut references are now called with the entire storage directly
5.- Added Questog entries for Butterly Ring Quest

Note: Delete grass with your map editor on top of the big red mushroom near carlin, pos: 32319, 31701, 7

Everything has been debuged and tested at https://tibiatales.com
@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR converts Threatened Dreams storages to named keys, expands the quest catalog (adds missions 4–8), adds new action and event handlers, and migrates NPC and quest scripts to use fully-qualified Storage.Quest.U11_40.ThreatenedDreams.* keys while introducing several creatureSayCallback/voice entries.

Changes

Threatened Dreams Quest System Refactor

Layer / File(s) Summary
Storage schema and quest catalog definitions
data-otservbr-global/lib/core/storages.lua, data-otservbr-global/lib/core/quests/catalog/045_threatened_dreams.lua
Storage entries converted from indexed numeric arrays to named fields (TroubledAnimals, NightmareIntruders, AnUnlikelyCouple, etc.). Quest catalog gains missions 4–8 with full metadata and state texts; Nightmare Intruders dialog formatting adjusted.
New quest action scripts
data-otservbr-global/scripts/quests/threatened_dreams_quest/action_map.lua, action_present.lua, action_treasure.lua, action_swan_spots.lua, action_big_fly_agaric.lua
New Action handlers: assemble map parts, place presents on beds, trigger treasure at a coordinate, pluck swan feathers with per-spot cooldowns, and big fly agaric progression.
Updated action scripts and quest handlers
data-otservbr-global/scripts/quests/threatened_dreams_quest/*.lua
Many existing action and quest scripts migrated from the local ThreatenedDreams alias to fully-qualified Storage.Quest.U11_40.ThreatenedDreams.* keys; gating/transform flows now check named storages (e.g., NightmareIntruders) and position-specific storages.
Movement triggers, herbs, and death events
movement_*.lua, event_raven_herb_bush.lua, nightmare_monsters_kills.lua
Movement triggers and raven-herb cooldowns now use named storages; added CreatureEvent to track nightmare monster kills tied to NightmareIntruders state.
NPCs with storage migration and new callbacks
data-otservbr-global/npc/*.lua
Numerous NPC scripts updated to remove the ThreatenedDreams alias and read/write explicit Storage.Quest.U11_40.ThreatenedDreams.* fields without changing dialogue flow. Several NPCs gained npcConfig.voices and creatureSayCallback handlers for quest interactions and topic flows (e.g., grumpy_stone, tired_tree, tooth_fairy, valindara).
Maelyrra: Nightmare Intruders orchestration
data-otservbr-global/npc/maelyrra.lua
Refactored dialogue and "yes" flows to initialize and advance NightmareIntruders across stages, set related counters/access flags, and gate subsequent interactions.
Taegen: Mission03 crafting & shop edits
data-otservbr-global/npc/taegen.lua
Refactored to use Mission03 named keys (AnUnlikelyCouple, DarkSunCatcher), adjusted crafting flows, and simplified shop entries in the edited region.
Tereban functions
data-otservbr-global/npc/tereban_functions.lua
Updated quest parsing to use TroubledAnimals and FeathersCount named storages; minor comment fix.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • dudantas
  • majestyotbr

Poem

🐰 I hopped through storages, names now clear,
new missions planted, voices soft and near.
Maps and feathers, actions tied in code,
NPCs now speak where quest keys rode.
A rabbit cheers — the dreams have been restored.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'update: threatened dreams quest' directly and clearly summarizes the main change: a comprehensive update to the Threatened Dreams quest system across storage definitions, quest catalog, NPCs, and quest action scripts.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements and refactors several missions of the 'Threatened Dreams' quest, including updating quest catalog states, defining named storage keys, implementing dialogue and quest logic for multiple NPCs, and adding action scripts for quest mechanics like combining map parts, placing presents, and gathering swan feathers. The review feedback highlights several critical issues, including a state-resetting bug in Valindara's NPC script, a repeatable reward exploit in the map-combining action, a double-talk bug and an uninitialized storage variable in the Tooth Fairy NPC script, and potential runtime crashes across multiple action scripts due to missing checks for nil toPosition values.

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.

Comment on lines +75 to +85
if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission03.AnUnlikelyCouple) < 4 then
npcHandler:say("We can talk about this later if you help Maelyrra first.", npc, creature)
end
if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission03.AnUnlikelyCouple) == 4 then
npcHandler:say("You did us a great favour, mortal being! Well, as I promised I will craft you a feathery cloak. Bring me one hundred swan feathers and I will make them into a beautiful robe. Do you have enough feathers yet?", npc, creature)
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission05.SwanFeatherCloak, 1)
npcHandler:setTopic(playerId, 1)
else
return false
end
elseif MsgContains(message, "yes") and npcHandler:getTopic(playerId) == 1 then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This logic has a critical state-resetting bug. If the player has already completed the feathery cloak quest (SwanFeatherCloak == 2), mentioning cloak or swan will reset their quest state back to 1 and ask them for feathers again. Additionally, returning false when the message is partially handled can cause unexpected NPC behavior. This suggestion adds proper checks for the quest state and returns true when handling the keywords.

	if MsgContains(message, "cloak") or MsgContains(message, "swan") then
		local unlikelyCouple = player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission03.AnUnlikelyCouple)
		local swanFeatherCloak = player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission05.SwanFeatherCloak)
		if unlikelyCouple < 4 then
			npcHandler:say("We can talk about this later if you help Maelyrra first.", npc, creature)
			return true
		elseif swanFeatherCloak == 2 then
			npcHandler:say("I already crafted a feathery cloak for you.", npc, creature)
			return true
		elseif unlikelyCouple == 4 then
			if swanFeatherCloak < 1 then
				npcHandler:say("You did us a great favour, mortal being! Well, as I promised I will craft you a feathery cloak. Bring me one hundred swan feathers and I will make them into a beautiful robe. Do you have enough feathers yet?", npc, creature)
				player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission05.SwanFeatherCloak, 1)			
				npcHandler:setTopic(playerId, 1)
				return true
			elseif swanFeatherCloak == 1 then
				npcHandler:say("Do you have the one hundred swan feathers for your cloak?", npc, creature)
				npcHandler:setTopic(playerId, 1)
				return true
			end
		else
			return false
		end

Comment on lines +3 to +4
function action.onUse(player, item, fromPosition, target, toPosition, isHotkey)
if player:getItemCount(24943) >= 1 and player:getItemCount(24944) >= 1 and player:getItemCount(24945) >= 1 and player:getItemCount(24946) >= 1 then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

This action allows players to combine the map parts even if they have already completed the quest (GrumpyStone == 5), which resets their quest state back to 4 and allows them to claim the treasure rewards again. Restricting this action to when GrumpyStone is exactly 3 prevents this repeatable reward exploit.

function action.onUse(player, item, fromPosition, target, toPosition, isHotkey)
	if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone) ~= 3 then
		return false
		end
	if player:getItemCount(24943) >= 1 and player:getItemCount(24944) >= 1 and player:getItemCount(24945) >= 1 and player:getItemCount(24946) >= 1 then

Comment on lines +109 to +119
npcHandler:say({
"As I'm the tooth fairy it should not surprise you to hear that I have a small collection. Yes, a tooth collection, of course. But I'm still lacking some special specimens. ...",
"I would give you a little reward if you bring me all of the following: an orc tooth, a shark tooth, a vampire tooth, a behemoth fang, a carrion worm fang or a werewolf fangs.",
}, npc, creature)
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.TeethCollection, 1)
end
if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.TeethCollection) == 1 then
npcHandler:say("Did you brought all the teeth for my collection?", npc, creature)
npcHandler:setTopic(playerId, 2)
end
elseif MsgContains(message, "yes") and npcHandler:getTopic(playerId) == 2 then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This sequential if structure causes a double-talk bug. If TeethCollection is less than 1, the first block executes and sets TeethCollection to 1. The second if block immediately evaluates to true and executes as well, causing the NPC to say both messages in a single turn. Changing the second if to elseif resolves this issue.

		if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.TeethCollection) < 1 then
			npcHandler:say({
				"As I'm the tooth fairy it should not surprise you to hear that I have a small collection. Yes, a tooth collection, of course. But I'm still lacking some special specimens. ...",
				"I would give you a little reward if you bring me all of the following: an orc tooth, a shark tooth, a vampire tooth, a behemoth fang, a carrion worm fang or a werewolf fangs.",
			}, npc, creature)
			player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.TeethCollection, 1)
		elseif player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.TeethCollection) == 1 then
			npcHandler:say("Did you brought all the teeth for my collection?", npc, creature)
			npcHandler:setTopic(playerId, 2)
		end

Comment on lines +98 to +106
npcHandler:say({
"Thank you very much, human being! You have to find three children: Quero's daughter in Thais, Allen's son in Venore and Rowenna's daughter in Carlin. Go to their bedrooms and find their milk teeth. ...",
"Usually they put them on their bed stands. Then you have to put the gifts on their beds. Please take these presents and go to Thais, Carlin and Venore. Come back with the milk teeth.",
}, npc, creature)
player:addItem(25302, 3)
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.ToothFairy, 1)
npcHandler:setTopic(playerId, 0)
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The PresentsPlaced storage is not initialized to 0 when the mission is accepted. Since uninitialized storage values return -1 in OTServBR, the first present placement will increment it to 0 instead of 1, resulting in an incorrect count of placed presents. Initializing PresentsPlaced to 0 here ensures the count is accurate.

	elseif MsgContains(message, "yes") and npcHandler:getTopic(playerId) == 1 then
		npcHandler:say({
			"Thank you very much, human being! You have to find three children: Quero's daughter in Thais, Allen's son in Venore and Rowenna's daughter in Carlin. Go to their bedrooms and find their milk teeth. ...",
			"Usually they put them on their bed stands. Then you have to put the gifts on their beds. Please take these presents and go to Thais, Carlin and Venore. Come back with the milk teeth.",
		}, npc, creature)
		player:addItem(25302, 3)
		player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.ToothFairy, 1)
		player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced, 0)
		npcHandler:setTopic(playerId, 0)
	end

Comment on lines +3 to +4
function action.onUse(player, item, fromPosition, target, toPosition, isHotkey)
if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.ToothFairy) == 1 and player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced) < 3 then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using toPosition without verifying if it is non-nil can cause a runtime error (attempt to index a nil value) if the item is used on oneself or directly from the inventory. Adding a defensive check for toPosition prevents potential crashes.

function action.onUse(player, item, fromPosition, target, toPosition, isHotkey)
	if not toPosition then
		return false
	end
	if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.ToothFairy) == 1 and player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced) < 3 then

Comment on lines +3 to +4
function action.onUse(player, item, fromPosition, target, toPosition, isHotkey)
if toPosition.x == 32363 and toPosition.y == 32416 and toPosition.z == 7 then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using toPosition without verifying if it is non-nil can cause a runtime error (attempt to index a nil value) if the item is used on oneself or directly from the inventory. Adding a defensive check for toPosition prevents potential crashes.

function action.onUse(player, item, fromPosition, target, toPosition, isHotkey)
	if not toPosition then
		return false
	end
	if toPosition.x == 32363 and toPosition.y == 32416 and toPosition.z == 7 then

Comment on lines +24 to +25
function swanfeather.onUse(player, item, fromPosition, target, toPosition, isHotkey)
if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission03.AnUnlikelyCouple) < 4 then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using toPosition without verifying if it is non-nil can cause a runtime error (attempt to index a nil value) if the item is used on oneself or directly from the inventory. Adding a defensive check for toPosition prevents potential crashes.

function swanfeather.onUse(player, item, fromPosition, target, toPosition, isHotkey)
	if not toPosition then
		return false
	end
	if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission03.AnUnlikelyCouple) < 4 then

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

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/lib/core/quests/catalog/045_threatened_dreams.lua (1)

8-8: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Storage contract is broken by positional mission indexing.

Line 8, Line 48, and Line 79 still use MissionXX[1], but these missions were refactored to named fields in data-otservbr-global/lib/core/storages.lua. These expressions now resolve to nil, so mission progress lookup will not bind to valid storage IDs.

Suggested fix
-			storageId = Storage.Quest.U11_40.ThreatenedDreams.Mission01[1],
+			storageId = Storage.Quest.U11_40.ThreatenedDreams.Mission01.TroubledAnimals,
...
-			storageId = Storage.Quest.U11_40.ThreatenedDreams.Mission02[1],
+			storageId = Storage.Quest.U11_40.ThreatenedDreams.Mission02.NightmareIntruders,
...
-			storageId = Storage.Quest.U11_40.ThreatenedDreams.Mission03[1],
+			storageId = Storage.Quest.U11_40.ThreatenedDreams.Mission03.AnUnlikelyCouple,

Also applies to: 48-48, 79-79

🤖 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/lib/core/quests/catalog/045_threatened_dreams.lua` at
line 8, The storage lookups use positional indexing
(Storage.Quest.U11_40.ThreatenedDreams.Mission01[1]) which now return nil after
the storages refactor; replace those bracketed positional accesses with the
refactored named storage fields (e.g., use
Storage.Quest.U11_40.ThreatenedDreams.Mission01 directly or the exact named
property defined in lib/core/storages.lua) for each occurrence (the three
MissionXX references) so storageId binds to the actual storage ID.
🧹 Nitpick comments (1)
data-otservbr-global/scripts/quests/threatened_dreams_quest/nightmare_monsters_kills.lua (1)

16-17: ⚡ Quick win

Remove redundant storage assignment.

Line 17 sets NightmareIntruders to 1 immediately after line 16 confirms it already equals 1. This assignment has no effect and can be removed.

♻️ Proposed fix
 		if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission02.NightmareIntruders) == 1 then
-			player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission02.NightmareIntruders, 1)
 			if creature:getName():lower() == nightmareMonsters[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/scripts/quests/threatened_dreams_quest/nightmare_monsters_kills.lua`
around lines 16 - 17, Remove the redundant assignment: the code checks
player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission02.NightmareIntruders)
== 1 and then immediately calls
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission02.NightmareIntruders,
1); delete the player:setStorageValue(...) line so the check remains but no
pointless reassignment occurs.
🤖 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/045_threatened_dreams.lua`:
- Line 56: The player-facing quest description contains a typo in the string
"You killed %d weakened frazzlemaws and %d efeebled silencers." — locate that
exact string (the quest result/description text in 045_threatened_dreams.lua)
and change "efeebled" to "enfeebled" so the message reads "You killed %d
weakened frazzlemaws and %d enfeebled silencers."

In `@data-otservbr-global/npc/grumpy_stone.lua`:
- Around line 68-95: Fix the two typographical errors in the NPC dialogue
strings by replacing the incorrect substrings "I'm afriad" with "I'm afraid"
inside the multi-line say block beginning with "As much as I dislike admiting
it..." (used with npcHandler:say) and replacing "apropiate" with "appropriate"
in the later response string that starts "No, this won't work. Your hands are
too smooth..."; update those literal text entries so the player-facing messages
read correctly.

In `@data-otservbr-global/npc/tired_tree.lua`:
- Around line 69-70: Fix the typo in the NPC dialogue: in the branch that checks
player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.TiredTree)
== 1, update the npcHandler:say call to replace "drit off" with "drift off" so
the line reads that the story could help "drift off to sleep".

In `@data-otservbr-global/npc/tooth_fairy.lua`:
- Around line 115-116: Fix the grammar in the npc dialogue string used in the
npcHandler:say call: change "Did you brought all the teeth for my collection?"
to "Did you bring all the teeth for my collection?" in the branch that checks
player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.TeethCollection)
== 1 so the displayed text is grammatically correct.
- Around line 119-130: The current branch uses chained player:removeItem calls
(inside the MsgContains "yes" / npcHandler:getTopic == 2 block) which can remove
some items before failing and cause item loss; change the logic to first check
the player has all required tooth item counts (use player:getItemCount(itemId)
or player:hasItem(itemId) for 10196, 22649, 9685, 5893, 10275, 22052), and only
if all checks pass perform the removals with player:removeItem for each id, then
run player:addItem(3026,6), player:setStorageValue(...TeethCollection,2) and
give the achievement via player:hasAchievement / player:addAchievement;
otherwise say the "You don't have all..." message and do not remove anything,
and still call npcHandler:setTopic(playerId, 0).

In `@data-otservbr-global/scripts/quests/threatened_dreams_quest/action_map.lua`:
- Around line 4-12: Before assembling the map, verify the player's current quest
state by reading Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone
(using player:getStorageValue) and only allow removal of items, awarding of
24947, saying the message, and setting the storage to 4 if the storage equals
the expected prior state (e.g., 3); if not, abort/return false (or send an
appropriate message). Locate the block that checks
player:getItemCount(24943..24946) and wrap that logic with the storage check so
the item combination cannot advance the quest out of sequence.

In
`@data-otservbr-global/scripts/quests/threatened_dreams_quest/action_present.lua`:
- Line 21: The increment on
Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced can undercount
when the stored value is -1 (unset); update action_present.lua to first read the
value via
player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced),
normalize it to 0 if it's less than 0, then increment and write back with
player:setStorageValue(...). This ensures PresentsPlaced starts at 0 before the
+1 increment so mission progress counts correctly.

In
`@data-otservbr-global/scripts/quests/threatened_dreams_quest/event_raven_herb_bush.lua`:
- Around line 46-52: The cooldown math for RavenHerbTimer is using milliseconds
but os.time() returns seconds, causing a ~20-day timer; update the set/get logic
in the block that reads
Storage.Quest.U11_40.ThreatenedDreams.Mission03.RavenHerbTimer to use seconds
(e.g., replace the existing os.time() + 60 * 30 * 1000 expression with os.time()
+ 60 * 30) so the check and player:setStorageValue store a 30-minute cooldown
correctly while keeping the existing messages (MESSAGE_EVENT_ADVANCE and the
earlier check) intact.

---

Outside diff comments:
In `@data-otservbr-global/lib/core/quests/catalog/045_threatened_dreams.lua`:
- Line 8: The storage lookups use positional indexing
(Storage.Quest.U11_40.ThreatenedDreams.Mission01[1]) which now return nil after
the storages refactor; replace those bracketed positional accesses with the
refactored named storage fields (e.g., use
Storage.Quest.U11_40.ThreatenedDreams.Mission01 directly or the exact named
property defined in lib/core/storages.lua) for each occurrence (the three
MissionXX references) so storageId binds to the actual storage ID.

---

Nitpick comments:
In
`@data-otservbr-global/scripts/quests/threatened_dreams_quest/nightmare_monsters_kills.lua`:
- Around line 16-17: Remove the redundant assignment: the code checks
player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission02.NightmareIntruders)
== 1 and then immediately calls
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission02.NightmareIntruders,
1); delete the player:setStorageValue(...) line so the check remains but no
pointless reassignment occurs.
🪄 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: e5673b53-2d07-4219-b27f-69d37c49ea27

📥 Commits

Reviewing files that changed from the base of the PR and between 44f1c2f and 5e3fe77.

📒 Files selected for processing (36)
  • data-otservbr-global/lib/core/quests/catalog/045_threatened_dreams.lua
  • data-otservbr-global/lib/core/storages.lua
  • data-otservbr-global/npc/a_swan.lua
  • data-otservbr-global/npc/ahmet.lua
  • data-otservbr-global/npc/alkestios.lua
  • data-otservbr-global/npc/aurita.lua
  • data-otservbr-global/npc/chief_grarkharok.lua
  • data-otservbr-global/npc/dancing_fairy.lua
  • data-otservbr-global/npc/ghostly_wolf.lua
  • data-otservbr-global/npc/grumpy_stone.lua
  • data-otservbr-global/npc/ikassis.lua
  • data-otservbr-global/npc/irmana.lua
  • data-otservbr-global/npc/maelyrra.lua
  • data-otservbr-global/npc/taegen.lua
  • data-otservbr-global/npc/tereban_functions.lua
  • data-otservbr-global/npc/tired_tree.lua
  • data-otservbr-global/npc/tooth_fairy.lua
  • data-otservbr-global/npc/valindara.lua
  • data-otservbr-global/scripts/quests/threatened_dreams/nightmare_monsters_kills.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/action_big_fly_agaric.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/action_fairies_release.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/action_map.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/action_moon_mirror.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/action_poacher_book.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/action_poacher_notes.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/action_present.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/action_starlight_vial.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/action_sun_catcher.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/action_swan_spots.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/action_treasure.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/action_whelp_fur.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/event_raven_herb_bush.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/movement_kroazur_room.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/movement_poacher_notes.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/movement_swan_feathers.lua
  • data-otservbr-global/scripts/quests/threatened_dreams_quest/nightmare_monsters_kills.lua
💤 Files with no reviewable changes (1)
  • data-otservbr-global/scripts/quests/threatened_dreams/nightmare_monsters_kills.lua

Comment thread data-otservbr-global/lib/core/quests/catalog/045_threatened_dreams.lua Outdated
Comment on lines +68 to +95
"As much as I dislike admiting it. I need help. WE need help. See, when we arrived in this part of the world, we quickly realised that we can't linger here while keeping our true forms. We need to take over animals, plants or stones. ...",
"And we had to hurry. Some of my siblings were luckier than us and had the chance to take over animals like deer, birds or squirrels. We had to choose stones and now we are stuck here. ...",
"I don't know for how long and with this issue you can't help us. I'm afriad. But there is something else, a trifle compared to our main problem. Nonetheless, it is annoying. ...",
"As for myself I'm fine but my sisters and brothers surrounding us are not. Their backs are itching terribly and we don't have arms and hands to scratch ourselves. Would you help us and scratch my siblings' backs to bring them some relief?",
}, npc, creature)
npcHandler:setTopic(playerId, 1)
end
elseif player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone) == 1 then
if
player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone1) == 1
and player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone2) == 1
and player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone3) == 1
and player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone4) == 1
and player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone5) == 1
then
npcHandler:say({
"Thank You! Their lamentation lapsed into silence. Thus, I assume you brought them some relief. Here, take this map part in return. I'm not interested in this treasure anymore. I just want to return to our hidden realm. ...",
"Search for the last part somewhere in the Fields of Glory. It's hidden in a big fly agaric.",
}, npc, creature)
player:addItem(24945, 1)
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone, 2)
npcHandler:setTopic(playerId, 0)
else
npcHandler:say("You haven't tended to all the suffering stones yet.", npc, creature)
end
end
elseif MsgContains(message, "yes") and npcHandler:getTopic(playerId) == 1 then
npcHandler:say("No, this won't work. Your hands are too smooth. I guess you have to search for an apropiate tool. But no metal please! The sound of metal on stone is gruesome!", npc, creature)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Typographical errors in dialogue strings.

Two spelling mistakes in player-facing text:

  • Line 70: "I'm afriad" → "I'm afraid"
  • Line 95: "apropiate" → "appropriate"
✏️ Proposed fix
-					"I don't know for how long and with this issue you can't help us. I'm afriad. But there is something else, a trifle compared to our main problem. Nonetheless, it is annoying. ...",
+					"I don't know for how long and with this issue you can't help us. I'm afraid. But there is something else, a trifle compared to our main problem. Nonetheless, it is annoying. ...",
-		npcHandler:say("No, this won't work. Your hands are too smooth. I guess you have to search for an apropiate tool. But no metal please! The sound of metal on stone is gruesome!", npc, creature)
+		npcHandler:say("No, this won't work. Your hands are too smooth. I guess you have to search for an appropriate tool. But no metal please! The sound of metal on stone is gruesome!", npc, creature)
🤖 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/grumpy_stone.lua` around lines 68 - 95, Fix the two
typographical errors in the NPC dialogue strings by replacing the incorrect
substrings "I'm afriad" with "I'm afraid" inside the multi-line say block
beginning with "As much as I dislike admiting it..." (used with npcHandler:say)
and replacing "apropiate" with "appropriate" in the later response string that
starts "No, this won't work. Your hands are too smooth..."; update those literal
text entries so the player-facing messages read correctly.

Comment on lines +69 to +70
if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.TiredTree) == 1 then
npcHandler:say("Have you found a story that could help us to drit off to sleep?", npc, creature)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Typographical error in dialogue string.

Line 70: "drit off" → "drift off"

✏️ Proposed fix
-			npcHandler:say("Have you found a story that could help us to drit off to sleep?", npc, creature)
+			npcHandler:say("Have you found a story that could help us to drift off to sleep?", npc, creature)
🤖 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/tired_tree.lua` around lines 69 - 70, Fix the typo
in the NPC dialogue: in the branch that checks
player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.TiredTree)
== 1, update the npcHandler:say call to replace "drit off" with "drift off" so
the line reads that the story could help "drift off to sleep".

Comment on lines +115 to +116
if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.TeethCollection) == 1 then
npcHandler:say("Did you brought all the teeth for my collection?", npc, creature)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Grammar error in dialogue string.

Line 116: "Did you brought" → "Did you bring"

✏️ Proposed fix
-			npcHandler:say("Did you brought all the teeth for my collection?", npc, creature)
+			npcHandler:say("Did you bring all the teeth for my collection?", npc, creature)
🤖 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/tooth_fairy.lua` around lines 115 - 116, Fix the
grammar in the npc dialogue string used in the npcHandler:say call: change "Did
you brought all the teeth for my collection?" to "Did you bring all the teeth
for my collection?" in the branch that checks
player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.TeethCollection)
== 1 so the displayed text is grammatically correct.

Comment on lines +119 to +130
elseif MsgContains(message, "yes") and npcHandler:getTopic(playerId) == 2 then
if player:removeItem(10196, 1) and player:removeItem(22649, 1) and player:removeItem(9685, 1) and player:removeItem(5893, 1) and player:removeItem(10275, 1) and player:removeItem(22052, 1) then
npcHandler:say("Oh, I see! You really found all the teeth for me! Thank you, human being! Please take this in return.", npc, creature)
player:addItem(3026, 6)
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.TeethCollection, 2)
if not player:hasAchievement("Toothfairy Assistant") then
player:addAchievement("Toothfairy Assistant")
end
else
npcHandler:say("You don't have all of the teeth I need for my collection.", npc, creature)
end
npcHandler:setTopic(playerId, 0)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Chained removeItem calls can cause item loss on partial failure.

The and-chained removeItem calls have side effects before short-circuit evaluation stops. If a player has some but not all required teeth, the ones that exist get removed but the quest doesn't complete, causing permanent item loss.

Check all items exist first, then remove them:

🐛 Proposed fix
 	elseif MsgContains(message, "yes") and npcHandler:getTopic(playerId) == 2 then
-		if player:removeItem(10196, 1) and player:removeItem(22649, 1) and player:removeItem(9685, 1) and player:removeItem(5893, 1) and player:removeItem(10275, 1) and player:removeItem(22052, 1) then
+		if
+			player:getItemCount(10196) >= 1
+			and player:getItemCount(22649) >= 1
+			and player:getItemCount(9685) >= 1
+			and player:getItemCount(5893) >= 1
+			and player:getItemCount(10275) >= 1
+			and player:getItemCount(22052) >= 1
+		then
+			player:removeItem(10196, 1)
+			player:removeItem(22649, 1)
+			player:removeItem(9685, 1)
+			player:removeItem(5893, 1)
+			player:removeItem(10275, 1)
+			player:removeItem(22052, 1)
 			npcHandler:say("Oh, I see! You really found all the teeth for me! Thank you, human being! Please take this in return.", npc, creature)
🤖 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/tooth_fairy.lua` around lines 119 - 130, The current
branch uses chained player:removeItem calls (inside the MsgContains "yes" /
npcHandler:getTopic == 2 block) which can remove some items before failing and
cause item loss; change the logic to first check the player has all required
tooth item counts (use player:getItemCount(itemId) or player:hasItem(itemId) for
10196, 22649, 9685, 5893, 10275, 22052), and only if all checks pass perform the
removals with player:removeItem for each id, then run player:addItem(3026,6),
player:setStorageValue(...TeethCollection,2) and give the achievement via
player:hasAchievement / player:addAchievement; otherwise say the "You don't have
all..." message and do not remove anything, and still call
npcHandler:setTopic(playerId, 0).

Comment on lines +4 to +12
if player:getItemCount(24943) >= 1 and player:getItemCount(24944) >= 1 and player:getItemCount(24945) >= 1 and player:getItemCount(24946) >= 1 then
player:removeItem(24943, 1)
player:removeItem(24944, 1)
player:removeItem(24945, 1)
player:removeItem(24946, 1)
player:addItem(24947, 1)
player:say("The old map is artfully drawn. It tells you to search for a stone sun mosaic in the very south of Thais.", TALKTYPE_MONSTER_SAY)
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone, 4)
return true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add quest-state gate before assembling the map.

The action upgrades Mission04.GrumpyStone to 4 based only on item possession. Without checking current mission state, players can bypass intended progression if map parts are obtained outside the normal flow.

Suggested fix
 function action.onUse(player, item, fromPosition, target, toPosition, isHotkey)
-	if player:getItemCount(24943) >= 1 and player:getItemCount(24944) >= 1 and player:getItemCount(24945) >= 1 and player:getItemCount(24946) >= 1 then
+	if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone) ~= 3 then
+		return false
+	end
+	if player:getItemCount(24943) >= 1 and player:getItemCount(24944) >= 1 and player:getItemCount(24945) >= 1 and player:getItemCount(24946) >= 1 then
📝 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
if player:getItemCount(24943) >= 1 and player:getItemCount(24944) >= 1 and player:getItemCount(24945) >= 1 and player:getItemCount(24946) >= 1 then
player:removeItem(24943, 1)
player:removeItem(24944, 1)
player:removeItem(24945, 1)
player:removeItem(24946, 1)
player:addItem(24947, 1)
player:say("The old map is artfully drawn. It tells you to search for a stone sun mosaic in the very south of Thais.", TALKTYPE_MONSTER_SAY)
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone, 4)
return true
if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone) ~= 3 then
return false
end
if player:getItemCount(24943) >= 1 and player:getItemCount(24944) >= 1 and player:getItemCount(24945) >= 1 and player:getItemCount(24946) >= 1 then
player:removeItem(24943, 1)
player:removeItem(24944, 1)
player:removeItem(24945, 1)
player:removeItem(24946, 1)
player:addItem(24947, 1)
player:say("The old map is artfully drawn. It tells you to search for a stone sun mosaic in the very south of Thais.", TALKTYPE_MONSTER_SAY)
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone, 4)
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/scripts/quests/threatened_dreams_quest/action_map.lua`
around lines 4 - 12, Before assembling the map, verify the player's current
quest state by reading
Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone (using
player:getStorageValue) and only allow removal of items, awarding of 24947,
saying the message, and setting the storage to 4 if the storage equals the
expected prior state (e.g., 3); if not, abort/return false (or send an
appropriate message). Locate the block that checks
player:getItemCount(24943..24946) and wrap that logic with the storage check so
the item combination cannot advance the quest out of sequence.

return false
end
player:setStorageValue(bedStorage, 1)
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced, player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced) + 1)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Normalize PresentsPlaced before incrementing.

Line 21 increments directly from storage value; if it is -1 (unset), progress undercounts and can stall mission completion after all three beds are done.

Suggested fix
-			player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced, player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced) + 1)
+			local currentPlaced = math.max(player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced), 0)
+			player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced, currentPlaced + 1)
📝 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
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced, player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced) + 1)
local currentPlaced = math.max(player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced), 0)
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced, currentPlaced + 1)
🤖 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/threatened_dreams_quest/action_present.lua`
at line 21, The increment on
Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced can undercount
when the stored value is -1 (unset); update action_present.lua to first read the
value via
player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.PresentsPlaced),
normalize it to 0 if it's less than 0, then increment and write back with
player:setStorageValue(...). This ensures PresentsPlaced starts at 0 before the
+1 increment so mission progress counts correctly.

Comment on lines +46 to +52
if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission03.RavenHerbTimer) > os.time() then
player:sendTextMessage(MESSAGE_EVENT_ADVANCE, "The raven herb cannot be collected right now.")
return true
end

player:sendTextMessage(MESSAGE_EVENT_ADVANCE, message)
player:setStorageValue(config.storage, os.time() + 60 * 30 * 1000)
player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission03.RavenHerbTimer, os.time() + 60 * 30 * 1000)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Critical: Incorrect cooldown calculation inflates timer by 1000×.

Line 52 calculates the cooldown as os.time() + 60 * 30 * 1000, which equals os.time() + 1,800,000 seconds (approximately 20.8 days). Since os.time() returns seconds (not milliseconds), multiplying by 1000 creates an extremely long cooldown instead of the intended 30 minutes.

🐛 Proposed fix
-	player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission03.RavenHerbTimer, os.time() + 60 * 30 * 1000)
+	player:setStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission03.RavenHerbTimer, os.time() + 30 * 60)
🤖 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/threatened_dreams_quest/event_raven_herb_bush.lua`
around lines 46 - 52, The cooldown math for RavenHerbTimer is using milliseconds
but os.time() returns seconds, causing a ~20-day timer; update the set/get logic
in the block that reads
Storage.Quest.U11_40.ThreatenedDreams.Mission03.RavenHerbTimer to use seconds
(e.g., replace the existing os.time() + 60 * 30 * 1000 expression with os.time()
+ 60 * 30) so the check and player:setStorageValue store a 30-minute cooldown
correctly while keeping the existing messages (MESSAGE_EVENT_ADVANCE and the
earlier check) intact.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/npc/rowenna.lua (1)

59-62: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Remove duplicate message definitions.

The NPC message strings are defined twice: first at lines 59–62, then again at lines 138–141. The second set completely overrides the first, making the initial definitions dead code. Remove the first set (lines 59–62) to eliminate this duplication.

♻️ Proposed refactor to remove duplication

Remove lines 59–62:

-npcHandler:setMessage(MESSAGE_GREET, "Welcome to the finest weaponshop in the land, |PLAYERNAME|! Tell me if you're looking for a good {trade}.")
-npcHandler:setMessage(MESSAGE_FAREWELL, "Good bye. Come back soon.")
-npcHandler:setMessage(MESSAGE_WALKAWAY, "Good bye. Come back soon.")
-npcHandler:setMessage(MESSAGE_SENDTRADE, "Of course, just browse through my wares.")
 npcHandler:addModule(FocusModule:new(), npcConfig.name, true, true, true)

Keep only the definitions at lines 138–141.

Also applies to: 138-141

🤖 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/rowenna.lua` around lines 59 - 62, Duplicate NPC
message definitions exist for npcHandler:setMessage with MESSAGE_GREET,
MESSAGE_FAREWELL, MESSAGE_WALKAWAY, and MESSAGE_SENDTRADE; remove the earlier
set so only the later definitions remain. Locate the first block that calls
npcHandler:setMessage for MESSAGE_GREET, MESSAGE_FAREWELL, MESSAGE_WALKAWAY, and
MESSAGE_SENDTRADE (the ones around the top of the file) and delete that entire
block, keeping the later block of npcHandler:setMessage calls intact to avoid
dead/overridden definitions.
🧹 Nitpick comments (1)
data-otservbr-global/npc/rowenna.lua (1)

81-81: 💤 Low value

Consider using count instead of subType for consistency.

The exercise wraps items use subType while all other exercise items in the shop use count. Both work correctly, but using count consistently throughout the shop list would improve readability.

♻️ Proposed refactor for consistency
-	{ itemName = "durable exercise wraps", clientId = 50294, buy = 1250000, subType = 1800 },
+	{ itemName = "durable exercise wraps", clientId = 50294, buy = 1250000, count = 1800 },
-	{ itemName = "exercise wraps", clientId = 50293, buy = 347222, subType = 500 },
+	{ itemName = "exercise wraps", clientId = 50293, buy = 347222, count = 500 },
-	{ itemName = "lasting exercise wraps", clientId = 50295, buy = 10000000, subType = 14400 },
+	{ itemName = "lasting exercise wraps", clientId = 50295, buy = 10000000, count = 14400 },

Also applies to: 87-87, 98-98

🤖 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/rowenna.lua` at line 81, Replace the inconsistent
use of the key subType with count for the exercise wrap entries: locate the NPC
shop item entries containing itemName "durable exercise wraps" (clientId 50294,
buy = 1250000, subType = 1800) and the other two entries mentioned, and change
the field name from subType to count while keeping the numeric value unchanged
so the table uses count consistently like the other exercise items.
🤖 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/npc/rowenna.lua`:
- Around line 59-62: Duplicate NPC message definitions exist for
npcHandler:setMessage with MESSAGE_GREET, MESSAGE_FAREWELL, MESSAGE_WALKAWAY,
and MESSAGE_SENDTRADE; remove the earlier set so only the later definitions
remain. Locate the first block that calls npcHandler:setMessage for
MESSAGE_GREET, MESSAGE_FAREWELL, MESSAGE_WALKAWAY, and MESSAGE_SENDTRADE (the
ones around the top of the file) and delete that entire block, keeping the later
block of npcHandler:setMessage calls intact to avoid dead/overridden
definitions.

---

Nitpick comments:
In `@data-otservbr-global/npc/rowenna.lua`:
- Line 81: Replace the inconsistent use of the key subType with count for the
exercise wrap entries: locate the NPC shop item entries containing itemName
"durable exercise wraps" (clientId 50294, buy = 1250000, subType = 1800) and the
other two entries mentioned, and change the field name from subType to count
while keeping the numeric value unchanged so the table uses count consistently
like the other exercise items.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 82f78bc3-b05b-4cde-8e75-f3bb5ca9cd83

📥 Commits

Reviewing files that changed from the base of the PR and between 5e3fe77 and bbab0dd.

📒 Files selected for processing (3)
  • data-otservbr-global/npc/allen.lua
  • data-otservbr-global/npc/quero.lua
  • data-otservbr-global/npc/rowenna.lua

Sorry, I forgot about this one!
@sonarqubecloud

sonarqubecloud Bot commented Jun 9, 2026

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/scripts/actions/tools/rake.lua`:
- Around line 16-41: The Mission04 branch is incorrectly an outer-level elseif
so it runs only when target.itemid != 6094; move the Mission04 logic inside the
existing target.itemid == 6094 branch (where the Shattered Isles check runs) by
nesting the positions/positionStorages loop and its checks under that block (use
the same target.itemid check instead of the top-level elseif), ensuring you
reference the same symbols: target.itemid == 6094,
player:getStorageValue(...ThreatenedDreams.Mission04.GrumpyStone), positions,
positionStorages, and the Storage.Quest.U11_40.ThreatenedDreams.Mission04.StoneN
storages so the rake-on-stone behavior only executes when the rake is used on
itemid 6094.
🪄 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: 77cc22d8-1f16-4ff7-a079-da16b4d5c036

📥 Commits

Reviewing files that changed from the base of the PR and between c3b71b5 and 8e4266c.

📒 Files selected for processing (1)
  • data-otservbr-global/scripts/actions/tools/rake.lua

Comment on lines +16 to +41
-- Threatened Dreams Mission04
elseif player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone) == 1 then
local positions = {
{ x = 32617, y = 31863, z = 7 },
{ x = 32616, y = 31864, z = 7 },
{ x = 32615, y = 31866, z = 7 },
{ x = 32620, y = 31864, z = 7 },
{ x = 32619, y = 31865, z = 7 },
}
local positionStorages = {
[1] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone1,
[2] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone2,
[3] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone3,
[4] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone4,
[5] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone5,
}
for i, pos in ipairs(positions) do
if toPosition.x == pos.x and toPosition.y == pos.y and toPosition.z == pos.z then
if player:getStorageValue(positionStorages[i]) == 1 then
return true
end
player:setStorageValue(positionStorages[i], 1)
player:say("*scratch* *scratch*", TALKTYPE_MONSTER_SAY)
return true
end
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Critical control-flow bug: Mission04 logic will never execute on the correct item.

Line 17 uses elseif, creating a branch mutually exclusive with the target.itemid == 6094 check at line 9. This means:

  • When the player uses the rake on itemid 6094 (the stones), the code enters the line 9 block, executes the Shattered Isles check, then exits—never reaching line 17.
  • When the rake is used on any OTHER itemid, line 17's condition is evaluated, but the Mission04 code runs without verifying the target is actually a stone.

Based on context snippets, the quest requires using the rake on stones (itemid 6094). The Mission04 block must be nested inside the target.itemid == 6094 branch, not as a separate top-level elseif.

🐛 Proposed fix: nest Mission04 inside the itemid 6094 block
 	elseif target.itemid == 6094 then
 		if player:getStorageValue(Storage.Quest.U7_8.TheShatteredIsles.TheGovernorDaughter) == 1 then
 			toPosition:sendMagicEffect(CONST_ME_POFF)
 			Game.createItem(6093, 1, Position(32422, 32770, 1))
 			player:say("You have found a ring.", TALKTYPE_MONSTER_SAY)
 			player:setStorageValue(Storage.Quest.U7_8.TheShatteredIsles.TheGovernorDaughter, 2)
 		end
 		-- Threatened Dreams Mission04
-	elseif player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone) == 1 then
+		if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone) == 1 then
-		local positions = {
+			local positions = {
-			{ x = 32617, y = 31863, z = 7 },
+				{ x = 32617, y = 31863, z = 7 },
-			{ x = 32616, y = 31864, z = 7 },
+				{ x = 32616, y = 31864, z = 7 },
-			{ x = 32615, y = 31866, z = 7 },
+				{ x = 32615, y = 31866, z = 7 },
-			{ x = 32620, y = 31864, z = 7 },
+				{ x = 32620, y = 31864, z = 7 },
-			{ x = 32619, y = 31865, z = 7 },
+				{ x = 32619, y = 31865, z = 7 },
-		}
+			}
-		local positionStorages = {
+			local positionStorages = {
-			[1] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone1,
+				[1] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone1,
-			[2] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone2,
+				[2] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone2,
-			[3] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone3,
+				[3] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone3,
-			[4] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone4,
+				[4] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone4,
-			[5] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone5,
+				[5] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone5,
-		}
+			}
-		for i, pos in ipairs(positions) do
+			for i, pos in ipairs(positions) do
-			if toPosition.x == pos.x and toPosition.y == pos.y and toPosition.z == pos.z then
+				if toPosition.x == pos.x and toPosition.y == pos.y and toPosition.z == pos.z then
-				if player:getStorageValue(positionStorages[i]) == 1 then
+					if player:getStorageValue(positionStorages[i]) == 1 then
-					return true
+						return true
-				end
+					end
-				player:setStorageValue(positionStorages[i], 1)
+					player:setStorageValue(positionStorages[i], 1)
-				player:say("*scratch* *scratch*", TALKTYPE_MONSTER_SAY)
+					player:say("*scratch* *scratch*", TALKTYPE_MONSTER_SAY)
-				return true
+					return true
-			end
+				end
-		end
+			end
+		end
 	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
-- Threatened Dreams Mission04
elseif player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone) == 1 then
local positions = {
{ x = 32617, y = 31863, z = 7 },
{ x = 32616, y = 31864, z = 7 },
{ x = 32615, y = 31866, z = 7 },
{ x = 32620, y = 31864, z = 7 },
{ x = 32619, y = 31865, z = 7 },
}
local positionStorages = {
[1] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone1,
[2] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone2,
[3] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone3,
[4] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone4,
[5] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone5,
}
for i, pos in ipairs(positions) do
if toPosition.x == pos.x and toPosition.y == pos.y and toPosition.z == pos.z then
if player:getStorageValue(positionStorages[i]) == 1 then
return true
end
player:setStorageValue(positionStorages[i], 1)
player:say("*scratch* *scratch*", TALKTYPE_MONSTER_SAY)
return true
end
end
-- Threatened Dreams Mission04
if player:getStorageValue(Storage.Quest.U11_40.ThreatenedDreams.Mission04.GrumpyStone) == 1 then
local positions = {
{ x = 32617, y = 31863, z = 7 },
{ x = 32616, y = 31864, z = 7 },
{ x = 32615, y = 31866, z = 7 },
{ x = 32620, y = 31864, z = 7 },
{ x = 32619, y = 31865, z = 7 },
}
local positionStorages = {
[1] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone1,
[2] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone2,
[3] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone3,
[4] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone4,
[5] = Storage.Quest.U11_40.ThreatenedDreams.Mission04.Stone5,
}
for i, pos in ipairs(positions) do
if toPosition.x == pos.x and toPosition.y == pos.y and toPosition.z == pos.z then
if player:getStorageValue(positionStorages[i]) == 1 then
return true
end
player:setStorageValue(positionStorages[i], 1)
player:say("*scratch* *scratch*", TALKTYPE_MONSTER_SAY)
return true
end
end
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/tools/rake.lua` around lines 16 - 41,
The Mission04 branch is incorrectly an outer-level elseif so it runs only when
target.itemid != 6094; move the Mission04 logic inside the existing
target.itemid == 6094 branch (where the Shattered Isles check runs) by nesting
the positions/positionStorages loop and its checks under that block (use the
same target.itemid check instead of the top-level elseif), ensuring you
reference the same symbols: target.itemid == 6094,
player:getStorageValue(...ThreatenedDreams.Mission04.GrumpyStone), positions,
positionStorages, and the Storage.Quest.U11_40.ThreatenedDreams.Mission04.StoneN
storages so the rake-on-stone behavior only executes when the rake is used on
itemid 6094.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant