- 
                Notifications
    You must be signed in to change notification settings 
- Fork 16
Adding MP support to TextUtil #36
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
base: develop
Are you sure you want to change the base?
Conversation
| public class TextUtil { | ||
| private static final MiniMessage MINI_MESSAGE = MiniMessage.miniMessage(); | ||
|  | ||
| private static boolean miniplaceholders = false; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this to make it fit your codestyle in PlaceholderUtil.java.
I would suggest introducing static booleans like this to HooksManager.java instead
| Hi, sorry for reviewing it this late. I was going through the changes, which I think are well implemented, but I'd like to understand the reason behind this change. As far as I can tell, we already use  Thanks in advance! | 
| I finally had time to come back to this project and take a brief look at the PR again. As I already wrote in the title, this PR introduces Miniplaceholders (https://github.com/MiniPlaceholders/MiniPlaceholders) support to Akropolis. This allows all Akropolis messages ( and methods in  | 
| Due to the time that has passed since I opend this PR, I would be willing to rebase this PR so it is based on your newest version instead of the old one. That would also allow changing the commit messages to fit your spec. | 
| I understand now, I didn't notice previously that the messages from  | 
| I did the rebase earlier today. You changed the behavior of the replacements in commit a8a5b74 to use string replacements instead of converting between components and text. I propose using standard MiniMessage TagResolvers for this, because that is exactly what they are designed for. I have marked this PR as a draft for now. As soon as I find time to test the changes, I will mark it as ready for review. | 
| Thank you! And you're right, I should probably use TagResolvers for that. | 
With this PR I suggest adding basic MiniPlaceholder support to TextUtil.
I implemented this trying to hopefully fit with your codestyle.
Feel free to request changes.