Added translations #19

FrankHeijden · 2017-10-18T18:34:02Z

Added some translations to the config.yml

+ Translations

Phoenix616

Awesome that you took it upon yourself to implement these changes but I feel that this could be designed a bit better.

Additionally to the individual comments I think it would be better to avoid all the null checks either by having a central sendMessage method in the main class that handles them or by returning a general "message not found" error when the getting a message with a non-existing key.

That could clean up the code a lot and keep down the number of the changed lines.

Phoenix616 · 2017-10-18T19:06:26Z

src/main/java/com/wfector/command/Help.java

+        }
+        if(sender.hasPermission("csn.command.cleandatabase") && plugin.getMessage("help.cleandatabase-all") != null) {
+            helpItems.add(plugin.getMessage("help.cleandatabase-all"));
+        }


Would be good to merge all these so that sender.hasPermission("csn.command.cleandatabase") is only called once?

Phoenix616 · 2017-10-18T19:06:50Z

src/main/resources/config.yml

@@ -1,5 +1,5 @@
 # Chest Shop Notifier
-# Config file for 1.2
+# Config file for 1.3


If you want to increase the version please also do so in the pom.xml.

Phoenix616 · 2017-10-18T19:12:53Z

src/main/java/com/wfector/command/CommandRunner.java

@@ -91,7 +105,9 @@ public boolean onCommand(final CommandSender sender, Command cmd, String label,
                                    if (userId != null) {
                                        cleaner.cleanUser(userId);
                                    } else {
-                                        sender.sendMessage(ChatColor.RED + args[i + 1] + " is not a valid username/uuid input for " + param.getUsage() + "!");
+                                        if(plugin.getMessage("invalid-username") != null) {
+                                            sender.sendMessage(plugin.getMessage("invalid-username").replace("{typo}", args[i + 1]).replace("{usage}", param.getUsage()));


Would be good if we could use a better way to replace placeholders in messages. Something like getMessage("invalid-username", "type", args[i + 1]) via a getMessage(String key, String... replacements) method?

Added changes

Added changes, reduced size

Added changes

updated, added some translations

Updated

changed version

FrankHeijden · 2017-10-18T22:09:50Z

Should be good to go 👍

Phoenix616 · 2017-10-18T22:42:12Z

src/main/java/com/wfector/command/History.java

@@ -73,8 +73,7 @@ private void gatherResults() {
                Statement readStatement = c.createStatement();
                int rowsUpdated = readStatement.executeUpdate("UPDATE csnUUID SET `Unread`='1' WHERE `ShopOwnerId`='" + userId.toString() + "'");

-                if (rowsUpdated > 0 && plugin.getMessage("history-marked-read") != null)


I think you removed too much here ;)

Phoenix616 · 2017-10-18T22:43:18Z

src/main/java/com/wfector/notifier/ChestShopNotifier.java

+//        return input;
+        return ChatColor.translateAlternateColorCodes('&', input);
+    }
+


Do we really need this extra method just to translate the color codes?

Phoenix616 · 2017-10-18T22:44:24Z

src/main/java/com/wfector/notifier/ChestShopNotifier.java

+        } else {
+            return "Missing string 'messages." + key + "' in config.yml";
+        }
+    }


I think we only need one getMessage method as java will just use the one below with a zero sized array if no argument is provided after the first one.

Phoenix616 · 2017-10-18T22:45:04Z

src/main/java/com/wfector/notifier/ChestShopNotifier.java

+        String s = getConfig().getString("messages." + key);
+        if (s != null && !s.isEmpty()) {
+            for (int i = 0; i < replacements.length; i++) {
+                s = s.replace("{" + i + "}", replacements[i]);


You need to get replacements[i+1] here and check for i + 1 < replacements.length

Removed too much :P

1 getMessage method; Fixed texty getMessage

wrong file :p

fixed removal

FrankHeijden · 2017-10-19T12:13:54Z

I hope this fixed the getMessage method; im not very skilled in java. Learning with the process 💯

Phoenix616 · 2017-10-19T14:57:58Z

src/main/java/com/wfector/command/Help.java

+
+    public Help(ChestShopNotifier main) {
+        plugin = main;
+    }

    public static void SendDialog(CommandSender sender) {


One last thing that I noticed then it should be good:
You either need to pass the plugin instance to the method or make the method non-static and initialise the Help object in the CommandRunner class in order to access the plugin field. (Help is never initialised in the CommandRunner). I suggest fixing the case of the sendDialog method at the same time.

- static

FrankHeijden · 2017-10-19T15:59:07Z

src/main/java/com/wfector/command/CommandRunner.java

@@ -134,7 +134,7 @@ public boolean onCommand(final CommandSender sender, Command cmd, String label,
                            if (userId == null) {
                                OfflinePlayer target = plugin.getServer().getPlayer(args[1]);
                                if (target == null) {
-                                    target = plugin.getServer().getOfflinePlayer(args[1]);
+                                    target = plugin.getServer().getOfflinePlayer(NameManager.getUUID(args[1]));


is it possible to use this method instead of the deprecated getOfflinePlayer(args[1]) method?

There is not really a need to get the OfflinePlayer here as it was only used to get the player's UUID which can be replaced with the NameManager method call.

Phoenix616 · 2017-10-19T16:38:18Z

src/main/java/com/wfector/command/CommandRunner.java

@@ -25,39 +25,39 @@ public CommandRunner(ChestShopNotifier plugin) {
    public boolean onCommand(final CommandSender sender, Command cmd, String label, String[] args) {
        if(args.length == 0) {
            Help.SendDialog(sender);


You are still calling a method here statically that isn't static. You need to do something like new Help(this).sendDialog(sender). Your IDE should warn you about this. ;)

It did not, it gave me an error when using new Help(this).sendDialog(sender)
"Help (com.wfector.notifier.ChestShopNotifier) in Help cannot be applied to (com.wfector.command.CommandRunner)"

new Help(plugin).SendDialog(sender); works though

Phoenix616 · 2017-10-19T16:39:02Z

src/main/java/com/wfector/command/CommandRunner.java

@@ -17,6 +16,7 @@

 public class CommandRunner implements CommandExecutor {
    private final ChestShopNotifier plugin;
+    private Help Help;


Imo. there is no need to store the Help command in a separate field. IT can be instantiated when the command is used.

How could i achieve that? if i remove that line the class would break when trying to get something from Help.

You don't need the field (that you don't assign anywhere anyways) if you create a new object instance when it is needed. That would be more obvious if you used the normal, lower-case starting naming scheme of variables, fields and methods.

Also there is another help dialog further down on line 114.

Im not exactly sure what i must do now, do i need to remove the uppercase Help to help? And change that on line 114 too?

Yes, instantiate a new Help object on line 114 too and remove the field, it never gets used nor assigned anywhere.

Fixed SendDialog

FrankHeijden · 2017-10-20T15:49:16Z

Fixed 📦

FrankHeijden added 10 commits October 18, 2017 20:21

Update config.yml

9d92ee7

+ Translations

Update CleanDatabase.java

c3dfab2

+ Translations

Update Clear.java

c75f052

+ Translations

Update CommandRunner.java

125bfcb

+ Translations

Update Help.java

7e0534c

+ Translations

Update History.java

927d7bc

+ Translations

Update BatchRunner.java

7589248

+ Translations

+ Translations

d809c78

+ Translations

+ Translations

216dcad

+ Translations

Update plugin.yml

87f0185

FrankHeijden mentioned this pull request Oct 18, 2017

language/message #18

Closed

Phoenix616 requested changes Oct 18, 2017

View reviewed changes

FrankHeijden added 10 commits October 18, 2017 23:58

Update CleanDatabase.java

225c45b

Added changes

Update Clear.java

a629f2a

Added changes

Update CommandRunner.java

5ce2830

Added changes

Update Help.java

0d08d03

Added changes, reduced size

Update ChestShopNotifier.java

40e4b76

Added changes

Update LoginRunner.java

13eed79

Added changes

Update config.yml

1286047

updated, added some translations

Update Converter.java

691cfac

Updated

Update History.java

0d0d95b

Updated

Update pom.xml

3151706

changed version

Phoenix616 reviewed Oct 18, 2017

View reviewed changes

FrankHeijden added 3 commits October 19, 2017 14:09

Update Help.java

4f7957e

Removed too much :P

Update ChestShopNotifier.java

5220535

1 getMessage method; Fixed texty getMessage

Update Help.java

6422315

wrong file :p

Update History.java

299f8cb

fixed removal

Phoenix616 reviewed Oct 19, 2017

View reviewed changes

FrankHeijden added 3 commits October 19, 2017 17:52

Update Help.java

8eda74b

- static

Update CommandRunner.java

6738e6e

Update CommandRunner.java

feebcff

FrankHeijden commented Oct 19, 2017

View reviewed changes

Phoenix616 reviewed Oct 19, 2017

View reviewed changes

FrankHeijden added 3 commits October 19, 2017 20:00

Update CommandRunner.java

1d8c7ac

Update CommandRunner.java

b7ffeb1

Update CommandRunner.java

f6faea4

Fixed SendDialog

Phoenix616 approved these changes Oct 20, 2017

View reviewed changes

Phoenix616 merged commit 88ca2e0 into ChestShop-authors:master Oct 20, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added translations #19

Added translations #19

FrankHeijden commented Oct 18, 2017 •

edited

Loading

Phoenix616 left a comment

Phoenix616 Oct 18, 2017

Phoenix616 Oct 18, 2017

Phoenix616 Oct 18, 2017

FrankHeijden commented Oct 18, 2017

Phoenix616 Oct 18, 2017

Phoenix616 Oct 18, 2017

Phoenix616 Oct 18, 2017

Phoenix616 Oct 18, 2017

FrankHeijden commented Oct 19, 2017

Phoenix616 Oct 19, 2017

FrankHeijden Oct 19, 2017 •

edited

Loading

Phoenix616 Oct 19, 2017

Phoenix616 Oct 19, 2017

FrankHeijden Oct 19, 2017

FrankHeijden Oct 19, 2017

Phoenix616 Oct 19, 2017

FrankHeijden Oct 19, 2017

Phoenix616 Oct 19, 2017 •

edited

Loading

FrankHeijden Oct 20, 2017

Phoenix616 Oct 20, 2017

FrankHeijden commented Oct 20, 2017

Added translations #19

Added translations #19

Conversation

FrankHeijden commented Oct 18, 2017 • edited Loading

Phoenix616 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FrankHeijden commented Oct 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FrankHeijden commented Oct 19, 2017

Choose a reason for hiding this comment

FrankHeijden Oct 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Phoenix616 Oct 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FrankHeijden commented Oct 20, 2017

FrankHeijden commented Oct 18, 2017 •

edited

Loading

FrankHeijden Oct 19, 2017 •

edited

Loading

Phoenix616 Oct 19, 2017 •

edited

Loading