Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Scripts/Battlegrounds: Implement Deephaul Ravine #30393

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Scripts/Battlegrounds: Implement Deephaul Ravine #30393

wants to merge 5 commits into from
+1,839 −2

Conversation

Golrag
Copy link
Contributor

@Golrag Golrag commented Oct 31, 2024 •
edited
Loading

Changes proposed:

  • Implement Deephaul Ravine
  • Make sure spawn group states are initialised before calling BattlegroundMapScript::OnInit

Issues addressed:

Closes: None

Tests performed:

  • Builds locally
  • Running around in battleground, capturing carts & crystals

Known issues and TODO list: (add/remove lines as needed)

  • Recheck with 11.0.5 on retail; see if they fixed some stuff that didnt match (texts like "the flag has been returned")
  • Check which achievement works and which not
  • [Out of Scope] When loading in the gates spawn in and you can run through them before they finish their closing animation. Update Object of every gameobject/npc is send AFTER the loading screen for some reason (in Map::Update).
  • [Out of Scope] The healing buff doesnt have a healing spell triggering. Some serverside spell stuff is going on and its not working in any bg.
  • The location where you have to drop the crystal already contains a crystal for some reason (something about npc animation?)
  • Crystal should be hidden till worldstate enables it

@Golrag Golrag force-pushed the bg_deephaul_ravine branch 2 times, most recently from 2e3bbba to 98a2b36 Compare November 1, 2024 20:17
Nyr97
Nyr97 reviewed Nov 22, 2024
View reviewed changes
src/server/scripts/Battlegrounds/DeephaulRavine/battleground_deephaul_ravine.cpp
void OnStart() override
{
BattlegroundScript::OnStart();
_scheduler.Schedule(15s, [&](TaskContext)
Copy link
Contributor

@Nyr97 Nyr97 Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small improvement: do you think you can move any timer to its own enum like Timings? You can make them static constexpr std::chrono::seconds, so it's easier to maintain if anything changes rather than having magic numbs:

enum Timings { static constexpr std::chrono::seconds FlagRespawnTime = 15s; static constexpr std::chrono::seconds CartRespawnTime = 5s; }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nyr97 Nyr97 Nyr97 left review comments

Assignees
No one assigned
Labels
Branch-master
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Golrag @Nyr97