Integrate Free Quests system CPP and refactor quest handling#3732
Integrate Free Quests system CPP and refactor quest handling#3732
Conversation
beats-dh
force-pushed
the
beats-fix-and-freeQuestCPP
branch
from
0a90c20 to
9e02501
Compare
|
|
This change isn't beneficial because it moves a component that was previously bound to the global pack to something loaded across the entire server, even though it's only needed by the global pack. This increases compilation time, distributes unused code across all scenarios, and makes maintenance and review more difficult, without any apparent benefit. |
lgrossi
force-pushed
the
beats-fix-and-freeQuestCPP
branch
3 times, most recently
from
69661ed to
f614712
Compare
Introduces a new Free Quests system with C++ backend integration, replacing the previous Lua-based implementation. Adds game_freequests.cpp/hpp, updates player login logic to apply free quests, and exposes Game.addFreeQuestData to Lua. Refactors quest data loading and validation, improves date/time formatting functions, and updates .gitignore and build configuration. Also fixes time formatting in database backup and tools.cpp to use chrono time points.
lgrossi
force-pushed
the
beats-fix-and-freeQuestCPP
branch
from
f614712 to
032a6db
Compare
|
|
This PR is stale because it has been open 45 days with no activity. |
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughA new Free Quests system is implemented that automatically applies predefined quests and storage data to players during login. The system includes C++ singleton management, Lua configuration integration, and player lifecycle hooks to deliver quests on account creation. Changes
Sequence Diagram(s)sequenceDiagram
participant Player
participant Game
participant GameFreeQuests
participant Config
participant Dispatcher
participant Storage
Player->>Game: onPlacedCreature() (login)
Game->>GameFreeQuests: applyFreeQuestsToPlayer(player)
GameFreeQuests->>Config: isEnabled()
Config-->>GameFreeQuests: true/false
alt Free Quests Enabled
GameFreeQuests->>Player: Notify quest receipt
GameFreeQuests->>Player: Equip outfits
GameFreeQuests->>Dispatcher: Schedule async task
Dispatcher->>GameFreeQuests: applyQuestStoragesAsync(playerId)
GameFreeQuests->>Storage: Apply quest storage values
Storage->>Player: Update quest progress/stage
GameFreeQuests->>Player: Notify quest application complete
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/creatures/players/player.cpp (1)
6040-6052: Add early return after failed login to prevent side-effectful operations on disconnecting players.When
g_creatureEvents().playerLogin()returns false andremovePlayer(true)is called, the function continues executing. This causesapplyFreeQuestsToPlayer()to send text messages and dispatch async operations on a player in the logout process, which is incorrect behavior.Additionally, the free quests are applied after the onLogin event, meaning Lua handlers won't observe this state at login time.
Proposed fix
void Player::onPlacedCreature() { // scripting event - onLogin if (!g_creatureEvents().playerLogin(static_self_cast<Player>())) { removePlayer(true); + return; } // Apply free quests system g_gameFreeQuests().applyFreeQuestsToPlayer(static_self_cast<Player>()); this->onChangeZone(this->getZoneType()); sendUnjustifiedPoints(); }
🤖 Fix all issues with AI agents
In @data-otservbr-global/lib/core/storages.lua:
- Line 115: Storage.FreeQuests is missing causing
player:setStorageValue(player:getStorageValue) calls in
data-canary/scripts/creaturescripts/free_quest.lua to fail; restore the storage
key by adding a definition for Storage.FreeQuests (value 30057 per the reserved
comment) into the storages table in storages.lua so the existing
player:setStorageValue(Storage.FreeQuests, stage) and
player:getStorageValue(Storage.FreeQuests) calls work again.
In @src/database/database.cpp:
- Around line 70-72: The code formats a std::chrono::system_clock::time_point
(now) directly into strings formattedDate and formattedTime which can yield
non-local times depending on fmt version; to fix, convert now to a time_t via
std::chrono::system_clock::to_time_t(now) and pass that through
fmt::localtime(...) when calling fmt::format for both formattedDate and
formattedTime (i.e., replace direct formatting of now with
fmt::format("{:%Y-%m-%d}", fmt::localtime(t)) and fmt::format("{:%H-%M-%S}",
fmt::localtime(t)) where t is the converted time_t).
In @src/game/functions/game_freequests.cpp:
- Around line 88-103: The static init logic in GameFreeQuests::addQuestData has
two issues: replace the unsigned comparison "m_currentStage <= 0" with an
appropriate check for zero (e.g., "m_currentStage == 0") to correctly detect an
invalid stage since m_currentStage is uint32_t, and address the non-thread-safe
static bool systemInitialized by either protecting initialization with a
mutex/lock or documenting that addQuestData is only called from a
single-threaded startup path; update the code around systemInitialized,
m_currentStage and the g_configManager() calls to use the chosen approach and
ensure logging (g_logger()) remains the same.
- Around line 10-16: The file game_freequests.cpp fails to build because
g_game() is used but game.hpp is not included; add an include for
"game/game.hpp" (or the proper header that defines g_game) at the top of
game_freequests.cpp alongside the other includes so the symbol g_game is
declared for use in functions within game_freequests.cpp.
In @src/utils/tools.cpp:
- Around line 476-483: The function formatTimeMs currently uses a broad
catch(...) which should be narrowed: change the catch block to catch const
std::out_of_range& e (to match formatDateTime and other formatters) and, if
available, log or include e.what() in the returned error path; i.e., replace
catch(...) with catch(const std::out_of_range& e) and add a simple log call or
incorporate the exception message into the "<invalid>" handling so the specific
exception is captured.
🧹 Nitpick comments (8)
.gitignore (1)
401-404: Consider moving.traeto its own section or a general IDE section.The
.traepattern (Trae AI IDE configuration) is placed under the# CLIONsection, but it's unrelated to CLion. Consider grouping it separately or renaming the section to something more general like# IDEs / JetBrains.src/utils/tools.cpp (1)
485-493: Consider consistent error return values across formatting functions.The error handling here is good (specific exception, logging). However, note the inconsistency with
formatTimeMswhich returns"<invalid>"on error while this returns an empty string. Consider aligning the behavior across similar functions for predictable caller handling.data/global.lua (1)
121-138: Consider improving readability of the validation condition.The validation on line 122 is quite long and hard to parse at a glance. Consider splitting it into multiple conditions or using an early-continue pattern:
♻️ Suggested refactor for readability
for i, quest in ipairs(questTable) do -- Validate quest structure if type(quest) ~= "table" then logger.warning(string.format("[WARNING] Invalid quest entry at index %d: not a table", i)) invalidQuests = invalidQuests + 1 goto continue end if not quest.storageName or type(quest.storageName) ~= "string" then logger.warning(string.format("[WARNING] Invalid quest entry at index %d: missing or invalid storageName", i)) invalidQuests = invalidQuests + 1 goto continue end if not quest.storage or type(quest.storage) ~= "number" then logger.warning(string.format("[WARNING] Invalid quest entry at index %d: missing or invalid storage", i)) invalidQuests = invalidQuests + 1 goto continue end if not quest.storageValue or type(quest.storageValue) ~= "number" then logger.warning(string.format("[WARNING] Invalid quest entry at index %d: missing or invalid storageValue", i)) invalidQuests = invalidQuests + 1 goto continue end if not Game.addFreeQuestData then logger.error("[ERROR] Game.addFreeQuestData function not available") return false end if Game.addFreeQuestData(quest.storageName, quest.storage, quest.storageValue) then validQuests = validQuests + 1 else logger.warning(string.format("[WARNING] Failed to add quest: %s (storage: %d, value: %d)", quest.storageName, quest.storage, quest.storageValue)) invalidQuests = invalidQuests + 1 end ::continue:: endsrc/game/functions/game_freequests.cpp (2)
22-39: Consider making the hardcoded outfit IDs configurable or constants.Lines 33-34 use hardcoded outfit IDs (251 for male citizen, 252 for female citizen). While the comments explain what they are, using named constants would improve maintainability.
116-127: Duplicate detection uses linear search.The duplicate check iterates through all existing quest data on every insertion. With ~300+ entries, this results in O(n²) complexity during loading. Consider using a
std::unordered_setwith a custom hash for the(storage, storageValue)pair if performance becomes a concern during startup.Note: The SonarCloud warning about using the return value from
emplace_back(line 123) can be safely ignored here since we don't need to access the newly inserted element.data-otservbr-global/scripts/creaturescripts/customs/freequests.lua (1)
249-257: Refactor to use named storage constants for consistency.Lines 249-252 and 257 use hardcoded storage IDs (12450, 12330, 12332, 12333, 100157) while lines 253-256 use named constants from the Storage table. This inconsistent pattern reduces code clarity and maintainability. Consider creating named constants in the Storage table for these legacy IDs—for example,
100157is already defined asJOIN_STORinkilling_in_the_name_of.luaand should be sourced from a shared Storage constant.src/game/functions/game_freequests.hpp (2)
22-31: Missing includes and forward declaration.The header uses standard library types (
std::string,std::vector,std::shared_ptr,uint32_t, etc.) andPlayerwithout the corresponding includes or forward declarations. While these may be provided transitively in the current build, self-contained headers improve maintainability.♻️ Suggested includes
#pragma once +#include <cstdint> +#include <memory> +#include <string> +#include <vector> + +class Player; + struct QuestData {
51-57: Naming inconsistency and undocumented magic number.
FreeQuestsdoesn't follow them_prefix convention used bym_questDataandm_currentStage. Additionally, the value30057appears to be a storage key but lacks documentation explaining its purpose.♻️ Suggested fix
private: std::vector<QuestData> m_questData; uint32_t m_currentStage = 0; - uint16_t FreeQuests = 30057; + uint16_t m_freeQuestsStorageKey = 30057; // Storage key for tracking free quests stage void applyQuestStoragesAsync(uint32_t playerId); bool isValidQuestData(const QuestData &questData) const;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.gitignoredata-otservbr-global/lib/core/storages.luadata-otservbr-global/scripts/creaturescripts/customs/freequests.luadata/global.luasrc/creatures/players/player.cppsrc/database/database.cppsrc/game/CMakeLists.txtsrc/game/functions/game_freequests.cppsrc/game/functions/game_freequests.hppsrc/lua/functions/core/game/game_functions.cppsrc/lua/functions/core/game/game_functions.hppsrc/utils/tools.cppvcpkg.json
🧰 Additional context used
🧬 Code graph analysis (5)
src/database/database.cpp (1)
src/lib/logging/logger.hpp (1)
now(130-142)
src/creatures/players/player.cpp (1)
src/lua/global/shared_object.hpp (3)
static_self_cast(28-30)static_self_cast(28-28)static_self_cast(33-35)
src/game/functions/game_freequests.cpp (3)
src/database/database.cpp (2)
getInstance(23-25)getInstance(23-23)src/game/functions/game_freequests.hpp (4)
player(44-44)playerId(56-56)questData(57-57)storageName(45-45)src/creatures/players/player.cpp (4)
storage(11140-11142)storage(11140-11140)storage(11144-11146)storage(11144-11144)
src/lua/functions/core/game/game_functions.cpp (3)
src/lua/functions/lua_functions_loader.cpp (6)
registerMethod(732-740)registerMethod(732-732)pushBoolean(585-591)pushBoolean(585-585)getString(401-408)getString(401-401)data/libs/functions/functions.lua (1)
isNumber(698-700)src/game/functions/game_freequests.hpp (1)
storageName(45-45)
src/game/functions/game_freequests.hpp (3)
src/creatures/players/player.cpp (4)
storage(11140-11142)storage(11140-11140)storage(11144-11146)storage(11144-11144)src/database/database.cpp (2)
getInstance(23-25)getInstance(23-23)src/game/functions/game_freequests.cpp (2)
getInstance(18-20)getInstance(18-18)
🪛 GitHub Check: macos-macos-debug
src/game/functions/game_freequests.cpp
[failure] 42-42:
use of undeclared identifier 'g_game'
🪛 GitHub Check: SonarCloud Code Analysis
src/game/functions/game_freequests.cpp
[warning] 18-18: Declare this method "[[nodiscard]]" as it does not modify the current object and doesn't have any parameter.
[warning] 61-61: Declare this method "[[nodiscard]]" as it is "const" and doesn't have any parameter.
[warning] 123-123: Use the value returned from "emplace_back".
[warning] 65-65: Declare this method "[[nodiscard]]" as it is "const" and doesn't have any parameter.
src/lua/functions/core/game/game_functions.cpp
[warning] 173-173: Remove the commented out code.
src/utils/tools.cpp
[warning] 480-480: "catch" a specific exception type.
🪛 GitHub Check: ubuntu-22.04-linux-debug
src/game/functions/game_freequests.cpp
[failure] 42-42:
‘g_game’ was not declared in this scope
🪛 GitHub Check: ubuntu-24.04-linux-debug
src/game/functions/game_freequests.cpp
[failure] 42-42:
‘g_game’ was not declared in this scope
🔇 Additional comments (11)
src/game/CMakeLists.txt (1)
3-4: LGTM!The new
game_freequests.cppis correctly added to the build alongside other function files, following the established pattern.vcpkg.json (1)
38-38: The baseline update is intentional and part of this PR. Git history shows it was updated in the same commit that integrates the Free Quests system ("Integrate Free Quests system CPP and refactor quest handling"). The new baseline commit exists and is valid in the vcpkg repository.src/utils/tools.cpp (2)
12-15: LGTM!The new includes are necessary:
configmanager.hppis used bygetMaxValuePerSkill()at line 2113, andposition.hppprovides thePositiontype used in direction-related functions.
446-474: LGTM!Good modernization to use
std::chrono::system_clock::from_time_t()with chrono format specifiers. This is cleaner and avoids potential thread-safety concerns withlocaltime.src/creatures/players/player.cpp (1)
34-37: Include placement looks good (kept out of headers).src/lua/functions/core/game/game_functions.hpp (1)
98-99: LGTM!The new Lua binding declaration follows the established pattern of other static methods in this class.
src/lua/functions/core/game/game_functions.cpp (1)
172-199: LGTM!The implementation follows the established patterns in this file:
- Proper parameter validation with type checks
- Clear error messages via
reportErrorFunc- Consistent return pattern using
pushBooleanNote: The SonarCloud warning about "commented out code" on line 173 is a false positive - this comment documents the Lua API signature, which is the standard convention throughout this file (see lines 123, 226, 254, etc.).
data/global.lua (1)
144-147: LGTM!The conditional loading correctly checks for the existence of
questTable, its type, and the configuration toggle before attempting to load free quests.data-otservbr-global/scripts/creaturescripts/customs/freequests.lua (1)
1-1: LGTM!Making
questTableglobal is required for the integration withdata/global.luawhich callssetTableQuestFree(questTable).src/game/functions/game_freequests.cpp (1)
41-55: Async pattern looks correct but verify player lifetime.The async dispatch correctly captures the player GUID and re-fetches the player in
applyQuestStoragesAsync. This handles the case where the player may have logged out between scheduling and execution.src/game/functions/game_freequests.hpp (1)
60-60: LGTM!The global accessor pattern is consistent with other singletons in the codebase (e.g.,
Database::getInstance).
| -- Reserved in Global.Storage.FamiliarSummonEvent60 = 30055 | ||
| ChayenneKeyTime = 30056, | ||
| FreeQuests = 30057, | ||
| -- Reserved in Global.Storage.FreeQuests = 30057 : Free Quests System cpp |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for any remaining references to Storage.FreeQuests in Lua files
echo "=== Searching for Storage.FreeQuests references ==="
rg -n "Storage\.FreeQuests" --type lua
echo -e "\n=== Searching for numeric storage value 30057 ==="
# Also check for the numeric storage value 30057 being used directly
rg -n "\b30057\b" --type luaRepository: opentibiabr/canary
Length of output: 692
🏁 Script executed:
cat -n data-canary/scripts/creaturescripts/free_quest.lua | head -40Repository: opentibiabr/canary
Length of output: 1453
Fix broken references to Storage.FreeQuests in the free_quest script.
Storage.FreeQuests was removed from the storage table and is no longer defined. However, data-canary/scripts/creaturescripts/free_quest.lua still references it:
- Line 17:
player:setStorageValue(Storage.FreeQuests, stage) - Line 32:
player:getStorageValue(Storage.FreeQuests) == stage
These calls will fail at runtime since Storage.FreeQuests will be nil. Either restore the storage key definition or update the script to use an alternative storage mechanism if the Free Quests system is now fully managed by C++.
🤖 Prompt for AI Agents
In @data-otservbr-global/lib/core/storages.lua at line 115, Storage.FreeQuests
is missing causing player:setStorageValue(player:getStorageValue) calls in
data-canary/scripts/creaturescripts/free_quest.lua to fail; restore the storage
key by adding a definition for Storage.FreeQuests (value 30057 per the reserved
comment) into the storages table in storages.lua so the existing
player:setStorageValue(Storage.FreeQuests, stage) and
player:getStorageValue(Storage.FreeQuests) calls work again.
| auto now = std::chrono::system_clock::now(); | ||
| std::time_t now_c = std::chrono::system_clock::to_time_t(now); | ||
| std::string formattedDate = fmt::format("{:%Y-%m-%d}", fmt::localtime(now_c)); | ||
| std::string formattedTime = fmt::format("{:%H-%M-%S}", fmt::localtime(now_c)); | ||
| std::string formattedDate = fmt::format("{:%Y-%m-%d}", now); | ||
| std::string formattedTime = fmt::format("{:%H-%M-%S}", now); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual code around lines 70-72 in database.cpp
cat -n src/database/database.cpp | sed -n '65,80p'Repository: opentibiabr/canary
Length of output: 836
🏁 Script executed:
# Check for fmt library inclusion and version information
find . -name "*.cpp" -o -name "*.h" -o -name "*.hpp" | xargs grep -l "fmt::" | head -5
rg "fmt/format|fmt::|#include.*fmt" --type cpp --type h -B1 -A1 | head -30Repository: opentibiabr/canary
Length of output: 2528
🏁 Script executed:
# Look for how the formatted time is used (backup directory context)
rg "formattedDate|formattedTime" --type cpp -B3 -A3Repository: opentibiabr/canary
Length of output: 994
🏁 Script executed:
# Check for fmt version specification in build files
find . -name "CMakeLists.txt" -o -name "*.cmake" -o -name "conanfile.txt" -o -name "*.json" | xargs grep -i "fmt" 2>/dev/null | head -20Repository: opentibiabr/canary
Length of output: 113
🌐 Web query:
fmt library chrono system_clock time_point local time UTC formatting behavior
💡 Result:
Short summary
- std::chrono::system_clock::time_point is a point in time (UTC-like); it has no timezone attached. [1][3]
- fmt provides chrono formatting with strftime-like specifiers (e.g. "{:%F %T}") and helper functions fmt::gmtime and fmt::localtime to convert a time_t to a std::tm for UTC or local-time formatting. Use those to force UTC vs local. [1][2]
- In practice, some fmt versions/uses have behaved differently from the Strict C++ wording (people have observed fmt printing local time when they expected UTC). If you need guaranteed behavior, explicitly convert the time_point to time_t and call fmt::gmtime (for UTC) or fmt::localtime (for local) before formatting. [6][4]
Minimal examples
-
Force UTC:
- auto t = std::chrono::system_clock::to_time_t(tp);
- fmt::print("{:%F %T %z}", fmt::gmtime(t)); // UTC
-
Force local time:
- auto t = std::chrono::system_clock::to_time_t(tp);
- fmt::print("{:%F %T %z}", fmt::localtime(t)); // local zone
Recommendations
- If you need full time-zone support (named zones, DS transitions), use C++20 facilities (zoned_time) or Howard Hinnant's date/tz library and then format explicitly. [4]
- For portable, unambiguous output, convert to time_t and call fmt::gmtime/fmt::localtime before formatting; do not rely on implicit defaults. [1][6]
Sources
[1] fmt documentation — chrono formatting and fmt::localtime / fmt::gmtime.
[2] fmt API (latest versions) showing chrono format support.
[3] Howard Hinnant discussion on printing system_clock::time_point (UTC vs local).
[4] Howard Hinnant — chrono I/O / date/tz guidance.
[6] StackOverflow discussion reporting fmt vs iostream differences and recommending explicit gmtime/localtime.
Use fmt::localtime() to guarantee local time formatting for backup directory names.
The direct formatting of system_clock::time_point with fmt::format("{:%Y-%m-%d}", now) does not guarantee local time output—behavior depends on the fmt version and implementation. Since backup directory names clearly require local time (not UTC), use the explicit wrapper:
auto now = std::chrono::system_clock::now();
auto t = std::chrono::system_clock::to_time_t(now);
std::string formattedDate = fmt::format("{:%Y-%m-%d}", fmt::localtime(t));
std::string formattedTime = fmt::format("{:%H-%M-%S}", fmt::localtime(t));This ensures portable, unambiguous local-time-based backup directory organization across different systems and fmt versions.
🤖 Prompt for AI Agents
In @src/database/database.cpp around lines 70 - 72, The code formats a
std::chrono::system_clock::time_point (now) directly into strings formattedDate
and formattedTime which can yield non-local times depending on fmt version; to
fix, convert now to a time_t via std::chrono::system_clock::to_time_t(now) and
pass that through fmt::localtime(...) when calling fmt::format for both
formattedDate and formattedTime (i.e., replace direct formatting of now with
fmt::format("{:%Y-%m-%d}", fmt::localtime(t)) and fmt::format("{:%H-%M-%S}",
fmt::localtime(t)) where t is the converted time_t).
| #include "pch.hpp" | ||
|
|
||
| #include "config/configmanager.hpp" | ||
| #include "creatures/players/player.hpp" | ||
| #include "game/functions/game_freequests.hpp" | ||
| #include "game/scheduling/dispatcher.hpp" | ||
| #include "lib/di/container.hpp" |
There was a problem hiding this comment.
Critical: Missing include causes build failure.
The static analysis indicates g_game is undeclared on line 42. The game.hpp header must be included to use g_game().
🐛 Proposed fix
#include "config/configmanager.hpp"
#include "creatures/players/player.hpp"
#include "game/functions/game_freequests.hpp"
+#include "game/game.hpp"
#include "game/scheduling/dispatcher.hpp"
#include "lib/di/container.hpp"🤖 Prompt for AI Agents
In @src/game/functions/game_freequests.cpp around lines 10 - 16, The file
game_freequests.cpp fails to build because g_game() is used but game.hpp is not
included; add an include for "game/game.hpp" (or the proper header that defines
g_game) at the top of game_freequests.cpp alongside the other includes so the
symbol g_game is declared for use in functions within game_freequests.cpp.
| bool GameFreeQuests::addQuestData(const std::string &storageName, uint32_t storage, int32_t storageValue) { | ||
| static bool systemInitialized = false; | ||
| if (!systemInitialized) { | ||
| if (!g_configManager().getBoolean(TOGGLE_FREE_QUEST)) { | ||
| g_logger().info("[{}] Free quest system is disabled", __FUNCTION__); | ||
| return false; | ||
| } | ||
|
|
||
| m_currentStage = g_configManager().getNumber(FREE_QUEST_STAGE); | ||
| if (m_currentStage <= 0) { | ||
| g_logger().warn("[{}] Invalid free quest stage: {}", __FUNCTION__, m_currentStage); | ||
| return false; | ||
| } | ||
|
|
||
| systemInitialized = true; | ||
| } |
There was a problem hiding this comment.
Static initialization has two concerns.
-
Type mismatch in comparison (line 97):
m_currentStageisuint32_t, so comparing with<= 0is misleading since unsigned values can never be negative. The check effectively only catches== 0. -
Thread safety: The
static bool systemInitializedflag is not thread-safe. IfaddQuestDatais called concurrently from multiple threads, there's a potential data race. Given that this is likely called only during startup from a single thread loading Lua scripts, this may be acceptable, but it's worth documenting.
🔧 Proposed fix for the comparison
m_currentStage = g_configManager().getNumber(FREE_QUEST_STAGE);
- if (m_currentStage <= 0) {
+ if (m_currentStage == 0) {
g_logger().warn("[{}] Invalid free quest stage: {}", __FUNCTION__, m_currentStage);
return false;
}🤖 Prompt for AI Agents
In @src/game/functions/game_freequests.cpp around lines 88 - 103, The static
init logic in GameFreeQuests::addQuestData has two issues: replace the unsigned
comparison "m_currentStage <= 0" with an appropriate check for zero (e.g.,
"m_currentStage == 0") to correctly detect an invalid stage since m_currentStage
is uint32_t, and address the non-thread-safe static bool systemInitialized by
either protecting initialization with a mutex/lock or documenting that
addQuestData is only called from a single-threaded startup path; update the code
around systemInitialized, m_currentStage and the g_configManager() calls to use
the chosen approach and ensure logging (g_logger()) remains the same.
| std::string formatTimeMs(int64_t ms) { | ||
| try { | ||
| auto tp = std::chrono::system_clock::from_time_t(ms / 1000); | ||
| return fmt::format("{:%H:%M:%S}", tp); | ||
| } catch (...) { | ||
| return "<invalid>"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Catch specific exception type instead of catch(...).
Per static analysis, the generic catch(...) is too broad. For consistency with formatDateTime and the other formatting functions in this file, catch std::out_of_range specifically and consider logging the error.
Proposed fix
std::string formatTimeMs(int64_t ms) {
try {
auto tp = std::chrono::system_clock::from_time_t(ms / 1000);
return fmt::format("{:%H:%M:%S}", tp);
- } catch (...) {
+ } catch (const std::out_of_range &exception) {
+ g_logger().error("Failed to format time ms with error code {}", exception.what());
return "<invalid>";
}
}🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 480-480: "catch" a specific exception type.
🤖 Prompt for AI Agents
In @src/utils/tools.cpp around lines 476 - 483, The function formatTimeMs
currently uses a broad catch(...) which should be narrowed: change the catch
block to catch const std::out_of_range& e (to match formatDateTime and other
formatters) and, if available, log or include e.what() in the returned error
path; i.e., replace catch(...) with catch(const std::out_of_range& e) and add a
simple log call or incorporate the exception message into the "<invalid>"
handling so the specific exception is captured.
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @beats-dh. * #3732 (comment) The following files were modified: * `data/global.lua` * `src/creatures/players/player.cpp` * `src/database/database.cpp` * `src/game/functions/game_freequests.cpp` * `src/game/functions/game_freequests.hpp` * `src/lua/functions/core/game/game_functions.cpp` * `src/utils/tools.cpp`
|
This PR is stale because it has been open 45 days with no activity. |
2 participants
Description
Introduces a new Free Quests system with C++ backend integration, replacing the previous Lua-based implementation. Adds game_freequests.cpp/hpp, updates player login logic to apply free quests, and exposes Game.addFreeQuestData to Lua. Refactors quest data loading and validation, improves date/time formatting functions, and updates .gitignore and build configuration. Also fixes time formatting in database backup and tools.cpp to use chrono time points.
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.