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

Fix crashes and make options work #21

Merged
merged 8 commits into from
Mar 27, 2024
Merged

Conversation

anzz1
Copy link
Contributor

@anzz1 anzz1 commented Jan 5, 2022

  • Setting option Buff.MessageTimer = 0 actually disables the timed emotes and phrases now
  • Fix crash on setting Buff.NumWhispers = 0 (disable whispers)
  • Fix crash on setting Buff.NumPhrases = 0 (disable random talk)

anzz1 added 2 commits January 5, 2022 22:53
* Setting option Buff.MessageTimer = 0 actually disables the timed emotes and phrases now
* Fix crash on setting Buff.NumWhispers = 0 (disable whispers)
* Fix crash on setting Buff.NumPhrases = 0 (disable random talk)
@Helias
Copy link
Member

Helias commented Jan 6, 2022

There is a sql issue, InhabitType does not exist.
@acidmanifesto may you can easily fix it?

@acidmanifesto
Copy link

acidmanifesto commented Jan 6, 2022

There is a sql issue, InhabitType does not exist.
@acidmanifesto may you can easily fix it?

My net is out. But the fix is to remove the inhabit collumn in the query. So remove inhabittype and the 3 in the query

As per commit azerothcore-wotlk@2d4e17f InhabitType was removed from creature_template table
@anzz1
Copy link
Contributor Author

anzz1 commented Jan 6, 2022

pushed a change to remove InhabitType

@acidmanifesto
Copy link

pushed a change to remove InhabitType

Perfect

anzz1 added 2 commits January 10, 2022 13:44
* Replaced deprecated config api to fix build
* add enable/disable option
Add enable/disable option
@anzz1
Copy link
Contributor Author

anzz1 commented Jan 10, 2022

pushed another change to replace deprecated config api with new GetOption api to fix build

@Helias
Copy link
Member

Helias commented Mar 26, 2022

thanks for your PR, unfortunately there are some merge conflicts

@pangolp
Copy link
Contributor

pangolp commented Mar 27, 2024

@anzz1 Greetings. You have to enable the possibility that we can make commits within the branch, or on the contrary, we will not be able to update it, so that it can be merged. In any case, I will have to create another pull request, and give you the credits within it. But if we can add changes to this same branch, the better, so we finish working on it, and we can merge it.

@anzz1
Copy link
Contributor Author

anzz1 commented Mar 27, 2024

@pangolp
image
This box "Allow edits by maintainers" is checked, and I think it's enabled by default?

In any case do whatever you want with this, copy-paste to another PR is completely fine by me if you're having problem updating this one, I'm only here to help and I don't care about credits and such.

@pangolp
Copy link
Contributor

pangolp commented Mar 27, 2024

This box "Allow edits by maintainers" is checked, and I think it's enabled by default?

In any case do whatever you want with this, copy-paste to another PR is completely fine by me if you're having problem updating this one, I'm only here to help and I don't care about credits and such.

I downloaded your branch this morning, I tried to push it and it gave me an error. But if you say it is possible to do it, I will try again. I don't need permissions on the repository, if not, on its restroom, at least on the patch-1 branch, which is where this change is located. I need to push a commit, so I add some changes and we merge it. But I'll try again.

@pangolp
Copy link
Contributor

pangolp commented Mar 27, 2024

I'll tell you what I wanted to do to make it clearer. If you have it enabled to accept modifications from other people, we could make changes to the branch, and merge everything necessary into this pull request, without the need to create another one. But that is a permissions issue, which you must configure in the fork that you created.

git clone https://github.com/anzz1/mod-npc-buffer.git -b patch-1
git push origin patch-1

@anzz1
Copy link
Contributor Author

anzz1 commented Mar 27, 2024

Yes, I understand, I have this feature enabled and this info box says it should work
image
Maybe you don't have write access to azerothcore/mod-npc-buffer ?
Or maybe GitHub is just bugged. I just now unchecked and re-checked the box to also answer the "have you tried turning it off and on again?" question 😄

@pangolp
Copy link
Contributor

pangolp commented Mar 27, 2024

Maybe you don't have write access to azerothcore/mod-npc-buffer ?
Or maybe GitHub is just bugged. I just now unchecked and re-checked the box to also answer the "have you tried turning it off and on again?" question 😄

No. I tried it this morning, now that I remember at work, where I don't even use ssh. I had to clone it over https. Maybe that was the problem. I'll try again. But it is good to know that you have the option enabled, to rule out errors. I will try again.

@pangolp
Copy link
Contributor

pangolp commented Mar 27, 2024

Indeed, it was something related to permits. So I'll move on, from now on. Thanks for reviewing it.

@pangolp
Copy link
Contributor

pangolp commented Mar 27, 2024

Ready, I think that will be enough.
I will wait for the tests to pass, and if they are successful.
I'll merge it, thanks for your contribution.

@pangolp pangolp merged commit 0888f37 into azerothcore:master Mar 27, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants