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

Added LangUtils support. #1693

Merged
merged 18 commits into from
Mar 13, 2021
Merged

Added LangUtils support. #1693

merged 18 commits into from
Mar 13, 2021

Conversation

apachezy
Copy link
Contributor

I added a hook to BentoBox to support LangUtils. If I have any problems with my work, please correct me.

LangUtils can be used to localize the names of items, entities, enchants, etc.
Since the original author and NyaaCat Community are not actively updating, I'm reconstructing LangUtils.

My LangUtils project address is:
https://github.com/apachezy/LangUtils

@tastybento tastybento added the Type: Enhancement Improvement or modification which is usually a new feature. label Feb 27, 2021
@tastybento
Copy link
Member

Interesting. I tried this a while ago with #1549 but it never got finished. I assume other parts of the code can use this for translations.

@tastybento tastybento requested a review from BONNe February 27, 2021 20:33
Copy link
Member

@BONNe BONNe left a comment

Choose a reason for hiding this comment

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

Should we need both repositories?
What is the difference

@BONNe
Copy link
Member

BONNe commented Feb 27, 2021

Also, I am not a keen fan of reflection. Isn't there a way how to access plugin directly without reflection?

@apachezy
Copy link
Contributor Author

Should we need both repositories?
What is the difference

Because I was in China, there was a high delay in visiting GitHub, so I used another domestic VCS repositorie, but it seems that there was also a high delay in visiting gitee from abroad......

@tastybento
Copy link
Member

@BONNe Do you think this is ok to merge?

@BONNe
Copy link
Member

BONNe commented Mar 7, 2021

I do not see obvious issues with it.

However, someone will need to take care of explaining this:

    private static boolean doHook(Plugin plugin) {
        // Because there are other plugins with the same name,
        // we should check here whether it is the plugin we need.
        boolean classExists;
        try {
            Class.forName("com.meowj.langutils.lang.LanguageHelper");
            classExists = true;
        } catch (ClassNotFoundException e) {
            classExists = false;
        }

        hooked = plugin != null && plugin.isEnabled() && classExists;
        return hooked;
    }

If there are different LanguageHelper plugins, we could run into issues when people used incorrect one and complain why they do not work.

@apachezy apachezy closed this Mar 8, 2021
@apachezy apachezy reopened this Mar 8, 2021
@apachezy
Copy link
Contributor Author

apachezy commented Mar 8, 2021

Ahhhh, I accidentally clicked to close! ! !

@apachezy
Copy link
Contributor Author

apachezy commented Mar 8, 2021

I do not see obvious issues with it.

However, someone will need to take care of explaining this:

    private static boolean doHook(Plugin plugin) {
        // Because there are other plugins with the same name,
        // we should check here whether it is the plugin we need.
        boolean classExists;
        try {
            Class.forName("com.meowj.langutils.lang.LanguageHelper");
            classExists = true;
        } catch (ClassNotFoundException e) {
            classExists = false;
        }

        hooked = plugin != null && plugin.isEnabled() && classExists;
        return hooked;
    }

If there are different LanguageHelper plugins, we could run into issues when people used incorrect one and complain why they do not work.

Sorry, my English is very bad.

This is because I have seen other plug-ins called "LangUtils" elsewhere, but their functions are completely different! Therefore, a judgment to determine whether the "class" exists is added.
I have also considered whether to increase the judging whether some methods exist.

@BONNe
Copy link
Member

BONNe commented Mar 8, 2021

Hmm, now I am more confused than previously.

The code you added means that there is a lot of LangUtils plugins that comes from com.meowj.langutils.lang

@apachezy
Copy link
Contributor Author

apachezy commented Mar 8, 2021

Hmm, now I am more confused than previously.

The code you added means that there is a lot of LangUtils plugins that comes from com.meowj.langutils.lang

What I mean is that the name of some plugins in Plugin.yml is "LangUtils".

Yes it is. . . . You are right to say that, the original author of LangUtils, LangUtils of the Nyaacat community, are all from com.meowj.langutils.lang.
But the LangUtils I refactored is compatible with theirs. . .

@tastybento
Copy link
Member

We'll need to list at least one that we are compatible with in the docs. I assume the root one is this one? https://github.com/NyaaCat/LanguageUtils

Can you provide other links?

@apachezy
Copy link
Contributor Author

apachezy commented Mar 8, 2021

We'll need to list at least one that we are compatible with in the docs. I assume the root one is this one? https://github.com/NyaaCat/LanguageUtils

Can you provide other links?

Please do not include https://github.com/NyaaCat/LanguageUtils in the document because it is not compatible with BentoBox.

Currently only https://github.com/apachezy/LangUtils is fully compatible with BentoBox, because it implements more game object name translation.

@apachezy apachezy marked this pull request as draft March 10, 2021 18:31
@apachezy apachezy marked this pull request as ready for review March 11, 2021 10:22
@tastybento tastybento self-requested a review March 12, 2021 01:50
@@ -82,6 +82,8 @@ private void openPanel(User user, String ivPanelName) {

private PanelItem getPanelItem(DamageCause c, User user) {
PanelItemBuilder pib = new PanelItemBuilder();
// todo: Please consider adding translation fields for each entry
// of "DamageCause" in the language file in the future.
Copy link
Member

Choose a reason for hiding this comment

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

I made #1705

@tastybento
Copy link
Member

I'm good with this. If @BONNe is good, then we merge.

@BONNe
Copy link
Member

BONNe commented Mar 12, 2021

Yes, I am good with it

@tastybento tastybento merged commit 0f0d8b9 into BentoBoxWorld:develop Mar 13, 2021
@apachezy apachezy deleted the ApacheZy-LangUtils branch March 13, 2021 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Improvement or modification which is usually a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants