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.

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

[WIP][3.3.5] Implement spawn group management in InstanceScript #20103

Merged
merged 4 commits into from
Aug 3, 2017

Conversation

Treeston
Copy link
Contributor

@Treeston Treeston commented Aug 2, 2017

What it says on the tin - begin dynspawn follow-up work by gradually phasing out linked_respawn in favor of spawn groups for instance spawn control.

Medium term, once all instances have appropriate DB data, this will allow us to remove the instanceId column from the respawn tables altogether, relying exclusively on this system.

New DB table instance_spawn_groups:

  • instanceMapId - duh
  • bossStateId - duh
  • bossStates - bitmask, 1 << state for EncounterState values
  • flags - bitmask, currently allows 0x1 (FLAG_ACTIVATE_SPAWN) and 0x2 (FLAG_BLOCK_SPAWN)

A spawn group is activated if any of its FLAG_ACTIVATE_SPAWN conditions are met, unless any of its FLAG_BLOCK_SPAWN conditions are met - this allows for some neat complex management if necessary.

PS: Instance spawn data is currently being crowdsourced - check this thread.

@@ -347,6 +381,13 @@ bool InstanceScript::_SkipCheckRequiredBosses(Player const* player /*= nullptr*/
return player && player->GetSession()->HasPermission(rbac::RBAC_PERM_SKIP_CHECK_INSTANCE_REQUIRED_BOSSES);
}

void InstanceScript::Create()
{
for (int8 i = 0; i < bosses.size(); ++i)
Copy link

Choose a reason for hiding this comment

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

/home/travis/build/TrinityCore/TrinityCore/src/server/game/Instances/InstanceScript.cpp:386:24:
 fatal error:
 comparison of integers of different signs: 'int8' (aka 'signed char') and 'size_type' (aka 'unsigned long') [-Wsign-compare]
    for (int8 i = 0; i < bosses.size(); ++i)
                     ~ ^ ~~~~~~~~~~~~~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I am capable of reading travis output. Thank you.

@@ -21,6 +21,7 @@

#include "ZoneScript.h"
#include "Common.h"
#include "ObjectMgr.h"
Copy link
Member

Choose a reason for hiding this comment

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

objection.
not this header.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hrm, just use the actual type (std::unordered_multimap< etc) instead of the typedef?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or no, that won't work either (type itself is defined in ObjectMgr.h). I'll think of something.

uint32 spawnGroupId;
uint8 flags;
};
typedef std::unordered_multimap<uint16, InstanceSpawnGroupInfo> InstanceSpawnGroupMap;
Copy link
Member

Choose a reason for hiding this comment

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

I still don't like this, none of these headers should include each other (in any direction)
Proposed fix: change the container to std::unordered_map<uint16, std::vector<InstanceSpawnGroupInfo>>
and hold a pointer to that vector in InstanceScript

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but where do we define InstanceSpawnGroupInfo? Class definitions need to be available at point of substitution, iirc?

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't matter. vector permits incomplete types

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, great.

@Krudor
Copy link
Contributor

Krudor commented Aug 2, 2017

Hell yes

@Treeston Treeston merged commit 84590be into TrinityCore:3.3.5 Aug 3, 2017
@Treeston Treeston deleted the 3.3.5-instancespawngroup branch August 3, 2017 22:23
@Keader
Copy link
Contributor

Keader commented Jan 23, 2020

@Treeston , @Ovahlord we need update the wiki with this table.

@ghost
Copy link

ghost commented Jan 23, 2020

Oh right. I see. At least we need a new table name entry and the preliminary field names,
so others can fill in the necessary explanations based on the following SQL :

CREATE TABLE `instance_spawn_groups` (
  `instanceMapId` smallint(5) unsigned not null,
  `bossStateId` tinyint unsigned not null,
  `bossStates` tinyint unsigned not null,
  `spawnGroupId` int unsigned not null,
  `flags` tinyint unsigned not null,
  PRIMARY KEY (`instanceMapId`,`bossStateId`,`spawnGroupId`,`bossStates`)

Shauren pushed a commit that referenced this pull request Aug 22, 2020
…wn groups based on boss state. (#20103)

(cherry picked from commit 84590be)
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.

4 participants