Trinitycore: Core/Entities: Silithyst can't be obtained from Silithys Geyser in PvP

[durotar]

TrinityCore rev. 5df0bcdd4e2d (clean installation)
http://www.wowhead.com/quest=9419/scouring-the-desert (Alliance side - prev quest 9415)
http://www.wowhead.com/quest=9422/scouring-the-desert (Horde side - prev quest 9416)
Both quests can't be completed:
Silithyst can't be obtained from Silithys Geyser

• Note:

Tested with a normal account / player <-- quest doesn't work

Tested with a GM account / player <-- quest do work

Tested with a GM account changed to normal account <-- quest do work

[Killyana]

Cannot reproduce, tested with GM account and then player account and it always work as intended.

[Killyana]

Seems not related to GM or player status, in the same account I have a Tauren and a Night elf, I can get the Silithyst with the Elf but not with Tauren, both are level 80 and the quest not taken or done.

Also not related to the race or class, I created a new Tauren shaman and I was able to get the Silithyst.

[Killyana]

I found the problem, when you create a new character his name will be blue also when you teleporte to a capital your name will turn blue, that means the pvp is disabled, in this case you can get the Silithyst, but if you turn on/off the pvp mode your name will turn green and you will not be able to get the Silithyst, if I'm not wrong this must be the opposite if the pvp is on you can get the Silithyst and if it's off you cannot.

[ghost]

Yes, the names are blue. Both my test characters, lvl 75 dwarf hunter and lvl 79 orc rogue, were able to pick up the Silithyst, get the buff and deliver it in their faction camps. I made 1 delivery to complete the quest and another to test normal delivery without the quest. I'll retest with PvP ON and update.

e266278f7d02 2015-11-27 22:10:21 +0100 / TDB 335.60 + updates including 2015_11_27_00_world.sql
Confirmed. If PvP flag is on, the Silithyst object despawns, but no player buff and nothing to deliver.

• http://www.wowhead.com/object=181598/silithyst-geyser
• http://www.wowhead.com/object=181597/silithyst-mound
• http://www.wowhead.com/spell=29519/silithyst (player buff while returning to base)
• http://www.wowhead.com/spell=29534/traces-of-silithyst (player buff after delivery)

/src/server/scripts/OutdoorPvP/OutdoorPvPSI.cpp
/src/server/scripts/OutdoorPvP/OutdoorPvPSI.h

[Kittnz]

i think this is related to #13103

[durotar]

Then, maybe other PvP zones also have problems

[Killyana]

No, the issue here is the condition is reverted, it must be if the pvp is on you can get the Silithyst and if it's off you cannot, not the opposite.

Probably the problem is here: https://github.com/TrinityCore/TrinityCore/blob/8f3a9b7fe338456dff08e1fccdb2b672d9d19299/src/server/game/Entities/GameObject/GameObject.cpp#L1761

[Thomazeto]

still bugged, the object despawn, but the player only gets the buff if he's non-flagged for PvP.

[ghost]

TrinityCore rev. hash, please?

[Thomazeto]

rev: 58dad50

[funar]

TrinityCore rev. c76931c2c095 2019-03-17 16:57:05 +0100 (3.3.5 branch) (Unix, RelWithDebInfo, Static)

Oddly enough, this is still an issue. Playing on a PVP realm. The geysers still disappear, but nothing happens.

[illfated]

Maybe the aura check logic is the opposite of what it needs to be to work in PvP mode?

[Wyrserth]

This fixes the issue but I'm not sure if it is a valid fix. Returning true prevents execution of OutdoorPvPSI's version of HandleCustomSpell().

There's also a smaller issue, when right-clicking on the geyser (not the mound) with PvP disabled, the player doesn't get the spell but the gameobject still despawns. I'm not sure what error message should be shown in that case, but I'm sure the gobj shouldn't despawn (because in fact, nothing happens because the player doesn't have PvP enabled).

[illfated]

(just copying the diff in here to make it even more visible)

 bool OPvPCapturePoint::HandleCustomSpell(Player* player, uint32 /*spellId*/, GameObject* /*go*/)
 {
     if (!player->IsOutdoorPvPActive())
-        return false;
-    return true;
+        return true;
+    return false;
 }

 bool OutdoorPvP::HandleOpenGo(Player* player, GameObject* go)

Well, it definitely looks like you swapped the logic around, which makes sense, because it is supposed to be a PvP-only player event and not be available unless player is flagged for PvP. Whether it is considered a hack or not, I don't know (sorry).

[wywgk]

TrinityCore rev. c76931c 2019-03-17 16:57:05 +0100 (3.3.5 branch) (Unix, RelWithDebInfo, Static)

Oddly enough, this is still an issue. Playing on a PVP realm. The geysers still disappear, but nothing happens.

TrinityCore rev. a054ce864d29 2019-08-16 06:27:38 +0000 (3.3.5 branch) (Unix, RelWithDebInfo, Static)
enable pvp :nothing
disable pvp:get the buff

[illfated]

That makes me think @Wyrserth's suggested change should be valid, but knowing that the side effect is that it prevents the execution of OutdoorPvPSI's version of HandleCustomSpell(), more changes are needed as well (like making the gameobject not despawn or non-interactive when PvP is disabled). I don't have coding skills to solve that problem, sorry.

[illfated]

As a side note, I would like to mention that during the last few months, while playing on tcubuntu.northeurope.cloudapp.azure.com, my level 58-64 orc hunter character has always logged in with PvP status enabled in the icon, although the selection status from the game client GUI says it is disabled. While in this state, visibly flagged as PvP (but the UI selector saying disabled), I am able to capture PvP flags in Hellfire Peninsula as well as other maps.

---

edit: To be able to remove the PvP status and become non-PvP again, I need to teleport to a different map or zone, because it is not enough to use the GUI selector or /pvp command-line switcher.

[Rushor]

fix for both

diff --git a/src/server/game/OutdoorPvP/OutdoorPvP.cpp b/src/server/game/OutdoorPvP/OutdoorPvP.cpp
index 96a61cdc5a..2fe4e7c0f1 100644
--- a/src/server/game/OutdoorPvP/OutdoorPvP.cpp
+++ b/src/server/game/OutdoorPvP/OutdoorPvP.cpp
@@ -495,7 +495,7 @@ bool OPvPCapturePoint::HandleCustomSpell(Player* player, uint32 /*spellId*/, Gam
 {
     if (!player->IsOutdoorPvPActive())
         return false;
-    return true;
+    return false;
 }

 bool OutdoorPvP::HandleOpenGo(Player* player, GameObject* go)

[illfated]

Hmm, interesting. I did not consider that option, but then again, I am not a real coder.

[illfated]

From https://github.com/TrinityCore/TrinityCore/pull/26240#issuecomment-798891397 by Shauren:

This is a wrong fix. Real issue is that all OutdoorPvP hooks are called for all types - so this breaks because HandleCustomSpell is called first on OutdoorPvPHP (hellfire zone), it returns true so HandleCustomSpell of OutdoorPvPSI is not called.

The fix should be to only call HandleCustomSpell on OutdoorPvP object for current zone, not to break OutdoorPvP even further by changing return value

Anyone want to take a closer look at that suggestion, to see if this issue can be solved?

[offl]

Considering all PvP files are outdated and usually almost everything in them is wrong and unnecessary, handling of object should be moved out of OutdoorPvP if possible and if will work as intended(without honor farm bugs) after moving to spell scripts SAI whatever
Didn't really checked but I don't understand why object is handled in HandleCustomSpell in first place
Ideally OutdoorPvPSI needs more love in general