fix(Scripts/ICC): make the Lady Deathwhisper elevator wait 7s at each… - #27149
fix(Scripts/ICC): make the Lady Deathwhisper elevator wait 7s at each…#27149EricksOliveira wants to merge 1 commit into
Conversation
… stop The elevator was driven by a free running 3000 ms tick that had nothing to do with the moment it actually reached an end of its path, so the time it stood still was uniformly random in [0, 3000] ms and a raid often could not board it. Count the dwell only while the transport is parked, so it always starts on arrival: rearm the timer while it is moving, and flip GOState once 7000 ms have passed at a stop. StaticTransport::Update clamps PathProgress to exactly 0 and GetPauseTime() and freezes there while GOState matches, so those comparisons detect "parked at a stop" exactly. The ride itself is untouched. The client renders it from TransportAnimation.dbc, so pause time and path progress are left alone and no SQL is needed. Closes azerothcore#20324 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
github-actions
Bot
added
Script
📝 WalkthroughWalkthroughThe change adds a named seven-second dwell-time constant for the Lady Deathwhisper elevator. The instance constructor uses this constant to initialize the elevator timer. Elevator updates reset the timer during movement, decrement it while the transport is parked at an endpoint, and reverse direction when the dwell interval expires. Merge Risk: 🟡 Moderate · up to The change is intended to guarantee a 7-second elevator stop, but the current timing logic can shorten that stop or cause immediate departure when an endpoint is reached. This bounded correctness issue should be addressed or explicitly accepted before merging. 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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: e4d2201f-5a17-4b62-b55c-69ddba689999
📒 Files selected for processing (1)
src/server/scripts/Northrend/IcecrownCitadel/instance_icecrown_citadel.cpp
| else if (DarkwhisperElevatorTimer <= diff) | ||
| { | ||
| if (trans->GetGoState() == GO_STATE_READY && trans->GetPathProgress() == 0) | ||
| trans->SetGoState(GO_STATE_ACTIVE); | ||
| else if (trans->GetGoState() == GO_STATE_ACTIVE && trans->GetPathProgress() == trans->GetPauseTime()) | ||
| trans->SetGoState(GO_STATE_READY); | ||
| DarkwhisperElevatorTimer = DARKWHISPER_ELEVATOR_DWELL_TIME; | ||
| trans->SetGoState(atBottom ? GO_STATE_ACTIVE : GO_STATE_READY); | ||
| } | ||
| } | ||
| else | ||
| DarkwhisperElevatorTimer -= diff; | ||
| else | ||
| DarkwhisperElevatorTimer -= diff; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f '^(Map|Transport)\.(cpp|h)$' . \
-x rg -n -C 6 \
'InstanceMap::Update|instance_data->Update|StaticTransport::Update|GetPathProgress|GetPauseTime' {}Repository: azerothcore/azerothcore-wotlk
Length of output: 6406
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- review guidance files ---'
for f in .agents/docs/code-review.md .agents/docs/self-review-rules.md .agents/docs/cpp-guidelines.md .agents/docs/cpp-scripts.md; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
sed -n '1,240p' "$f"
else
printf '\n--- missing: %s ---\n' "$f"
fi
done
printf '\n--- candidate files ---\n'
fd -t f -i 'map|transport' src/server/game src/server/scripts 2>/dev/null | head -80
printf '\n--- target source ---\n'
sed -n '1610,1710p' src/server/scripts/Northrend/IcecrownCitadel/instance_icecrown_citadel.cpp
printf '\n--- target diff ---\n'
git diff -- src/server/scripts/Northrend/IcecrownCitadel/instance_icecrown_citadel.cpp
printf '\n--- update implementations ---\n'
rg -n -C 18 'void (Map|InstanceMap|StaticTransport)::Update|instance_data->Update|Map::Update' src/server/gameRepository: azerothcore/azerothcore-wotlk
Length of output: 50387
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- review guidance files ---'
for f in .agents/docs/code-review.md .agents/docs/self-review-rules.md .agents/docs/cpp-guidelines.md .agents/docs/cpp-scripts.md; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
sed -n '1,240p' "$f"
else
printf '\n--- missing: %s ---\n' "$f"
fi
done
printf '\n--- target source ---\n'
sed -n '1610,1710p' src/server/scripts/Northrend/IcecrownCitadel/instance_icecrown_citadel.cpp
printf '\n--- target diff ---\n'
git diff -- src/server/scripts/Northrend/IcecrownCitadel/instance_icecrown_citadel.cpp
printf '\n--- update implementations ---\n'
rg -n -C 18 'void (Map|InstanceMap|StaticTransport)::Update|instance_data->Update|Map::Update' src/server/gameRepository: azerothcore/azerothcore-wotlk
Length of output: 50387
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- map update ordering ---'
sed -n '436,565p' src/server/game/Maps/Map.cpp
sed -n '968,1025p' src/server/game/Entities/Transport/Transport.cpp
printf '\n--- elevator declarations and initialization ---\n'
rg -n -C 8 'DARKWHISPER_ELEVATOR|DarkwhisperElevatorTimer|LadyDeathwisperElevatorGUID|GO_LADY_DEATHWHISPER_ELEVATOR' \
src/server/scripts/Northrend/IcecrownCitadel/instance_icecrown_citadel.cpp
printf '\n--- repository state ---\n'
git status --short
git diff --stat
git diff --unified=30 -- src/server/scripts/Northrend/IcecrownCitadel/instance_icecrown_citadel.cpp
printf '\n--- relevant current-master history summary ---\n'
git log -8 --oneline -- src/server/scripts/Northrend/IcecrownCitadel/instance_icecrown_citadel.cppRepository: azerothcore/azerothcore-wotlk
Length of output: 12080
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- change under review ---'
git show --format=fuller --stat --oneline 786a2c20
git show --format= --unified=25 786a2c20 -- src/server/scripts/Northrend/IcecrownCitadel/instance_icecrown_citadel.cpp
printf '\n--- parent implementation ---\n'
git show 786a2c20^:src/server/scripts/Northrend/IcecrownCitadel/instance_icecrown_citadel.cpp |
sed -n '55,85p;195,225p;1635,1685p;1895,1925p'
printf '\n--- update-diff bounds ---\n'
rg -n -C 5 't_diff|diff.*MAX|MAX.*diff|Update\([^)]*diff|World::Update' \
src/server/game/World src/server/game/Maps src/server/game/Server | head -240
printf '\n--- deterministic timer model ---\n'
python3 - <<'PY'
DWELL = 7000
def tick(timer, progress, diff, pause):
# StaticTransport::Update runs before InstanceMap::Update.
arrived = progress < pause and progress + diff >= pause
progress = pause if arrived else min(progress + diff, pause)
parked = progress == pause
if not parked:
timer = DWELL
action = "moving"
elif timer <= diff:
timer = DWELL
action = "depart"
else:
timer -= diff
action = "parked"
return timer, progress, action, arrived
for diff in (1000, 8000):
result = tick(DWELL, 6500, diff, 7000)
print({"diff": diff, "result": result})
PYRepository: azerothcore/azerothcore-wotlk
Length of output: 33187
Reset the dwell timer when the transport reaches an endpoint.
StaticTransport::Update runs before this code, so subtracting the full diff can shorten the stop or depart immediately when diff >= 7 seconds. Track endpoint arrival and decrement DarkwhisperElevatorTimer only on later updates.
Source: MCP tools
Self-review — fixElevatorICC → masterOutcome clean — 1 round, zero findings Reviewed By Claude Code, Opus 5 — self-review v0.3, 2026-08-15 Tested in-game by the author 7 s dwell timed at both stops over consecutive cycles; ridden as a passenger up and down; first departure after the Lady Deathwhisper kill; leaving and re-entering a saved instance with her already Review details (1 round)Intent Once Lady Deathwhisper is defeated, her elevator in the Oratory of the Damned (GameObject 202220, a Project rules Coverage every changed file reviewed; the changeset is a single file and Codestyle Round 1 —
|
1 participant
… stop
The elevator was driven by a free running 3000 ms tick that had nothing to do with the moment it actually reached an end of its path, so the time it stood still was uniformly random in [0, 3000] ms and a raid often could not board it.
Count the dwell only while the transport is parked, so it always starts on arrival: rearm the timer while it is moving, and flip GOState once 7000 ms have passed at a stop. StaticTransport::Update clamps PathProgress to exactly 0 and GetPauseTime() and freezes there while GOState matches, so those comparisons detect "parked at a stop" exactly.
The ride itself is untouched. The client renders it from TransportAnimation.dbc, so pause time and path progress are left alone and no SQL is needed.
Changes Proposed:
This PR proposes changes to:
AI-assisted Pull Requests
Important
Using AI tools to prepare pull requests is allowed, but it must be disclosed and it must follow our AC guidelines for AI Agentic Engineering (link below).
You are expected to fully understand the changes you submit and to be able to explain and justify them when maintainers ask.
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.