Trinitycore: DB/Creature: Set correct flags on Shatterspear Laborers

[shdr212]

Description:
Shatterspear Laborers in Ruins of Mathystra in Darkshore look as if they're dead on the floor (feint death),
and can't be attacked by players. They are slain objectives for a quest ( #25801 ).

Wowhead npc link: https://www.wowhead.com/quest=13504/shatterspear-laborers
Video on a quest related to npcs, where you can see how the npcs should work: https://www.youtube.com/watch?v=87sVpn6tClM&feature=youtu.be

Expected behaviour:
Npcs should be alive and attackable by players.

Steps to reproduce the problem:

1. go to Ruins of Mathystra in Darkshore
2. Target the Shatterspear Laborers npc

Suggested solution:

UPDATE `creature_template` SET `unit_flags`=0, `unit_flags2`=0, `unit_flags3`=0 WHERE `entry`=32861;
UPDATE `creature` SET `unit_flags`=0 WHERE `guid`=260181 OR `guid`=260182 OR `guid`=260188 or `guid`=260189 OR `guid`=260190;

Branch(es):
master

TC rev. hash/commit:
51c126c07aa6d7044bec01183602930c3596249e

TrinityCore rev. 51c126c07aa6 2021-01-11 21:04:58 +0100 (master branch) (Win64, Debug, Static)
Using SSL version: OpenSSL 1.0.2n  7 Dec 2017 (library: OpenSSL 1.0.2h  3 May 2016)
Using Boost version: 1.72.0
Using MySQL version: 5.7.32
Using CMake version: 3.19.2
Compiled on: Windows 10.0.19041
Automatic database updates are enabled for the following databases: Auth, Characters, World, Hotfixes
Worldserver listening connections on port 8085
Realmlist (Realm Id: 1) configured in port 8085
VMAPs status: Enabled. LineOfSight: 1, getHeight: 1, indoorCheck: 1
MMAPs status: Disabled

Operating system: Windows 10

[offl]

Please post your solution in #25801

[shdr212]

@offl I had this discussion in IRC, on whether to open a new issue or simply comment in the old one, and I understood it's better to create a new issue.

Perhaps a clear guideline in the wiki about it would be helpful.
Specifically here: https://trinitycore.atlassian.net/wiki/spaces/tc/pages/2130102/SQL+Guidelines

[offl]

Usually people post solutions in old issues. In some cases it's correct, in some not, it depends on multiple things and I doubt we can explain all of them in a guide
Seems you're right, this one looks better

[Aokromes]

updated wiki to state fixes for known bugs must be posted on existing bug reports.

[shamage]

@shdr212 - thank you for your great work , NPCs behave as they should (after implementing your fix)

[illfated]

@shdr212 : You can use ```sql at the start of your SQL code block if you want to show the syntax highlighting.

[shdr212]

@shamage happy to hear that!

@illfated cool edited; thank you!

[matanshukry]

I believe this was merged in #25801 and can be closed?

[offl]

I think the guy who wrote https://github.com/TrinityCore/TrinityCore/issues/25801#issuecomment-761105215 just applied sql locally

[matanshukry]

@offl you didn't merge it? I assumed so since there wasn't any comment indicating a fix request, and you closed the issue.

[illfated]

@matanshukry : If the SQL had been merged, you would have seen a commit reference in this page. Also, Aokromes took your word for it to close this ticket, without checking sources.

edit: Also, the closing of issue ticket #25801 was unfortunately done without writing a closing comment, like "Duplicate of #..."

[matanshukry]

@illfated that's kind of weird, closing the earlier issue (4 Jan) as a duplicate of the newer issue (15 Jan).
Also, last comment from a tc member in this issue was by Aokromes, which states the fix should be posted at the other issue, which it was - and then closed. Which again, I just assumed it was merged.

Anyway in light of that I'm assuming now it wasn't merge and one of those issues should be reopened.
Although we have the fix and I haven't seen any review comment about it - is there anything wrong with it not being merged yet?

[illfated]

Thank you for sharing your thoughts on the issues. Fair point. Normally, the oldest ticket should stay open while the newer one(s) should be closed. One of the possible exceptions is when the newer ticket contains a better explanation or has more useful content like both suggested solutions and confirmations that the suggested solution works.

Regarding when a suggested solution can be merged, that is more up to the TC members to decide. It could be down to the amount of testing (or lack thereof), but it is usually better to get feedback from TC members why the suggested solution has not made it into the commits yet.