fix: throttle imbuement tracker updates to 1s interval #3766
Conversation
Added a timestamp check to updateImbuementTrackerStats to prevent frequent updates, ensuring updates only occur if at least one second has passed since the last update. Introduced m_lastImbuementTrackerUpdate to track the last update time.
dudantas
changed the title
There was a problem hiding this comment.
Pull Request Overview
This pull request adds rate-limiting to the imbuement tracker stats update mechanism to prevent excessive updates. The implementation checks whether the tracker window is open and ensures updates occur at most once per second using a timestamp-based throttling approach.
Key Changes:
- Added timestamp tracking member variable
m_lastImbuementTrackerUpdateto track the last update time - Refactored
updateImbuementTrackerStats()to implement early returns for closed windows and throttle updates to 1-second intervals
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/creatures/players/player.hpp | Adds m_lastImbuementTrackerUpdate member variable to track last update timestamp |
| src/creatures/players/player.cpp | Implements throttling logic with early returns and 1-second interval checks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bool moved = false; | ||
| bool m_isDead = false; | ||
| bool imbuementTrackerWindowOpen = false; | ||
| mutable int64_t m_lastImbuementTrackerUpdate = 0; |
There was a problem hiding this comment.
[nitpick] This variable is placed among boolean member variables, but would be better positioned near other timestamp tracking variables for better organization. Consider moving it to be near similar variables like lastHazardSystemCriticalHit (line 1658) or in the section around lines 1500-1525 where other last* timestamp variables are grouped (e.g., lastAttack, lastUIInteraction, lastToggleMount, etc.).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| const int64_t currentTime = OTSYS_TIME(); | ||
| if (currentTime - m_lastImbuementTrackerUpdate < 1000) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The new throttling logic discards calls that occur within one second of the previous update and never schedules a refresh after the window elapses. When two state changes happen quickly (for example, entering and immediately leaving a protection zone or equipping multiple imbuements in quick succession), the second call returns before sending data and there is no later invocation to push the final state, leaving the imbuement tracker UI stuck with stale values until some unrelated event triggers another update. The throttle should coalesce updates rather than suppressing the last one.
Useful? React with 👍 / 👎.
|
We have some methods that call this function when equipping or unequipping items. If you replace one item with another, it will only update the unequipped one. |
|
This pull request introduces a rate-limiting mechanism for updating imbuement tracker stats in the
Playerclass to prevent excessive updates within short time intervals. The update logic now checks if the tracker window is open and ensures updates occur at most once per second.Imbuement tracker update improvements:
m_lastImbuementTrackerUpdateto thePlayerclass to track the last update time for imbuement stats.Player::updateImbuementTrackerStats()to return early if the tracker window is closed or if less than one second has passed since the last update, thereby rate-limiting the update calls.