Skip to content
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

[suggest] Add command for GM to learn all the available skills at current level #6339

Closed
wants to merge 3 commits into from

Conversation

Bogir
Copy link
Contributor

@Bogir Bogir commented Jun 13, 2021

Changes Proposed:

  • learn all the available skills that can be learned throw ClassMaster at current level

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.

@Kitzunu
Copy link
Member

Kitzunu commented Jun 13, 2021

Probably better to do after #6109

@FrancescoBorzi
Copy link
Contributor

build fails :(

@Kitzunu
Copy link
Member

Kitzunu commented Jun 13, 2021

/home/runner/work/azerothcore-wotlk/azerothcore-wotlk/src/server/scripts/Commands/cs_learn.cpp: In static member function ‘static void learn_commandscript::learn(Player*, std::unordered_map<unsigned int, CreatureTemplate>::const_iterator, CreatureTemplate)’:
/home/runner/work/azerothcore-wotlk/azerothcore-wotlk/src/server/scripts/Commands/cs_learn.cpp:137:81: error: unused parameter ‘itr’ [-Werror=unused-parameter]
  137 |     static void learn(Player* player, CreatureTemplateContainer::const_iterator itr, CreatureTemplate cInfo)
      |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
compilation terminated due to -Wfatal-errors.

@Bogir
Copy link
Contributor Author

Bogir commented Jun 13, 2021

ok, rebuilding from every begining .... Let's see..

@Kitzunu
Copy link
Member

Kitzunu commented Jun 13, 2021

It wont show on MSVC because it's a forgiving compiler, but you can see the CI's that look for more stuff

@Krejza9
Copy link
Contributor

Krejza9 commented Jun 14, 2021

@Bogir Install Linux and compile it (and Stop using Windows for server apps :-D)

@Bogir
Copy link
Contributor Author

Bogir commented Jun 14, 2021

@Bogir Install Linux and compile it (and Stop using Windows for server apps :-D)

I stopped using *nix in 1992 and I will never come back to it.
If your compiller can't build core - this does not prevent us from working under Win :D
You can fix it to work even on Mars if is it necessary for your system or for your ambitions.
This patch only suggestion and can be not merged, but can be used in private case

@Krejza9
Copy link
Contributor

Krejza9 commented Jun 14, 2021

If you program something for AC , You have to program it so that it can be compiled on Linux, Windows, MacOS.
If you don't care if it doesn't work on other systems.
Then ask someone to fix it, or cancel the PR

@Bogir
Copy link
Contributor Author

Bogir commented Jun 14, 2021

done
Thanks to @Krejza9, @Winfidonarleyan and @FrancescoBorzi for help

@@ -30,6 +30,7 @@ class learn_commandscript : public CommandScript, public PlayerCommand
{
static std::vector<ChatCommand> learnAllMyCommandTable =
{
{ "lvl", SEC_GAMEMASTER, false, &HandleLearnAllMyLvlCommand, "" },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alphabetical order

@@ -105,6 +106,70 @@ class learn_commandscript : public CommandScript, public PlayerCommand
return true;
}

static bool HandleLearnAllMyLvlCommand(ChatHandler* handler, char const* /*args*/)
Copy link
Member

@Yehonal Yehonal Jun 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be better to move the logic of this function in a separate file in order to let people reuse it for other purposes (custom scripts etc).

In this way, modders could create scripts that automatically teach you spells, for instance, when you level up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree with handling commands in other files than the designated ones

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I said "the logic", which means creating a new function elsewhere and using that one in HandleLearnAllMyLvlCommand

that function can be reused then in other scripts/modules/logics

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I oversaw that :D

@ghost ghost removed Script labels Oct 5, 2021
@Azcobu Azcobu added Script WIP Work in Progress. CORE Related to the core labels Oct 6, 2021
@FrancescoBorzi FrancescoBorzi added the PR Abandoned Original author stopped working on this PR. Feel free to take over. label Oct 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CORE Related to the core PR Abandoned Original author stopped working on this PR. Feel free to take over. Script WIP Work in Progress.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants