-
Notifications
You must be signed in to change notification settings - Fork 493
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement Book Soothsaying for dummies + Profession specialisation updates and fixes #1705
base: development
Are you sure you want to change the base?
Conversation
missing comma
comma again
Editing broadcast_text is not allowed. That table is bruteforced from classic. The text fields are 100% correct. |
And the male and female texts are per the gender of the NPC which has the gossip, not the player who talks to them. |
Ah sorry for the confusion on my side, I'll fix it right away |
Remove changes to `broadcast_text`, and add text to NPCs appropriately.
Not sure what I'm trying to do, but I hope it works
I always forget doing the commas
Not a fan of the custom npc_text and gossip_menu ids. |
I'm not, either, but I don't have sniffs for them. However, in my view it's better to have a core system to professions implemented regardless of whether the backend is 100% accurate aesthetically (I have provided ample proof for correct usage for every custom id I've used). |
After a quick check I seem to have erroneously added some of the insertions into npc_text. The missing IDs of the missing entries are very easily known since their neighbouring entries are sorted numerically by broadcast text id. The only thing missing is the gossip menu id for any gossip sub menus |
Removed custom npc_text ids
Relearning via gossip only added in 1.10
🍰 Pullrequest
This pull request implements the intended ways to change profession specialisations, as well as generally reworking profession handling to use cmangos implementation.
Server hosts be warned: merging this PR will cause respecialisation to be broken on old characters
The blizz-like implementation uses hidden "specialisation reputation" (details below) that would not have been rewarded to characters that have previously completed specialisation quests. Once this PR is ready for merge there will be another PR linked here that uses quest completion instead of the reputation to prevent any bugs.
Changes
Remove QUEST STATUS NONE update for abandoned professions
Fixes 🐛 [Bug] Profession quests are able to be completed multiple times when abandoning and retraining a trade skill #1681
Allows for implementation of Book Soothsaying for dummies
Implement Book Soothsaying for dummies for Engineering and Leatherworking specialisations
Added patch 1.10, before this there was no way to switch specialisations for Engineering and Leatherworking.
Video from original WotLK
Remove profession specialisation ranks in spell_chain
Implied by SkillLineAbility.dbc: There's no previous rank for profession specialisations. They are not part of the profession spell chain.
Fixes 🐛 [Bug] Completing quest Tribal Leatherworking [5148] teaches you Artisan Leatherworking even though it shouldn't #1686
Fixes 🐛 [Bug] When abandoning and retraining a profession sometimes developmental skills are incorrectly retained #1683
Forum post:
Implement vanilla mechanics and scripts for Blacksmithing specialisation quests
Blacksmithing quests are unlocked by a gossip option by either Myolor Sunderfury (Alliance) or Krathok Moltenfist (Horde) that gives the player reputation for the hidden "Armorsmithing" or "Weaponsmithing" factions. There is a small gossip script that fires where the respective faction's "guide" points at the relevant NPC that starts the quest for the specialisation you chose and says a short line of text.
Armorsmithing/Weaponsmithing gossip script:
Example for Horde Weaponsmith: Video
Example for Horde Armorsmith: Video
Example for Alliance Armorsmith: Video
For Alliance there is additional gossip given by the relevant Blacksmith trainers in Ironforge and Stormwind that guides you toward Myolor to choose a specialisation:
Example for Bengus Deepforge: Video
Therum Deepforge: Relevant Wiki article
This gossip remains available as long as you have the required 200 blacksmithing (and level 40+) but do not have a blacksmithing specialisation.
Bengus Deepforge gossip
The wiki article for Therum Deepforge also reveals missing gossip. From looking at the PTR I have concluded this gossip was bugged during vanilla due to being incorrectly displayed at a Blackmsmithing skill of 1:
This gossip would be given before Therum Deepforge can train you. However, currently on the Classic PTR when you have trained blacksmithing but your skill is under 50 he says the following:
This gossip does not exist in
broadcast_text
, meaning it's a later addition to fix a bug relating to this NPCs gossip, which could only have been due to using this second gossip when he shouldn't have.Although it's worth mentioning that the relevant Wiki article lists the gossip under "Trained" which would imply this behaviour, without other circumstances this would not constitute any sort of proof.
No vanilla system for changing Blacksmithing specialisations
An announcement on new features for TBC all but outright confirms that specialisation change was impossible in vanilla.
Profession specialisation 'dummy' factions
Different profession specialisation quests reward reputations with hidden factions with the corresponding name (e.g. Completing the quest Tribal Leatherworking rewards reputation with a faction called Leatherworking - Tribal). This useful for cutting down on the amount of conditions required for the implementation (no need to seperately check horde and alliance quests).
Pre-1.10 profession specialisation mechanics
Before Patch 1.10, on abandoning and retraining a profession which you specialised in, LearnQuestRewardedSpells would reteach whichever specialisation you had immediately on training the profession.
All relearning methods via gossip (Leatherworking & Engineering via Soothsaying and Blacksmithing & Weaponsmithing sub specialisations via their trainers) were added in 1.10 with the change to LearnQuestRewardedSpells to no longer reteach specialisations immediately on retraining.
Forum post:
This PR does not implement the bugged behaviour mentioned above on being unable to learn specialty schematics (mainly as I lack the knowledge on how to implement it). Feel free to add it in the future...
Deprecate field
trainer_spell
in creature templateRemove remaining uses of
trainer_spell
(almost exclusively incorrectly in use for profession specialisations) and remove the field fromcreature_template
.How2Test
Todo / Checklist
trainer_spell
field in creature_template