Core/Quest: Quests related to events must be removed when the event ends

[kuskmen]

Changes proposed:

• Removes all quests related to not active events from player's quest log.
  Target branch(es): 3.3.5/master
• 3.3.5
• master

Issues addressed: Closes #23123

Tests performed:
.event start 1
.quest add 11955
.event stop 1
relog twice

Known issues and TODO list: (add/remove lines as needed)

• PlayerScript::OnLogin fires query that removes the quest for a character and only after second login quest is removed.
• Kinda worried about performance issues, I mean it seems to me that this script might turn out to be overkill, open for suggestions.

[ccrs]

I'd just integrate this into quest load/save system
and add a logic related to event ending -> remove current online players quests

[kuskmen]

I'd just integrate this into quest load/save system
and add a logic related to event ending -> remove current online players quests

Can you elaborate more, I am still not at the point where I can understand without giving more specific instructions :/

Where is this quest load/save system and how can I iterate over all online players?

[Kittnz]

This seems like a custom thing, i can't remember event quest getting removed when the event ended.

[Jildor]

I don't know if this is blizz or not, but, onLogin seems bad idea for performance.
Maybe do a cleanup on startup server if a conf option is enable?

[jackpoz]

onLogin seems bad idea for performance.

well it depends on what code is being run and what impact it has

[ghost]

This seems like a custom thing, i can't remember event quest getting removed when the event ended.

Fair point, neither can I (or is this a Mandela Effect, that we remember this incorrectly?).

[Killyana]

When an event starts quests related to this event are reset, and the player can do this quests again.
So if he keep a quest from a chain it doesn't make cense to reward this quest then take the qeust that starts this chain.

[ghost]

When an event starts quests related to this event are reset, and the player can do this quests again.
So if he keep a quest from a chain it doesn't make sense to reward this quest then take the quest that starts this chain.

Fair point, I didn't notice that the outcome would be that the event quest would be rewarded when removed.

[jackpoz]

maybe a flag in the db would be better

[jackpoz]

if we choose to keep the OnLogin() PlayerScript (note that each PlayerScript has a lot of hooks and even if empty, they add up), I would like you to profile a bit this code and find the most performant way to achieve the same result.
Maybe iterating the active player quests first, then check the game event mgr ? or maybe that's slow ? I have no clue.

[jackpoz]

any follow up on this ?

[ghost]

@kuskmen : I hope you will be able to look into this PR of yours, now that there are some holidays ahead.

[kuskmen]

@illfated hehe quite some holidays ahead I admit, I am gonna have to re-read all the comments and see what this stage was left off. If you guys say we still need this I can do it asap.

[msoky]

@kuskmen could you check if there is sometnig needed to finish it?

[kuskmen]

I think I read somewhere that this is not blizzlike behaviour so I thought it should be canceled? Am I wrong?

[ghost]

I am thinking maybe it would be better to move it first to the https://github.com/TrinityCore/TrinityCoreCustomChanges repository, let users play with it there for a while, then get it back here on 3.3.5 when it gets some more feedback after actual usage. What do you others think, is that a fair suggestion?

[jackpoz]

Closing this PR as it seems abandoned. Feel free to ask here to reopen it if anyone would like to work on it.