Implement unsupported achievement criteria types

FrancescoBorzi commented 5 years ago

DESCRIPTION:

The following data has been removed from our world DB:

DELETE FROM `achievement_criteria_data` WHERE `criteria_id` IN (4989, 4988, 5620, 5632, 5619, 4987);
INSERT INTO `achievement_criteria_data` (`criteria_id`, `type`, `value1`, `value2`, `ScriptName`) VALUES
(4987, 12, 0, 0, ''),
(4988, 12, 0, 0, ''),
(4989, 12, 1, 0, ''),
(5619, 12, 0, 0, ''),
(5620, 12, 1, 0, ''),
(5632, 12, 0, 0, '');

because it's currently unsupported by our core, causing:

> 2018-12-26 15:46:14 Table `achievement_criteria_data` has data for non-supported criteria type (Entry: 4989 Type: 19), ignored.
> 2018-12-26 15:46:14 Table `achievement_criteria_data` has data for non-supported criteria type (Entry: 4988 Type: 19), ignored.
> 2018-12-26 15:46:14 Table `achievement_criteria_data` has data for non-supported criteria type (Entry: 5620 Type: 73), ignored.
> 2018-12-26 15:46:14 Table `achievement_criteria_data` has data for non-supported criteria type (Entry: 5632 Type: 73), ignored.
> 2018-12-26 15:46:14 Table `achievement_criteria_data` has data for non-supported criteria type (Entry: 5619 Type: 73), ignored.
> 2018-12-26 15:46:14 Table `achievement_criteria_data` has data for non-supported criteria type (Entry: 4987 Type: 19), ignored.

The core should implement those criteria types and the data should be added to the DB.

EXTRA NOTES:

Some notes made by @wetbrownsauce and @grimgravy :

Some additional info about the specific entries referenced in case it can help with testing:

Entry 4987 in achievement_criteria.dbc is linked to Entry 932 in achievement.dbc which is "Total 5-player dungeons entered".
Entry 4988 in achievement_criteria.dbc is linked to Entry 933 in achievement.dbc which is "Total 10-player raids entered".
Entry 4989 in achievement_criteria.dbc is linked to Entry 934 in achievement.dbc which is "Total 25-player raids entered".
Entry 5619 in achievement_criteria.dbc is linked to Entry 1241 in achievement.dbc which is "Mal'Ganis defeated (Caverns of Time: Stratholme)".
Entry 5620 in achievement_criteria.dbc is linked to Entry 1515 in achievement.dbc which is "Mal'Ganis defeated (Heroic CoT: Stratholme)".
Entry 5632 in achievement_criteria.dbc is linked to Entry 1103 in achievement.dbc which is "Lich King 5-player dungeons completed (final boss killed)".

From achievement_criteria.dbc:

ID 4987 has Type 19
ID 4988 has Type 19
ID 4989 has Type 19
ID 5619 has Type 73
ID 5620 has Type 73
ID 5632 has Type 73

From AchievementMgr.cpp, line 1702+:

// FIXME: not triggered in code as result, need to implement
case ACHIEVEMENT_CRITERIA_TYPE_COMPLETE_RAID:
case ACHIEVEMENT_CRITERIA_TYPE_OWN_RANK:
case ACHIEVEMENT_CRITERIA_TYPE_TOTAL:
    break;                                   // Not implemented yet :(

ACHIEVEMENT_CRITERIA_TYPE_COMPLETE_RAID corresponds to Type 19, which is why those first 3 criteria don't work. I couldn't find much info about Type 73.

Achievement_Criteria.dbc type 19(COMPLETE_RAID) achievement_criteria_data type 12(TYPE_MAP_DIFFICULTY) AchievementMgr.cpp was not implemented in code

BRANCH(ES):

master

AC HASH/COMMIT:

https://github.com/azerothcore/azerothcore-wotlk/commit/37d9ef012608a1ddaa839f7e6b63e22fcd821d90

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/69717864-implement-unsupported-achievement-criteria-types?utm_campaign=plugin&utm_content=tracker%2F40032087&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F40032087&utm_medium=issues&utm_source=github).

sudlud commented 1 year ago

I've tried to look deeper into criteria 73, this is what I've got so far.

There only exist 3 criteria entries of type 73 ID 5632, Achievement 1103, Type 73, Asset_Id 18716, "Mal'Ganis" ID 5619, Achievement 1241, Type 73, Asset_Id 18716, "Mal'Ganis" ID 5620, Achievement 1515, Type 73, Asset_Id 18716, "Mal'Ganis"

So all 3 are related to Mal'Ganis, boss of Cavern of Times - Stratholme https://www.wowhead.com/wotlk/npc=26533/malganis

All referenced achievements are statistics and have several criteria to beat the final bosses of instances. Each criteria will increase the achievement's statistics value by 1, repeatable.

For achievement 1103 e.g. there are 13 criteria

11x type 0 ACHIEVEMENT_CRITERIA_TYPE_KILL_CREATURE
1x type 28 ACHIEVEMENT_CRITERIA_TYPE_BE_SPELL_TARGET
1x type 73

The 11 kill-creature-criteria obviously make sense as you would normally kill the final boss directly.

What is interesting: all 3 of the type-73-criteria are accompanied by a type-28-criteria, also named "Mal'Ganis" with Asset_Id 58630: ID 6432, Achievement 1103, "Lich King 5-player dungeons completed (final boss killed)", Type 28, Asset_Id 58630, "Mal'Ganis" ID 6435, Achievement 1241, "Mal'Ganis defeated (Caverns of Time: Stratholme)", Type 28, Asset_Id 58630, "Mal'Ganis" ID 6434, Achievement 1515, "Mal'Ganis defeated (Heroic CoT: Stratholme)", Type 28, Asset_Id 58630, "Mal'Ganis"

This is because Mal'Ganis is not killed in Cavern of Times, he will disappear on 1% HP, see e.g.: https://www.wowhead.com/wotlk/npc=26533/malganis#comments

Therefore the player group gets the spell 58630 instead to mark their victory and trigger the achievement: https://wotlk.evowow.com/?spell=58630

So now this leads to the question why those 3 achievements have 2 criteria entries for Mal'Ganis (type 28 and 73), which could each increase the statistics counter by 1 if they get fulfilled?

Right now I see 2 options: A: there's another way to beat Mal'Ganis which corresponds to criteria type 73 -> otherwise if one run of "killing" Mal'Ganis would fullfill criteria 28 AND 73, the statistics counter would be increased by 2 instead of 1 as both criteria get fullfilled B: the type-73-entries are some kind of development leftover / irrelevant (not too unlikely as there also exist criteria of other unknown types that reference non-existent achievement-IDs)

Additionaly NOT all "Mal'Ganis" achievement criteria of type 28 are accompanied by a type 73 criteria, e.g.: ID 6381 , Achievement 479 , "The Culling of Stratholme" ID 6808 , Achievement 500 , "Heroic: The Culling of Stratholme" ID 6433 , Achievement 1467, "Lich King 5-player bosses killed" ID 5887 , Achievement 1658, "Champion of the Frozen Wastes" ID 12681, Achievement 4456, "Random Lich King (normal) dungeons completed" ID 12801, Achievement 4556, "Random Lich King (heroic) dungeons completed"

So what makes those 3 achievements at the beginning more special to get an additional type-73-entry?

I've also tried to figure out what Asset_Id 18716 could refer to, but for now this doesn't make much sense to me:

data/sql/base/db_world/creature_template.sql: entry 18716: name "Shadowy Initiate" -> https://wotlk.evowow.com/?npc=18716 -> https://www.wowhead.com/tbc/npc=18716/shadowy-initiate --> NPC from TBC in Terokkar Forest

data/sql/base/db_world/creature_template.sql: modelid1 = 18716: name "Sergeant Altumus" -> https://www.wowhead.com/tbc/npc=19309/sergeant-altumus --> NPC from TBC, Hellfire Peninsula

data/sql/base/db_world/item_template.sql: entry 18716: name "Ash Covered Boots" -> https://www.wowhead.com/classic/item=18716/ash-covered-boots -> https://wotlk.evowow.com/?item=18716 --> blue item from Stratholme, dropped by "Archivist Galford" ---> interesting - Mal'Ganis residates in Stratholme, Cavern of Times

createrInfoDisplayExtra.dbc also has an entry with ID 18716 -> ???

data/sql/base/db_world/item_template.sql: name "Ember of Emberseer" / "Grime-Encrusted Object" -> https://www.wowhead.com/wotlk/item=21988/ember-of-emberseer#dropped-by --> Quest-Item from Blackrock -> https://www.wowhead.com/classic/item=9308/grime-encrusted-object --> Quest-Item aus Gnomegeran (?)

data/sql/base/db_world/gameobject.sql: guid 18716 -> ???

On another note there's also no define for criteria type 73 in DBCEnums.h. This might also support the fact that it's maybe not that relevant.

So to conclude this - right now I would actually prefer to not implement criteria type 73 at all as the type 28 criterias should be sufficient for the relevant achievements/statistics.

Kitzunu commented 1 year ago

Here's what I gathered for criteria 73 by poking around in TC Master

Implemented here, https://github.com/TrinityCore/TrinityCore/commit/22edf79c70519a571d9eeb7e397add695d7a28bb, though not feasible for us

/// @todo 73: Achievements 1515, 1241, 1103 (Name: Mal'Ganis)
CRITERIA_TYPE_SEND_EVENT = 73
(PlayerTriggerGameEvent)

sudlud commented 1 year ago

For implementing criteria type 19 ACHIEVEMENT_CRITERIA_TYPE_COMPLETE_RAID I might need some help to gather additional info from retail wotlk.

The achievements are all named "Total x-player dungeons / raids" entered, whereas the define is named "..._completed".

Also the criteria are named like "5 player dungeons run".

So my expedition would be that it actually counts dungeons / raids entered (not completed) as this corresponds to the text shown in the game.

Could someone please check the following in retail:

check / write down your current statistics counter value for "Total 5-player dungeons entered"
enter 5 player dungeon and observe if statistics counter gets increased
enter 5 player heroic dungeon and also observe of the statistics counter changed
(optional: also test for "total 10-/25-player raids entered by entering those, too)

Thanks!

Kitzunu commented 1 year ago

@azerothcore/sniffers

sudlud commented 1 year ago

Regarding criteria type 19 ACHIEVEMENT_CRITERIA_TYPE_COMPLETE_RAID:

I've just checked the behaviour on retail using a starter account with level 11 character.

right click character -> reset all instances of this character
enter Ragefire Chasm in OG -> observe increase by 1 for "5-player dungeons entered"
exit Ragefire Chasm in OG
enter Ragefire Chasm in OG -> observe NO CHANGE of "5-player dungeons entered"
right click character -> reset all instances of this character
enter Ragefire Chasm in OG -> observe increase by 1 for "5-player dungeons entered" (again)

note: you might have to switch statistics tab back & forth to update values shown there

conclusion:

the statistics relevant for ACHIEVEMENT_CRITERIA_TYPE_COMPLETE_RAID actually count the dungeons / raids ENTERED -> no increase on re-entering the same raids / same dungeon-session / ID

@Kitzunu would this justify renaming ACHIEVEMENT_CRITERIA_TYPE_COMPLETE_RAID to ACHIEVEMENT_CRITERIA_TYPE_ENTER_RAID?

Kitzunu commented 1 year ago

No, these are blizzard values/names. See https://wowdev.wiki/DB/Achievement_Criteria

sudlud commented 1 year ago

Criteria Type 117 is also unimplemented, used by statistic with:

achievement criteria 13258 "Disenchant rolls made on loot"
achievement 4778 "Disenchant rolls made on loot"

sudlud commented 1 year ago

So now the last remaining achievement criteria to implement is 73.

Kitzunu commented 11 months ago

Isn't that the one tied to Malganis in some way?

sudlud commented 11 months ago

Isn't that the one tied to Malganis in some way?

Yes, I in my comment above I had tried to find all relevant information on this:

https://github.com/azerothcore/azerothcore-wotlk/issues/1418#issuecomment-1712810240

So if there is no special game event that also allows to fight Mal'Ganis, I would actually just leave it unimplemented.

Kitzunu commented 11 months ago

I will close this as completed

azerothcore / azerothcore-wotlk