azerothcore-wotlk
azerothcore-wotlk copied to clipboard
azerothcore
fix(Scripts/Gossip): Trial of the crusader
Changes Proposed:
- Select the gossip and npc text from the database, instead of always displaying the same gossip and hardcore texts.
- Write the npc script to look like the blizzlike content.
Before
After
Issues Addressed:
- Closes
SOURCE:
Tests Performed:
- Windows 10
- In-game
How to Test the Changes:
-
.go c 67185 - Set the raid in normal 10 mode
- Perform the band normally, even dying, to verify that the npc shows the gossip again when re-entering.
Topics to test
- [ ] In each fight, a different gossip and text should be shown on the npc.
- [ ] The npc that triggers the event remains inactive until the previous event ends.
- [ ] The door closes at the start of the event (I have to reverse this, now I really like it much better and it makes sense this way, but it doesn't seem to work this way).
- [ ] The last fight could be invoked in 2 ways. Waiting on the platform for 60 seconds, or through the gossip. According to the video, the fight is only activated by the gossip, so the automatic start after 60 seconds is disabled.
- [ ] Upon death, please check that everything is back to normal.
- [ ] The translation of a final text of Tirion was missing, in fact, it was missing to relate it, because the translation already exists.
- [ ] The second boss, had a hardcore text, although I have to keep working on it, because as it has 2 texts, I have to randomly select one of them.
Known Issues and TODO List:
- [x] Restructure the script, because although it works, it could be written in a different way, so as not to repeat so much code.
- [x] Move the IDS, both from the Gossip Menu and the Npc text, inside an enum.
- [x] Some npc text is also missing to be added to the gossip_menu. Actually, to link them. Although it works without that, the information must be in the database.
- [x] When the gossip is selected, the door should close to prevent the player from leaving the instance and the event from running normally. Currently, in some instances, it allows the player to exit even when the event has started.
- [x] The gossip flag should be available when the event ends in its entirety. To avoid that the player can start an event without finishing the previous one.
How to Test AzerothCore PRs
When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].
You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:
http://www.azerothcore.org/wiki/How-to-test-a-PR
REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).
For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.
There are still some things to do, but at least the script is functional. I have tested it with a Spanish client, and it would be nice if other people could test it with other clients. And I have to improve, the things that I mention inside the TODO List.
I'm going to do a couple of commits, or things, that may be "extras" for some, but I want to use the pull request, to have some sort of "enhancements" to the commits. That show how the code is evolving, starting for example, with a series of if, and then, ending with a case. And that way, people who want to see the example and then replicate it (if they don't know how to do it), have an example to see.
The last event runs by itself after a while, that's why the NPC, at the end of the fourth battle, does not show in gossip. However, if the player exits and re-enters the instance, surely what happens is that the event is cut, and that is why it allows him to execute the gossip to call the event.
Thanks for you PR. It's tested and work fine :)
-[X] In each fight, a different gossip and text should be shown on the npc. -[X] The npc that triggers the event remains inactive until the previous event ends. -[X] Upon death, everything is back to normal.
No new issues find.
This PR, besides changing the gossip and some other things, also changes the way the fight is done. Because as I mentioned in a moment, previously, the player could execute the event and go out the door. Now, when starting the event, he is trapped inside, having to do the same, die, or disconnect. I don't understand why someone would start an event and leave the place, but it really seemed absurd and I wanted to change it. I don't know if that kind of change, it's something blizzlike, it's something more common sense.
This code, I don't know if it's really worth to merge it, beyond the gossip and the npc text. Because I make several changes in the logic of the fight, but reviewing videos, then I saw that these changes are not seen in other videos. It was more a matter of common sense. That the player, could not start the event, and leave the raid. But well, I leave it up to you, for me, it would be good to add it this way, but it will depend on you, if you want to do it or not.
This pull request, I would close it. Because I made changes, in the logic, that I don't know if they correspond. The texts, if they would have to be added and those things, but I'll make another pull request with that, but the rest of the things, I think, affect the "mechanics". I really like it better this way, making that the player can't leave, if he triggers the event, but I don't know if it's correct.
This pull request, I would close it. Because I made changes, in the logic, that I don't know if they correspond. The texts, if they would have to be added and those things, but I'll make another pull request with that, but the rest of the things, I think, affect the "mechanics". I really like it better this way, making that the player can't leave, if he triggers the event, but I don't know if it's correct.
@Nefertumm @Nyeriah if someone get the time to review this before to merge in the coming days :)
@Nefertumm @Nyeriah if someone get the time to review this before to merge in the coming days :)
I really don't know if it would be good to merge it, because it "changes" the logic a little bit. As I mentioned before, this makes the event, leaving the player locked, which seemed fine to me, but I don't know if it is the usual mechanics.
Even though this is already tested quit some time ago, i would feel confident of a re-test to ensure there is no issues. The author has some concerns and i really do not want to invalidate this PR.
The concerns I have, are with respect to the mechanics, although the code clearly could be improved. But so much time has passed since I made it, that in the worst case, we could close it and do it later as a last resort. The translations part was interesting, but then, the rest of the mechanics implies that once the event is triggered, the player or players involved will not be able to leave the event unless they die or successfully complete the event. It is more than anything a mechanism to prevent a player from talking to the npc and leaving through the door, preventing the event from being triggered unnecessarily.
Then also, incorrect ways of working in the document were found, but they were reused, based on the code that already existed. For example, the execution of the sound, from the code, instead of being called from the database. The problem is that if all the creature_text / broadcast_text, are correct, there is something that I do not quite understand. The second Boss, in one of its dialogues, uses 2 sentences, in a random way, while in the emulator, there is only one of them.
@pangolp
- create 2x account rank 0
- create 2x hero horde
- you can't click on npc
