feat(schedule-event): replace GlobalEvents with ScheduleEvent system#122
feat(schedule-event): replace GlobalEvents with ScheduleEvent system#122ramon-bernardo wants to merge 16 commits intodevfrom
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRemoves the C++ GlobalEvent subsystem and its Lua/C++ bindings; adds a Lua-native ScheduleEvent supporting intervals, exact times, and weekday schedules; renumbers RELOAD_TYPE constants; adds weekday and text-color constants; migrates scripts and removes related manifests/talkactions. Changes
Sequence Diagram(s)sequenceDiagram
%% Participants
participant Script as Lua Script
participant Schedule as ScheduleEvent (Lua)
participant Scheduler as Server Scheduler (addEvent)
participant Game as Game API
Note over Script,Schedule: Script constructs and configures a ScheduleEvent
Script->>Schedule: event = ScheduleEvent(timeOrSpec)
Script->>Schedule: event.onTrigger = callback
Script->>Schedule: event:register()
Schedule->>Scheduler: schedule via addEvent (interval | exact-time poll | weekday check)
Scheduler-->>Schedule: invoke event callback when due
Scheduler->>Game: callback may call Game API
Note over Scheduler,Schedule: per-instance _lastTrigger dedupes same-tick invocations
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (1)data/scripts/lib/schedule_event.lua (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (9)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
data/scripts/lib/schedule_event.lua (1)
29-35: Consider adding range validation for parsed time values.The
parseTimefunction validates format but not value ranges. Invalid values like "99:99:99" would parse successfully but never trigger. While this doesn't cause runtime errors, it could silently fail for users who make typos.🔎 Optional: Add range validation
local function parseTime(str) local h, m, s = str:match("^(%d%d):(%d%d):(%d%d)$") if not h then return nil end - return tonumber(h), tonumber(m), tonumber(s) + h, m, s = tonumber(h), tonumber(m), tonumber(s) + if h > 23 or m > 59 or s > 59 then + return nil + end + return h, m, s end
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
data/cpplinter.luadata/globalevents/globalevents.xmldata/globalevents/lib/globalevents.luadata/lib/compat/compat.luadata/lib/core/constants.luadata/scripts/lib/helper_constructors.luadata/scripts/lib/schedule_event.luadata/scripts/scheduleevents/raids.luadata/scripts/scheduleevents/raids/#test_raid.luadata/scripts/scheduleevents/server_save.luadata/scripts/scheduleevents/world_light.luadata/scripts/scheduleevents/world_time.luadata/scripts/talkactions/commands/force_event.luadata/scripts/talkactions/commands/reload.luasrc/CMakeLists.txtsrc/const.hsrc/game.cppsrc/globalevent.cppsrc/globalevent.hsrc/lua/CMakeLists.txtsrc/lua/modules.cppsrc/lua/modules/game.cppsrc/lua/modules/globalevent.cppsrc/lua/modules/globals.cppsrc/lua/register.hsrc/lua/script.cppsrc/scriptmanager.cppsrc/signals.cppvc18/atlas.vcxprojvc18/atlas.vcxproj.filters
💤 Files with no reviewable changes (20)
- src/const.h
- src/CMakeLists.txt
- src/lua/register.h
- src/lua/modules/game.cpp
- src/game.cpp
- src/scriptmanager.cpp
- src/lua/script.cpp
- data/lib/compat/compat.lua
- src/lua/CMakeLists.txt
- data/globalevents/globalevents.xml
- src/signals.cpp
- src/lua/modules/globals.cpp
- data/globalevents/lib/globalevents.lua
- data/scripts/talkactions/commands/force_event.lua
- src/lua/modules.cpp
- src/globalevent.h
- data/scripts/talkactions/commands/reload.lua
- vc18/atlas.vcxproj.filters
- src/globalevent.cpp
- src/lua/modules/globalevent.cpp
🧰 Additional context used
🧬 Code graph analysis (1)
data/scripts/lib/schedule_event.lua (1)
data/cpplinter.lua (1)
addEvent(1143-1143)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: test
- GitHub Check: windows
- GitHub Check: ubuntu
- GitHub Check: format
- GitHub Check: docker-image
- GitHub Check: Analyze (c-cpp)
🔇 Additional comments (14)
vc18/atlas.vcxproj (1)
123-245: Build configuration correctly reflects GlobalEvent removal.The globalevent.cpp and globalevent.h references have been cleanly removed from both the
<ClCompile>and<ClInclude>item groups. The file structure remains well-formed, items are correctly ordered alphabetically, and no orphaned or dangling references remain. The changes align perfectly with the broader migration away from the GlobalEvent subsystem to ScheduleEvent.Also applies to: 247-342, 346-346
data/lib/core/constants.lua (1)
1-7: LGTM!The weekday constants correctly align with Lua's
os.date("*t").wdayconvention where Sunday is 1 and Saturday is 7. This provides clear, readable identifiers for the new ScheduleEvent weekday-based scheduling feature.data/scripts/lib/schedule_event.lua (4)
1-27: LGTM!Clean Lua OOP implementation with proper metatable setup. The
__newindexmetamethod for interceptingonTriggerassignments provides good validation whilerawsetavoids infinite recursion.
75-89: LGTM!The 1-second polling approach with
_lastTriggerstamp deduplication is a reasonable pattern for clock-based events. The stamp includesydayensuring daily events won't re-trigger within the same day.
101-131: LGTM!Thorough validation of weekday keys and values. The separation between
dayTimes(for specific times) anddayIntervals(for interval-based) provides good flexibility.
153-162: Day-based intervals trigger immediately on registration, even mid-week.When registering a ScheduleEvent with day-based intervals (e.g.,
[FRIDAY] = 30000), the loop fires immediately if it's Friday. This differs from the time-based modes which wait for the scheduled time. This may be intentional, but worth confirming it matches expected behavior.data/scripts/lib/helper_constructors.lua (1)
1-1: LGTM!Correct removal of
GlobalEventfrom the classes list since it's been replaced byScheduleEvent, which manages its own construction via a custom__callmetamethod.data/scripts/scheduleevents/world_light.lua (1)
38-51: LGTM!Clean migration from
GlobalEventtoScheduleEvent(10000). The 10-second interval for world light updates is preserved, and the handler logic remains unchanged.data/scripts/scheduleevents/server_save.lua (1)
37-49: LGTM!Correct migration to time-based
ScheduleEvent("09:55:00"). The server save warning logic and countdown timer chain remain intact.data/scripts/scheduleevents/raids.lua (1)
7-35: LGTM!Clean migration from
GlobalEventtoScheduleEvent(CHECK_RAIDS_INTERVAL). The 60-second raid check interval and execution logic are preserved correctly.data/scripts/scheduleevents/world_time.lua (1)
4-14: LGTM!Correct migration with 2500ms interval matching the documented "2.5s realtime = 1min worldtime" ratio. World time calculation logic is preserved.
data/scripts/scheduleevents/raids/#test_raid.lua (1)
37-50: No action needed — this is a newly created test file, not a migration from legacy code.The file was added in commit f2ae7a0 and did not previously exist, so there is no unit conversion regression. The short 1800ms (1.8 second) interval is appropriate for a test raid file (indicated by the
#prefix) and allows rapid iteration during development. The events scheduled within (1000–30000ms) are deliberately staggered for testing purposes.Likely an incorrect or invalid review comment.
data/cpplinter.lua (2)
1847-1856: LGTM! Constants correctly defined and used by name.All Game.reload() calls reference the RELOAD_TYPE constants by name rather than hardcoded values, as shown in data/scripts/talkactions/commands/reload.lua. This ensures code remains maintainable if constant values change.
1039-1039: The suggested annotation change is incorrect and should not be applied.The current type annotation
table<number, number|table<string>>correctly captures that the table values can be either numbers (intervals) or tables of strings (multiple clock times). The implementation at lines 93-96 and 111-120 confirms this:[SUNDAY] = {"10:00:00", "23:00:00"}shows a table of time strings, and[FRIDAY] = 30000shows a numeric interval.The suggested fix to
table<number, number|string>removes the crucial fact that one value type is a table of strings, making it inaccurate. While the inner table annotation could more precisely usetable<integer, string>instead oftable<string>, the proposed change represents a regression.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
data/scripts/lib/schedule_event.lua (3)
30-36: Consider adding time range validation.The
parseTimefunction validates the format but doesn't verify that hours (0-23), minutes (0-59), and seconds (0-59) are within valid ranges. Whileos.datewill handle invalid values gracefully, explicit validation would catch configuration errors earlier.🔎 Optional enhancement to add range validation
local function parseTime(str) local h, m, s = str:match("^(%d%d):(%d%d):(%d%d)$") if not h then return nil end - return tonumber(h), tonumber(m), tonumber(s) + h, m, s = tonumber(h), tonumber(m), tonumber(s) + if h > 23 or m > 59 or s > 59 then + return nil + end + return h, m, s end
55-58: Consider wrapping callbacks in pcall for resilience.If
self.callback()throws an error, the loop will terminate and the scheduled event stops executing. This could lead to silent failures that are difficult to debug.🔎 Proposed fix to add error handling
local function loop() - self.callback() + local success, err = pcall(self.callback) + if not success then + print("[Error - ScheduleEvent] Callback failed: " .. tostring(err)) + end addEvent(loop, self.time) endApply the same pattern to the time-based callback at line 83 and the weekday callback at line 157.
154-163: Consider optimizing per-day interval loops.The current implementation schedules a loop that runs continuously every
intervalmilliseconds but only executes the callback whenos.date("*t").wday == dayIndex. This wastes CPU cycles checking the day on every iteration.For a Friday-only event with a 30-second interval, the loop checks the day 20,160 times per week but only executes 2,880 times.
💡 Alternative approach
While more complex, you could calculate the time until the next occurrence on the target day and schedule just once, then reschedule for the next week when it triggers. However, given that the current approach is simpler and the overhead is minimal for typical use cases, this optimization may be deferred.
data/cpplinter.lua (1)
1037-1044: Refine the type annotation forScheduleEvent.time.The current annotation
table<number, number|table<string>>is imprecise. Based on the implementation indata/scripts/lib/schedule_event.lua, whentimeis a table, the keys are weekday numbers (e.g.,SUNDAY,SATURDAY) and the values are either:
- A numeric interval in milliseconds, or
- An array of time strings in "HH:MM:SS" format
🔎 More precise type annotation
---@class ScheduleEvent ----@field time number|string|table<number, number|table<string>> Interval(ms), "HH:MM:SS" string, or table with day → interval/times +---@field time number|string|table<number, number|string[]> Interval(ms), "HH:MM:SS" string, or table mapping weekday constants to intervals or time string arrays ---@field onTrigger fun():nil Callback function executed when the event triggers ---@field register fun(self:ScheduleEvent):boolean Registers the event, returns true if successful
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
data/cpplinter.luadata/scripts/lib/schedule_event.luavc18/atlas.vcxproj
💤 Files with no reviewable changes (1)
- vc18/atlas.vcxproj
🧰 Additional context used
🧬 Code graph analysis (1)
data/scripts/lib/schedule_event.lua (1)
data/cpplinter.lua (1)
addEvent(1143-1143)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: format
- GitHub Check: ubuntu
- GitHub Check: windows
- GitHub Check: test
- GitHub Check: docker-image
- GitHub Check: Analyze (c-cpp)
🔇 Additional comments (5)
data/scripts/lib/schedule_event.lua (3)
1-16: LGTM!The constructor and callable metatable setup follow idiomatic Lua patterns. The initialization of
_lastTriggerasnilcorrectly supports the deduplication logic used in time-based scheduling.
138-146: Potential issue with _lastTrigger for multiple times on the same day.The
_lastTriggerfield is shared across all scheduled times. If the table includes multiple times for the same day (e.g.,[SUNDAY] = {"10:00:00", "18:00:00"}), both times will update the same_lastTriggerfield. This could prevent the second time from firing if both happen to match in the same second (unlikely but possible due to clock adjustments or system lag).🔎 Proposed fix
Instead of a single
_lastTrigger, consider using a table keyed by the full timestamp:- if now.hour == t[1] and now.min == t[2] and now.sec == t[3] - and self._lastTrigger ~= stamp then - self._lastTrigger = stamp + local lastTrigger = self._lastTrigger or {} + if now.hour == t[1] and now.min == t[2] and now.sec == t[3] + and lastTrigger[stamp] ~= true then + lastTrigger[stamp] = true + self._lastTrigger = lastTrigger self.callback()However, this would require cleanup to prevent unbounded memory growth. Given the rarity of this issue, it may be acceptable as-is.
Likely an incorrect or invalid review comment.
106-110: The weekday constants (SUNDAYthroughSATURDAY) are properly defined indata/lib/core/constants.luaand loaded globally bydata/lib/core/core.luabefore schedule events execute. The code at lines 106-110 correctly references these constants, and no issue exists with this validation logic.data/cpplinter.lua (2)
2303-2303: LGTM!The
SCHEDULER_MINTICKS = 50constant establishes a reasonable minimum interval (50ms) for scheduled events, preventing excessive CPU usage from overly frequent polling while maintaining responsive scheduling.
1847-1856: No action needed—the RELOAD_TYPE constants in data/cpplinter.lua (lines 1847-1856) are correctly aligned with the C++ enumReloadTypes_tin src/const.h.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
data/scripts/lib/schedule_event.lua (3)
54-70: Consider event lifecycle management for numeric interval mode.The recursive
addEvent(loop, self.time)pattern creates a self-sustaining callback loop with no built-in mechanism to stop or cancel it. If long-running servers need to unregister or modify scheduled events, consider adding:
- A stop/cancel method that sets an internal flag checked before re-scheduling
- Or document that ScheduleEvent instances are intended to persist for the server lifetime
165-177: Optional: Optimize weekday interval checks.Each interval loop independently checks
os.date("*t").wday == dayIndexon every trigger (line 167). If multiple weekdays have intervals, this creates redundant polling. Consider caching the current day at a global/module level and updating it once per day-change to reduceos.datecalls.
122-131: Consider validating non-empty time arrays.If a weekday entry contains an empty table (e.g.,
{ [SUNDAY] = {} }), the validation passes but no times are scheduled, which might be unintentional. Consider adding a check to warn if time arrays are empty.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
data/cpplinter.luadata/scripts/lib/schedule_event.lua
🧰 Additional context used
🧬 Code graph analysis (1)
data/scripts/lib/schedule_event.lua (1)
data/cpplinter.lua (1)
addEvent(1143-1143)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: windows
- GitHub Check: format
- GitHub Check: ubuntu
- GitHub Check: test
- GitHub Check: docker-image
- GitHub Check: Analyze (c-cpp)
🔇 Additional comments (10)
data/scripts/lib/schedule_event.lua (5)
1-16: LGTM: Clean constructor and metatable setup.The ScheduleEvent class initialization follows standard Lua patterns with proper metatable setup and a callable constructor.
18-28: LGTM: Proper validation of onTrigger callback.The
__newindexmetamethod correctly enforces type validation for the onTrigger callback and usesprint()for warnings, allowing graceful failure rather than throwing errors.
30-41: LGTM: Robust time string parser.The
parseTimehelper correctly validates HH:MM:SS format and range constraints.
77-101: Approve time-string mode with minor observation on polling precision.The 1-second polling interval with deduplication is appropriate for HH:MM:SS scheduling. Note that under heavy server load, if polling is delayed beyond 1 second, a trigger could be missed (though the deduplication prevents double-triggering).
116-120: Verify weekday constants are defined before use.The validation
day < SUNDAY or day > SATURDAYdepends on the constantsSUNDAYandSATURDAYbeing defined. If these constants are missing (e.g., not yet loaded), the comparison will fail silently or produce unexpected results.Run the following script to confirm the weekday constants are defined before this module loads:
#!/bin/bash # Description: Verify weekday constants SUNDAY and SATURDAY are defined in the codebase echo "=== Searching for SUNDAY constant definition ===" rg -n "^SUNDAY\s*=" --type lua echo -e "\n=== Searching for SATURDAY constant definition ===" rg -n "^SATURDAY\s*=" --type lua echo -e "\n=== Checking load order of schedule_event.lua ===" rg -n 'require.*schedule_event|dofile.*schedule_event' --type luadata/cpplinter.lua (5)
1037-1044: LGTM: Accurate type annotations for ScheduleEvent.The type annotations correctly document the ScheduleEvent API, including the union type for
time(number, string, or table) and the callable constructor.
2303-2303: Confirm SCHEDULER_MINTICKS constant value aligns with system requirements.The
SCHEDULER_MINTICKS = 50constant enforces a minimum 50ms interval for scheduled events. Verify this threshold is appropriate for the server's event system performance characteristics (e.g., thataddEventcan reliably handle sub-100ms intervals).
2305-2320: LGTM: TEXTCOLOR constants added.The new text color constants provide a comprehensive palette for in-game messages.
1037-2320: Weekday constants (SUNDAY through SATURDAY) are defined indata/lib/core/constants.luaand correctly used inschedule_event.lua(e.g., line 117). No issues detected.
1841-1856: No hardcoded RELOAD_TYPE values found—all references use symbolic names correctly.Verification confirms that all RELOAD_TYPE constant references throughout the codebase (in C++ src/const.h, src/game.cpp, and Lua data/cpplinter.lua) use symbolic names rather than hardcoded integers. The constants are properly registered in src/lua/modules/globals.cpp and consistently referenced in game logic. No issues detected.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
data/scripts/lib/schedule_event.lua (1)
165-177: Consider documenting the day-interval behavior.The
dayIntervalsimplementation fires the callback repeatedly at the specified interval, but only on the matching weekday. For example,[SUNDAY] = 30000will trigger every 30 seconds throughout Sunday, then stop on other days. This behavior might not be immediately obvious to users.Consider adding a code comment or documentation explaining this behavior:
+ -- For each day with an interval, create a loop that fires at the specified + -- interval but only when the current weekday matches for dayIndex, interval in pairs(dayIntervals) do local function loop() if os.date("*t").wday == dayIndex then
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
data/scripts/lib/schedule_event.luasrc/lua/modules/game.cpp
🧰 Additional context used
🧬 Code graph analysis (1)
data/scripts/lib/schedule_event.lua (1)
data/cpplinter.lua (1)
addEvent(1143-1143)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: ubuntu
- GitHub Check: windows
- GitHub Check: test
- GitHub Check: docker-image
- GitHub Check: format
- GitHub Check: Analyze (c-cpp)
🔇 Additional comments (6)
data/scripts/lib/schedule_event.lua (5)
1-16: LGTM!The constructor and metatable setup follow idiomatic Lua patterns. The
__callmetamethod provides a convenient shorthand for instantiation.
18-28: LGTM!The
__newindexmetamethod correctly validates thatonTriggeris a function and maps it to the internalcallbackfield. Usingrawsetprevents infinite recursion.
30-41: LGTM!The
parseTimefunction correctly validates the HH:MM:SS format and range constraints for hours, minutes, and seconds.
112-142: No action required—weekday constants are properly defined and globally accessible.The
SUNDAYandSATURDAYconstants used in this code are defined indata/lib/core/constants.luaand loaded globally viadata/lib/core/core.luathrough the standard Lua bootstrap mechanism (dofile). They are available throughout the codebase, including inschedule_event.lua, without any issues.Likely an incorrect or invalid review comment.
54-70: No issue here.SCHEDULER_MINTICKSis properly registered in Lua throughsrc/lua/modules/game.cpp:679viaregisterEnum(lsi, SCHEDULER_MINTICKS)before this module is loaded, making it available for use at line 55.Likely an incorrect or invalid review comment.
src/lua/modules/game.cpp (1)
679-679: SCHEDULER_MINTICKS is properly defined and correctly registered.The constant is defined in
src/scheduler.hwith value50(milliseconds minimum) and is already used throughout the codebase before being exported to Lua. No issues found.
| WEDNESDAY = 4 | ||
| THURSDAY = 5 | ||
| FRIDAY = 6 | ||
| SATURDAY = 7 |
There was a problem hiding this comment.
ISO 8601 dictates weeks start on Monday and we should follow that to integrate with other datetime code
[D] is the weekday number, from 1 through 7, beginning with Monday and ending with Sunday.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
data/scripts/lib/schedule_event.lua
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: docker-image
- GitHub Check: test
- GitHub Check: ubuntu
- GitHub Check: format
- GitHub Check: windows
- GitHub Check: Analyze (c-cpp)
🔇 Additional comments (4)
data/scripts/lib/schedule_event.lua (4)
1-28: LGTM: Clean class structure with good validation.The ScheduleEvent class is well-structured with standard Lua OOP patterns. The
__newindexmetamethod provides helpful runtime validation ofonTrigger, ensuring it's a function before assignment. The__callconvenience wrapper is a nice touch.
30-41: LGTM: Robust time parsing with validation.The
parseTimehelper correctly validates theHH:MM:SSformat and ensures values are within valid ranges. Returningnilon failure is idiomatic Lua.
43-70: LGTM: Interval mode with solid error handling.The number interval mode correctly validates parameters and uses
pcallto ensure callback errors don't terminate the recurring schedule. The recursiveaddEventpattern is appropriate for indefinite scheduling.
120-196: LGTM: Weekday mode supports flexible scheduling patterns.The table-based weekday mode elegantly handles both time-based schedules (specific times on specific days) and interval-based schedules (recurring intervals on specific days). The separation into
dayTimesanddayIntervalsis clean, and the validation ensures user input is well-formed.
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy GlobalEvents system and replaces it with a new Lua-based ScheduleEvent architecture. The new system provides a unified, type-safe way to handle interval-based, clock-based, and day-specific recurring events entirely in Lua without requiring XML configuration or C++ changes.
- Removes the entire C++ GlobalEvents subsystem (files, references, reload logic)
- Introduces a pure Lua ScheduleEvent implementation with support for intervals, exact times, and weekday schedules
- Migrates all existing scheduled events (world time/light, server save, raids) to the new system
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vc18/atlas.vcxproj.filters | Removes globalevent.cpp/h references from Visual Studio project filter configuration |
| vc18/atlas.vcxproj | Removes globalevent source/header files from Visual Studio project |
| src/signals.cpp | Removes GlobalEvents include and reload call from SIGHUP handler |
| src/scriptmanager.cpp | Removes GlobalEvents initialization, deletion, and global pointer |
| src/lua/script.cpp | Removes GlobalEvents include and extern declaration |
| src/lua/register.h | Removes registerGlobalEvent function declaration |
| src/lua/modules/globals.cpp | Removes RELOAD_TYPE_GLOBALEVENTS enum registration |
| src/lua/modules/globalevent.cpp | Deletes entire Lua binding module for GlobalEvent (181 lines) |
| src/lua/modules/game.cpp | Removes Game.startEvent Lua method and GlobalEvents include |
| src/lua/modules.cpp | Removes registerGlobalEvent call from module imports |
| src/lua/CMakeLists.txt | Removes globalevent.cpp from build sources |
| src/globalevent.h | Deletes header defining GlobalEvents and GlobalEvent classes (83 lines) |
| src/globalevent.cpp | Deletes implementation of GlobalEvents system (284 lines) |
| src/game.cpp | Removes GlobalEvents reload cases and clear() calls |
| src/const.h | Removes RELOAD_TYPE_GLOBALEVENTS enum value |
| src/CMakeLists.txt | Removes globalevent source/header from build configuration |
| data/scripts/talkactions/commands/reload.lua | Removes globalevent/globalevents reload type aliases |
| data/scripts/talkactions/commands/force_event.lua | Deletes /event talkaction that triggered GlobalEvents by name |
| data/scripts/scheduleevents/world_time.lua | Migrates from GlobalEvent to ScheduleEvent with 2500ms interval |
| data/scripts/scheduleevents/world_light.lua | Migrates from GlobalEvent to ScheduleEvent with 10000ms interval |
| data/scripts/scheduleevents/server_save.lua | Migrates from GlobalEvent time trigger to ScheduleEvent clock format |
| data/scripts/scheduleevents/raids/#test_raid.lua | Migrates test raid from GlobalEvent to ScheduleEvent |
| data/scripts/scheduleevents/raids.lua | Migrates raids checking logic to ScheduleEvent |
| data/scripts/lib/schedule_event.lua | Implements new ScheduleEvent class supporting intervals, clock times, and day-specific schedules |
| data/scripts/lib/helper_constructors.lua | Removes GlobalEvent from classes array |
| data/lib/core/constants.lua | Adds weekday constants (MONDAY through SUNDAY) |
| data/lib/compat/compat.lua | Removes GlobalEvent compatibility layer and deprecated raid execution functions |
| data/globalevents/lib/globalevents.lua | Deletes empty placeholder file |
| data/globalevents/globalevents.xml | Deletes empty XML configuration file |
| data/cpplinter.lua | Removes GlobalEvent type definitions, updates RELOAD_TYPE constants, adds ScheduleEvent type annotations and SCHEDULER_MINTICKS constant |
Comments suppressed due to low confidence (6)
data/scripts/scheduleevents/raids/#test_raid.lua:37
- The interval value of 1800 milliseconds appears to be incorrect. Based on the context (a raid event), this was likely intended to be 1800 seconds (30 minutes), which should be 1800000 milliseconds. The current value of 1800ms would cause the raid to trigger every 1.8 seconds, which is unrealistic for a raid event. This should be changed to 1800000 to match the likely original intent.
data/scripts/scheduleevents/raids/#test_raid.lua:47 - The
return truestatement at line 47 is unnecessary and misleading. The ScheduleEvent callback (onTrigger) doesn't use or expect a return value - it's executed viapcalland the result is only used to detect errors. This return statement should be removed for clarity.
data/scripts/scheduleevents/server_save.lua:46 - The
return not configManager.getBoolean(configKeys.SERVER_SAVE_SHUTDOWN)statement at line 46 is unnecessary and misleading. The ScheduleEvent callback (onTrigger) doesn't use or expect a return value - it's executed viapcalland the result is only used to detect errors. This return statement should be removed for clarity.
data/scripts/scheduleevents/world_light.lua:48 - The
return truestatements at lines 11, 42, and 48 are unnecessary and misleading. The ScheduleEvent callback (onTrigger) doesn't use or expect a return value - it's executed viapcalland the result is only used to detect errors. These return statements should be removed for clarity.
data/scripts/scheduleevents/world_time.lua:11 - The
return truestatement at line 11 is unnecessary and misleading. The ScheduleEvent callback (onTrigger) doesn't use or expect a return value - it's executed viapcalland the result is only used to detect errors. This return statement should be removed for clarity.
data/scripts/scheduleevents/raids.lua:12 - The
return truestatement at line 12 is unnecessary and misleading. The ScheduleEvent callback (onTrigger) doesn't use or expect a return value - it's executed viapcalland the result is only used to detect errors. This return statement should be removed for clarity.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pull Request Prelude
Changes Proposed
Removes the legacy GlobalEvents system and replaces it with the new ScheduleEvent architecture. All previously scheduled or repeated events are migrated to the new system, providing a unified, type-safe, and maintainable way to handle timed and recurring events.
Interval event:
Clock event:
Day-specific or mixed interval event example:
This change standardizes timed event handling and removes the need for global event polling or legacy scheduling logic.
Summary by CodeRabbit
New Features
Removals
Changes
✏️ Tip: You can customize this high-level summary in your review settings.