fix(Core/GameObject): Fix fishing bobber immediate expiration #24557
Conversation
Bobbers were expiring immediately after splash due to duration calculation producing values shorter than the 5-second click window. - Add minimum duration validation (5 seconds) in SpellEffects.cpp - Fix splash timing operator (> to >=) for correct window - Always reset respawn time to now+6s on splash for consistency This ensures players always have 6 seconds to click after splash while preserving random splash timing (3-13 seconds).
github-actions
bot
added
CORE
There was a problem hiding this comment.
Pull request overview
This PR adjusts fishing bobber timing so that bobbers no longer expire immediately after splashing and players consistently have a reasonable click window, while preserving the random splash timing.
Changes:
- Clamp the fishing bobber gameobject duration in
SpellEffects.cppso it never falls belowFISHING_BOBBER_READY_TIME(5s), and add a diagnostic warning when the calculated duration would be too short. - Change the fishing node splash trigger from
>to>=inGameObject.cppso the splash occurs correctly at the intended threshold. - On bobber splash, reset the gameobject respawn time to
now + FISHING_BOBBER_READY_TIME + 1seconds to give a consistent post-splash click window and log when the previous respawn time was too close.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/server/game/Spells/SpellEffects.cpp | Ensures fishing bobber duration is not shorter than the defined ready time, preventing negative/immediate splash and adding logging when duration had to be clamped. |
| src/server/game/Entities/GameObject/GameObject.cpp | Adjusts fishing node update logic to trigger splash at the correct time and to reset respawn time after splash, with additional logging about potentially problematic respawn times. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (oldRespawnTime <= now + FISHING_BOBBER_READY_TIME) | ||
| { | ||
| LOG_WARN("entities.gameobject", "Fishing bobber respawn time {} was too close to current time {} (only {}s remaining). Reset to {} seconds from now.", |
There was a problem hiding this comment.
This warning condition will be true for every normal fishing bobber splash, so the log will fire on every cast instead of only when there’s an abnormal respawn time. Because Update only enters this branch when GameTime::GetGameTime().count() >= m_respawnTime - FISHING_BOBBER_READY_TIME, it is always the case that oldRespawnTime <= now + FISHING_BOBBER_READY_TIME, making the check redundant and the message misleading. To avoid spamming WARN-level logs and misreporting normal behavior as problematic, consider tightening the condition (for example, logging only when oldRespawnTime <= now or when the remaining time is below a much smaller threshold) or removing the warning entirely if it’s no longer needed after the duration clamp in SpellEffects.cpp.
| if (oldRespawnTime <= now + FISHING_BOBBER_READY_TIME) | |
| { | |
| LOG_WARN("entities.gameobject", "Fishing bobber respawn time {} was too close to current time {} (only {}s remaining). Reset to {} seconds from now.", | |
| if (oldRespawnTime <= now) | |
| { | |
| LOG_WARN("entities.gameobject", "Fishing bobber respawn time {} was not in the future relative to current time {} ({}s remaining). Reset to {} seconds from now.", |
|
After some more testing, there is still a rare case where the bobber expires even before the splashing animation is triggered |
2 participants
Bobbers were expiring immediately after splash due to duration calculation producing values shorter than the 5-second click window.
This ensures players always have 6 seconds to click after splash while preserving random splash timing (3-13 seconds).
Changes Proposed:
This PR proposes changes to:
AI-assisted Pull Requests
Important
While the use of AI tools when preparing pull requests is not prohibited, contributors must clearly disclose when such tools have been used and specify the model involved.
Contributors are also expected to fully understand the changes they are submitting and must be able to explain and justify those changes when requested by maintainers.
These changes were created with the help of Claude Code.
Issues Addressed:
SOURCE:
The changes have been validated through:
Tests Performed:
This PR has been:
How to Test the Changes:
Known Issues and TODO List:
How to Test AzerothCore PRs
When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].
You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:
http://www.azerothcore.org/wiki/How-to-test-a-PR
REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).
For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.