Skip to content

Conversation

@offl
Copy link
Contributor

@offl offl commented Nov 22, 2025

Changes proposed:

  • Rewrite Nalorakk, I'm not happy with the way waves are implemented, but it works

Issues addressed:

none

Tests performed:

builds, tested in-game

{
BossAI::JustEngagedWith(who);
std::vector<Creature*> waveCreatures;
GetCreatureListWithOptionsInGrid(waveCreatures, me, 50.0f, { .StringId = NalorakkWave[_currentWaveCount] });
Copy link
Contributor

Choose a reason for hiding this comment

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

you can use instance as a holder, so you dont have to use grid searchers

Copy link
Contributor

Choose a reason for hiding this comment

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

aka: oncreaturecreate -> store in appropriate storage -> use it in w/e way u want (make a script in instance to attack a unit, for example)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Creatures aren't spawned all at once, they're summoned in waves and we still will have to use StringIds to store them

if (!creature)
return;

if (creature->HasStringId("NalorakkWave1"))
Copy link
Contributor

@ccrs ccrs Nov 23, 2025

Choose a reason for hiding this comment

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

the stringid is overkill if you ask me, you can "simply" check if the creature is within the related group

if (SpawnMetadata const* data = sObjectMgr->GetSpawnMetadata(SpawnObjectType::SPAWN_TYPE_CREATURE, me->GetSpawnId()))
            if (data->spawnGroupData && data->spawnGroupData->groupId == SPAWN_GROUP_MOJO)

maybe there is an easier way to access groupId on this context though (InstanceMapScript)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm currently scripting similar event and in that event I created no spawn groups

Copy link
Contributor

Choose a reason for hiding this comment

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

the stringid is overkill if you ask me, you can "simply" check if the creature is within the related group

if (SpawnMetadata const* data = sObjectMgr->GetSpawnMetadata(SpawnObjectType::SPAWN_TYPE_CREATURE, me->GetSpawnId()))
            if (data->spawnGroupData && data->spawnGroupData->groupId == SPAWN_GROUP_MOJO)

maybe there is an easier way to access groupId on this context though (InstanceMapScript)

we should not do that. SpawnGroups, as they are currently implemented, should only be used for controlling spawns, not to integrate them into scripts directly.

Copy link
Member

Choose a reason for hiding this comment

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

This is one of the reasons why stringid exists, to target specific spawns (or packs) without hardcoding guids (or spawn group ids)

Copy link
Contributor Author

@offl offl Nov 24, 2025

Choose a reason for hiding this comment

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

To be honest, I really doubt string ids are used on retail this way. I think what they did here is attached GameEvent to Death \ Despawn Event. Once all members in Spawn Group are dead, event is launched, then Nalorakk "catches" that event and executes script linked to that event

{
++killedUnitInWaveCounter[0];

if (killedUnitInWaveCounter[0] == 3)
Copy link
Contributor

Choose a reason for hiding this comment

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

is respawn/reset/wipe handling necessary? meaning, do you need to reset this counter to 0 somewhere in case of any of these situations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dunno how this event should be handled in any of these cases

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants