All spells buyable from class trainer

Confirmed on WoTLK (M2).
* Need to check if the issue exists on Vanilla (M0) and TBC (M1).

Working as intended on Cataclysm (M3) from the initial PR from Vanilla (M0).

From thinking on this, another issue could be at play here outside of the line of code reported in the post above.

Code originally changed from

bool prof = SpellMgr::IsProfessionSpell(trainer_spell->learnedSpell);

    // check level requirement
    if (!prof || GetSession()->GetSecurity() < AccountTypes(sWorld.getConfig(CONFIG_UINT32_TRADE_SKILL_GMIGNORE_LEVEL)))
    {
        if (getLevel() < reqLevel)
        {
            return TRAINER_SPELL_RED;
        }
    }

on lines 4748-4757 in v22.02.65 to this

bool prof = SpellMgr::IsProfessionSpell(trainer_spell->spell);
    if (prof || trainer_spell->reqLevel && (trainer_spell->reqLevel) < reqLevel)
    {
        return TRAINER_SPELL_RED;
    }

on lines 4763-4767 in v22.02.67

I thought it could have happened accidentally that the function getLevel() was not taken over bc it worked before and now it's discarded.

Made not much sense imho but I am no software developer so I haven't been able to figure out more about what's behind the dynamic properties yet. Just looked for the explicit difference between these two versions for now.

Happy to help!

I see similar issue on TBC too.

Looking at the commit that changed this code https://github.com/mangosone/server/commit/360ddbdf862780c35664d44e15f9de9a3be13dab#diff-60492af07b2ca76bed925294d27c48cab25f8db3a33423e3db010c3d800e8b68L4460

I guess it should be like below. But I am not very familiar with the objects either.

    bool prof = SpellMgr::IsProfessionSpell(trainer_spell->spell);
	// if we use || here all profession skills are "red"
    if (prof && trainer_spell->reqLevel && (trainer_spell->reqLevel) < reqLevel)
    {
        return TRAINER_SPELL_RED;
    }

    // check skill requirement
    // I am not sure, but the previous version checks !prof. Similarly, if we have "prof || xxxx" here, all profession skills are red
    if (!prof || trainer_spell->reqSkill && GetBaseSkillValue(trainer_spell->reqSkill) < trainer_spell->reqSkillValue)
    {
        return TRAINER_SPELL_RED;
    }

@Fyre, sorry for tagging you here. But looks like you are the author of that commit. can you take a look?

@D Q There are two authors to this commit, Fyre and I. Just never got around to fixing the issue yet and just being busy outside of the project.
Feel free to make a PR with the fix and will get it pushed out.

1 hour ago, Meltie said:

@D Q There are two authors to this commit, Fyre and I. Just never got around to fixing the issue yet and just being busy outside of the project.
Feel free to make a PR with the fix and will get it pushed out.

@Meltie Thanks for the quick response... I would love to contribute. But TBH, I am not very certain I understand the intention of that piece of the code. Only spotted the obvious "prof" part. Might create a PR and discuss further.

@D Q I have pushed a PR out on the original repository where the fix was initial committed too.