[patch]GOSSIP_NPC_FLAGS_CHECK

[virusav]

Ядро: 12157
Патч во вложении, копия http://paste2.org/p/2212329

Данный патч проверяет госсип меню на отсутствие пунктов с обязательными флагами в `gossip_menu_option`.`npc_option_npcflag`, которые есть в `creature_template`.`npcflag`.

Пример: у нпц есть флаг UNIT_NPC_FLAG_BANKER, но нет пункта меню с таким флагом.

[Vladimir]

Не сильно хорошо конечно что делается дополнительная выборка из базы, но тут зависит от сложности того же при пробежке по данным в памяти.

[virusav]

Я сначала начал писать патч, используя текущий цикл по оптионам, но в том варианте пришлось бы создавать массив для госсипа, в котором были бы все нпц, использующие госсип.
Далее надо было бы отслеживать начало и конец оптионов меню, суммировать флаги и при переходе к следующему меню делать проверку.

На мой взгляд, данный патч намного проще и менее ресурсоемкий..

[zergtmn]

Имхо такие проверки надо наоборот убирать из ядра и переносить в базу.

[virusav]

В базу перенести такие проверки нельзя, это просто хранилище данных.
Если только речь идет о триггерах, функциях и процедурах.

[Evgeniy]

Как по мне данные в базе - дело самой базы. Если они не правильные - база не корректная, а значит с такой базой нельзя запускаться и работать. Как вариант отдельная тулза которая проверяет базу на наличие ошибок.
Меньше проверок - быстрее работает.

[zergtmn]

База может больше чем просто хранилищем данных, смотря как ее использовать.
Данную проверку наверное нельзя реализовать триггером, но можно написать хранимую процедуру, которая будет проверять валидность данных в базе, и вызывать ее каждый раз при запуске (если в этом вообще есть смысл, в чем я сомневаюсь).

[virusav]

zergtmn, если данный патч не нужен в ядре, тогда надо переместить тему в отвергнутые, права на это есть.

[Vladimir]

Дополнительные проверки целостности базы нужны. Так как сейчас они делаются при старте сервера то не вижу в чем проблема с патчем чтобы говорить об отвергнутых. Я просто задал вопрос нельзя ли их без гемороя сделать чисто в коде - вижу что нельзя. Так что у меня других замечаний по патчу нет.

[Lordronn]

В тринити это (если я правильно понял что тут делают) сделано проще. При построении меню есть проверка флагов

            if (!(itr->second.OptionNpcflag & npcflags))
                continue;

в PrepareGossipMenu

[Konctantin]

может не надо создавать тип, а использовать готовую коллекцию:

std::map<NPCFlags, char*> associative = std::map<NPCFlags, char*>();
associative[UNIT_NPC_FLAG_TRAINER]             = "UNIT_NPC_FLAG_TRAINER";
associative[UNIT_NPC_FLAG_VENDOR]             = "UNIT_NPC_FLAG_VENDOR";
associative[UNIT_NPC_FLAG_FLIGHTMASTER]     = "UNIT_NPC_FLAG_FLIGHTMASTER";
associative[UNIT_NPC_FLAG_INNKEEPER]      = "UNIT_NPC_FLAG_INNKEEPER";
associative[UNIT_NPC_FLAG_BANKER]         = "UNIT_NPC_FLAG_BANKER";
associative[UNIT_NPC_FLAG_PETITIONER]     = "UNIT_NPC_FLAG_PETITIONER";
associative[UNIT_NPC_FLAG_TABARDDESIGNER] = "UNIT_NPC_FLAG_TABARDDESIGNER";
associative[UNIT_NPC_FLAG_BATTLEMASTER]   = "UNIT_NPC_FLAG_BATTLEMASTER";
associative[UNIT_NPC_FLAG_AUCTIONEER]     = "UNIT_NPC_FLAG_AUCTIONEER";
associative[UNIT_NPC_FLAG_STABLEMASTER]   = "UNIT_NPC_FLAG_STABLEMASTER";
associative[UNIT_NPC_FLAG_GUILD_BANKER]   = "UNIT_NPC_FLAG_GUILD_BANKER";

uint32 gossip_npcflags_all = 0;
for (std::map<NPCFlags, char*>::const_iterator itr = associative.begin(); itr != associative.end(); ++itr)
    gossip_npcflags_all |= itr->first;

QueryResult* result = WorldDatabase.PQuery (
                            "SELECT "
                            "`creature_template`.`entry`, `creature_template`.`gossip_menu_id`, (`creature_template`.`npcflag` & %u) &~ BIT_OR(`gossip_menu_option`.`npc_option_npcflag`) AS `flags` "
                            "FROM "
                            "`creature_template`, `gossip_menu_option` "
                            "WHERE "
                            "`creature_template`.`gossip_menu_id`>0 "
                            "AND `creature_template`.`ScriptName`='' "
                            "AND `gossip_menu_option`.`menu_id`=`creature_template`.`gossip_menu_id` "
                            "GROUP BY "
                            "`creature_template`.`entry`, `creature_template`.`gossip_menu_id` "
                            "HAVING "
                            "`flags`>0", gossip_npcflags_all);
if (result)
{
    BarGoLink bar (result->GetRowCount());
    do
    {
        bar.step();

        Field* fields = result->Fetch();

        for (std::map<NPCFlags, char*>::const_iterator itr = associative.begin(); itr != associative.end(); ++itr)
        {
            if (fields[2].GetUInt32() & itr->first)
                sLog.outErrorDb("Table `creature_template` with `entry` = %u and `gossip_menu_id` = %u has flag %s in `npcflag` but gossip menu does not have option with that value in `npc_option_npcflag`.", fields[0].GetUInt32(), fields[1].GetUInt32(), itr->second);
        }
    }
    while (result->NextRow());

    delete result;
}

[zergtmn]

Сообщение от Vladimir

Дополнительные проверки целостности базы нужны. Так как сейчас они делаются при старте сервера то не вижу в чем проблема с патчем чтобы говорить об отвергнутых. Я просто задал вопрос нельзя ли их без гемороя сделать чисто в коде - вижу что нельзя. Так что у меня других замечаний по патчу нет.

Проблема в загрузке по несколько минут в дебаге. Причем с пустыми таблицами в characters. Всяких проверок конечно можно налепить много, но какой смысл их постоянно прогонять когда база не меняется?

[virusav]

Lordronn, в ядре есть подобная проверка:

                if (gMenuItem.npc_option_npcflag & cInfo->npcflag)
                    found_flags_uses = true;

но это не то, т.к. проверка идет по флагам существующих оптионов, а мой патч находит отсутствующие.

[Vladimir]

В ядре есть специальный режим опциональных проверок для базы - там всякие не всегда корректные результаты и т.д. Почему туда не перенести проверку раз она не влияет на работу сервера. Проверки базы делаемые всегда для того чтобы не дать загрузится некорректным значениям влияющим на работу сервера своим наличием.
Проверки на отсуnствующие данные могу быть и опциональными, чисто для девелоперов базы.

[virusav]

Vladimir, я не осилил последний ваш пост.
Если бы знал, о чем речь, то сразу писал бы проверку там.

[KiriX]

Тогда уж может вести и отдельный лог, куда сразу будут складываться исправления?
Актуально, например, для исправления параметров в item_template да и ещё много чего можно пустить автоматом исправляться (ну или хотя бы выводить в отдельный лог запрос исправления).
В целом же я согласен скорее с zergtmn и Evgeniy тем более, что реализовать такие проверки силами самой базы (через триггеры, процедуры) вполне возможно. Давно пора использовать все возможности MySQL, о чём совсем недавно и писал сам Shmoo

[virusav]

KiriX, в данном случае нельзя сделать лог с исправлениями, т.к. оптионов нет, а телепаты в отпуске, чтобы сразу заложить все возможные варианты.

Насчет триггеров, функций и процедур: я давно уже поднимал вопрос об использовании этого функционала, но все осталось в ядре.

Простой пример: удаление персонажа из базы.
Если правильно настроить триггеры, то все записи будут автоматически удалены из связанных таблиц, не надо будет в ядре все это прописывать.
Если ядро загнется в процессе удаления, то не факт, что целостность данных останется в норме.

[KiriX]

Сообщение от virusav

KiriX, в данном случае нельзя сделать лог с исправлениями, т.к. оптионов нет, а телепаты в отпуске, чтобы сразу заложить все возможные варианты.

Насчет триггеров, функций и процедур: я давно уже поднимал вопрос об использовании этого функционала, но все осталось в ядре.

Простой пример: удаление персонажа из базы.
Если правильно настроить триггеры, то все записи будут автоматически удалены из связанных таблиц, не надо будет в ядре все это прописывать.
Если ядро загнется в процессе удаления, то не факт, что целостность данных останется в норме.

1) Я и не предполагаю, что исправить можно все возможные ошибки...
2) На счёт триггеров что-то как-то только говорят все (я не исключение ), а фактически не делается. А было бы полезно. Даже базоделателям можно много чего полезного из это извлечь.
3) Именно этот простой пример я и привёл в той теме, где шму про возможности базы заговорил

[virusav]

Кто-то говорит, а я во всю использую триггеры в своей работе.
Для мангоса пока не писал, т.к. при тесте патчей или работе над базой часто приходится накатывать базу с 0.

Если будет необходимость, напишу, что потребуется.

[KiriX]

Сообщение от virusav

Кто-то говорит, а я во всю использую триггеры в своей работе.
Для мангоса пока не писал, т.к. при тесте патчей или работе над базой часто приходится накатывать базу с 0.

Если будет необходимость, напишу, что потребуется.

На работе и у меня в оракловой базе и триггеры, и хранимые процедуры, и вторичные ключи используются. Я как раз о мангосе и говорил, что тут только разговоры.

[virusav]

Для этого нужно создать отдельное обсуждение, а не флудить в топике патча.

[RomanRom2]

Сообщение от virusav

Простой пример: удаление персонажа из базы.

для этого необязательно использовать триггеры.

[schmoozerd]

To throw in an alternative:

For checks like the ones above we can use _only_ a sql-script with some SELECT output.
Then there would no fancy output (that flag 2 == NPC_FLAG_QUESTGIVER), but the integrity check could be done as well.

And it should be enough, as this is only really required to be used by DB-developers, not normal routine

As example for this case here:

diff --git a/contrib/dbIntegrityChecks/001_checkGossipOptionFlags.sql b/contrib/dbIntegrityChecks/001_checkGossipOptionFlags.sql
new file mode 100644
index 0000000..031281d
--- /dev/null
+++ b/contrib/dbIntegrityChecks/001_checkGossipOptionFlags.sql
@@ -0,0 +1,16 @@
+-- This is a helper script to select gossip menu's that don't provide all gossip-options that might be required by npc-flags.
+
+SET @GOSSIP_SELECT_FLAGS=0x00000010/*TRAINER*/ | 0x00000080/*VENDOR*/ | 0x00002000/*FLIGHTMASTER*/ | 0x00010000/*INNKEEPER*/ | 0x00020000/*BANKER*/ | 0x00040000/*PETITIONER*/ | 0x00080000/*TABARDDESIGNER*/ | 0x00100000/*BATTLEMASTER*/ | 0x00200000/*AUCTIONEER*/ | 0x00400000/*STABLEMASTER*/ | 0x00800000/*GUILD_BANKER*/;
+
+SELECT ct.entry, ct.gossip_menu_id,
+       (ct.npcflag & @GOSSIP_SELECT_FLAGS) &~ BIT_OR(gmo.npc_option_npcflag) AS `flags`
+FROM creature_template AS ct, gossip_menu_option AS gmo
+WHERE
+ ct.gossip_menu_id > 0
+ AND ct.ScriptName=''
+ AND gmo.menu_id=ct.gossip_menu_id
+GROUP BY
+ ct.entry, ct.gossip_menu_id
+HAVING
+ `flags`>0;
+