Code Snippets/Classes

Discussion in 'Mod Development' started by Strikingwolf, Sep 19, 2014.

  1. squeek502

    squeek502 New Member

    I've seen this elsewhere and I really don't understand the point. These exact constants exist in the Minecraft code as net.minecraft.util.EnumChatFormatting, which is both more robust (has utility functions like getTextWithoutFormattingCodes) and more future proof (even though it's probably unlikely to break in this instance, it's still always better to use Minecraft's implementations wherever possible).

    Encouraging new modders to use anything other than EnumChatFormatting is dubious at best.
     
  2. Strikingwolf

    Strikingwolf New Member

    I think it is for things like item names and stuff. Though if you can do that with the enum I will remove it
     
  3. squeek502

    squeek502 New Member

    It's literally the exact same thing.
    Code:
    EnumChatFormatting.BLACK.toString().equals(StringHelper.BLACK)
    The only difference is that the EnumChatFormatting values aren't strings, so you can't concatenate them together directly, but that's easy to solve.
    Code:
    // doesn't work
    EnumChatFormatting.BLACK + EnumChatFormatting.BOLD + "bold text";
    
    // works
    EnumChatFormatting.BLACK.toString() + EnumChatFormatting.BOLD + "bold text";
    
    If the intention of StringHelper is to make them available as String objects, then, at the very least, the constants in StringHelper should be implemented using EnumChatFormatting.
     
  4. Strikingwolf

    Strikingwolf New Member

    Deleting it from the OP then
     
  5. chbachman

    chbachman Guest

    Well, StringHelper is designed to be a easier solution. Normally the full class looks something like this. And i do not think that the color changing code will be changing in future versions. However, i see that you should point new people to the EnumChatFormatting.
     
  6. squeek502

    squeek502 New Member

    I'd like to hear the justification for those formatting constants from the CoFH devs. I'm almost positive it'd be "that's how we did it a long time ago and we have just ignored it since." There's no benefit to not using EnumChatFormatting, and CoFH should probably fix that. Wrappers are fine, but outright reimplementation seems like it should only be a last resort.

    So much of Minecraft mod code (mine included) is simply "this works or has worked for someone else" or "this is something I tried and it happens to work so I'll use it" rather than anything that has real reasoning behind it being the way it is.

    For example, ever since I started modding (in 1.6.4), I've never understand the purpose of the @SidedProxy classes. Mods and mod tutorials all still use them, but unless I'm missing something, I've never really seen anything that they give you that the @Mod.EventHandler's don't. Can anyone explain that to me? I've gotten by fine without ever using them.

    I have similar issues with the LogHelper class in the OP. What benefit does it have over simply putting
    Code:
    public static final Logger Log = LogManager.getLogger(Reference.MOD_ID);
    in your @Mod class and then using it statically?

    The equivalent for 1.6.4:
    Code:
    public static final Logger Log = Logger.getLogger(Reference.MOD_ID);
    static
    {
        Log.setParent(FMLLog.getLogger());
    }
     
    Last edited: Oct 3, 2014
  7. squeek502

    squeek502 New Member

    And to not be a total negative nancy:

    Utility: KeyHelper; detects shift/control with proper Mac support
    Code:
    public class KeyHelper
    {
       public static boolean isCtrlKeyDown()
       {
         // prioritize CONTROL, but allow OPTION as well on Mac (note: GuiScreen's isCtrlKeyDown only checks for the OPTION key on Mac)
         boolean isCtrlKeyDown = Keyboard.isKeyDown(Keyboard.KEY_LCONTROL) || Keyboard.isKeyDown(Keyboard.KEY_RCONTROL);
         if (!isCtrlKeyDown && Minecraft.isRunningOnMac)
           isCtrlKeyDown = Keyboard.isKeyDown(Keyboard.KEY_LMETA) || Keyboard.isKeyDown(Keyboard.KEY_RMETA);
    
         return isCtrlKeyDown;
       }
    
       public static boolean isShiftKeyDown()
       {
         return Keyboard.isKeyDown(Keyboard.KEY_LSHIFT) || Keyboard.isKeyDown(Keyboard.KEY_RSHIFT);
       }
    }
    Utility: ColorHelper; converts between the common color types used within Minecraft code (hexadecimal color using an int, rgba using ints from 0-255, and rgba using floats from 0.0-1.0)
    Code:
    public class ColorHelper
    {
       public static float[] toNormalizedRGB(int color)
       {
         return toNormalizedRGBA(color | 255 << 24);
       }
    
       public static float[] toNormalizedRGBA(int color)
       {
         int[] rgba = toRGBA(color);
    
         return new float[]{rgba[0] / 255f, rgba[1] / 255f, rgba[2] / 255f, rgba[3] / 255f};
       }
    
       public static int[] toRGBA(int color)
       {
         int alpha = color >> 24 & 255;
         int red = color >> 16 & 255;
         int green = color >> 8 & 255;
         int blue = color & 255;
    
         return new int[]{red, green, blue, alpha};
       }
    
       public static int fromRGBA(int r, int g, int b, int a)
       {
         return (a & 255) << 24 | (r & 255) << 16 | (g & 255) << 8 | b & 255;
       }
    
       public static int fromRGB(int r, int g, int b)
       {
         return fromRGBA(r, g, b, 255);
       }
    
       public static int fromNormalizedRGBA(float r, float g, float b, float a)
       {
         return fromRGBA((int) r * 255, (int) g * 255, (int) b * 255, (int) a * 255);
       }
    
       public static int fromNormalizedRGB(float r, float g, float b)
       {
         return fromNormalizedRGBA(r, g, b, 1f);
       }
    }
    
     
    SatanicSanta likes this.
  8. chbachman

    chbachman Guest

    For the log helper class, I do the same thing.

    For the sided proxy, they are useful for having entire classes that are only on the server or client. They technically have no real use, as you said, everything is provided to you elsewhere, however they are extremely useful for registering textures, sounds and other client only things.
     
  9. squeek502

    squeek502 New Member

    Yes, but that can be done using event.getSide() == Side.CLIENT in the @Mod.EventHandler's. I mean, it's not a terrible thing to use @SidedProxy if it helps you organize things in some way, but it's weird to see it promoted in some tutorials as basically a requirement for any mod when it seems to be anything but. I've seen a few extremely sparse implementations of the @SidedProxy classes that hardly do anything and only seem to exist out of a false sense of obligation.
     
  10. Democretes

    Democretes New Member

    I've needed to get someone's display text, so EnumChatFormatting is next to useless for me. StringHelper is just less keystrokes for me so I intend on using it. I wouldn't consider it "dubious", I just like to do less work.

    And it is definitely not better to use Minecraft's implementation. Some of their code is just plain awful. TileEntityChest, for example, is terribly implemented. Instead of remembering which direction the chest is and storing a single TileEntity to reference, they make a different TileEntity to reference for each direction, which is just silly. It's like remembering where you sit in a classroom based upon the four people around you instead of one person and one direction. Needless to say, to make it work properly with any kind of item transfer, you have to work around their code, which you shouldn't have to do. Look around through the code sometime. I've found a half dozen redundencies that just sit there for no reason, literally doing nothing.
     
    SatanicSanta likes this.
  11. Qazplm601

    Qazplm601 Lord of the Tumbleweeds

    i have always wondered why no-one has made a mod that gets rid of that stuff in the code. (i know it would be incompatible with pretty much everything, i just wonder why no one has.)
     
  12. Democretes

    Democretes New Member

    If your talking about the redundencies, removing them wouldn't do all that much honestly. They do nothing, so why bother removing them?

    The bad code, on the other hand, could make modding a bit smoother, and might actually improve your fps, even by a small amount.
     
  13. squeek502

    squeek502 New Member

    No clue what you mean, but that can't possibly be true. As I said before, EnumChatFormatting.BLACK.toString().equals(StringHelper.BLACK). They, by definition, have to be able to be used for the same purposes. They are exactly equivalent.

    If you're concerned about keystrokes, just implement StringHelper like so:
    Code:
    class StringHelper
    {
        public static final String BLACK = EnumChatFormatting.BLACK.toString();
        // etc
    }
    
    I both agree and disagree. Yes, you should always strive to improve things where possible. However, copying and reimplementing an existing Minecraft class as your method of improving it should be a last resort in all instances and needs special justification. Maintaining compatibility with Minecraft code means that your mod will, on average, be more compatible with the base game and with other mods as a whole. For example, extending a Minecraft class and only overriding the bare minimum for what you want to do should be preferred over reimplementing the class entirely (see stuff like this).
     
    Last edited: Oct 3, 2014
  14. Qazplm601

    Qazplm601 Lord of the Tumbleweeds

    both really. i am mostly thinking about how i thought someone, somewhere would make a 1.7.10 chunk lag patch. (no, fastcraft DOES NOT do this :p)
    (even if it was a jar mod.)
     
  15. Democretes

    Democretes New Member

    I meant to say "I've never needed to get display text...". The only significant difference between the two is the getTextWithoutFormatting(), which is only needed for display text. I've never needed to do so. Could I make StringHelper.BLACK = EnumChatFormatting.BLACK, yeah, but it's just work. It's already setup, and I have no reason to change it just to make it reference MC code. In any case I need just a short tidbit of information, I'd rather have it in my own code rather than referncing other code simply for the fact that it's easier to dig through. I don't need to go through and check separate classes for information. It's simply more convenient. Anyone who looks at the StringHelper knows exactly what's going on without needing to reference MC code. For a basic modder it would be easier to reference MC code, for anyone that knows what's going on, it's easier to just do it yourself.
     
  16. Strikingwolf

    Strikingwolf New Member

    Added
    I use it because I like extremely simple referencing. I don't have to give the logLevel or nothing. Just straight up call the method
     
  17. squeek502

    squeek502 New Member

    @Democretes, I admit in this instance it's a minor thing that more than likely won't cause any issues for you, but that's a really weird stance for you to take. It's not a lot of work at all for you to fix, and it actually obscures information: any new modder might assume that Minecraft code doesn't define those constants anywhere, and therefore may never know about the EnumChatFormatting class (which is exactly what happened to me when I was learning modding; I didn't know about EnumChatFormatting when I was starting to write TiC Tooltips because other mods I saw just defined their own formatting constants or didn't even use static constants [looking at you, Tinkers Construct]).

    The goal is to implement things well so that other people can learn from well implemented things. It's a cycle, and from what I've seen in the modding community, we currently err on the side of learning from poorly thought out implementations that only seem to exist for arbitrary or legacy reasons. It'd be nice to break away from that wherever possible.[DOUBLEPOST=1412298735][/DOUBLEPOST]
    You can do the same with the implementation I posted (it gives you a Logger instance, which has similar methods to what you have in LogHelper):
    Code:
    Log.debug("message");
    Log.info("message");
    Log.error("message");
    Log.warn("message");
    // etc
    
     
    Last edited: Oct 3, 2014
  18. Strikingwolf

    Strikingwolf New Member

    hmmm, I may switch over sometime, but right now I'm gonna stick with what I'm used to.
     
  19. Democretes

    Democretes New Member

    True, it is a trivial thing to argue, I just don't agree with the way Minecraft does it. It's bad form. There's no point in making it an enum over a static constant String. It just adds a few more keystrokes. You could essentially do the same thing with Strings and it would be more friendly when adding multiple cases together where as enums don't do that as well. It's not that I'm opposed to the idea, just the code in question.

    Even though it is a miniscule amount of work for me to change the values to something else, it is essentially work, and as a college student, it is my duty try and dodge as much work as I can.
     
  20. squeek502

    squeek502 New Member

    So I take it you also have a DirectionHelper where you define public static final int DOWN = 0 instead of using ForgeDirection.DOWN? EnumChatFormatting is exactly what Enum's are for.

    I'll save you the tremendous effort (5 seconds using a regex find+replace):
    Code:
      public static final String BLACK = EnumChatFormatting.BLACK.toString();
      public static final String BLUE = EnumChatFormatting.DARK_BLUE.toString();
      public static final String GREEN = EnumChatFormatting.DARK_GREEN.toString();
      public static final String TEAL = EnumChatFormatting.DARK_AQUA.toString();
      public static final String RED = EnumChatFormatting.DARK_RED.toString();
      public static final String PURPLE = EnumChatFormatting.DARK_PURPLE.toString();
      public static final String ORANGE = EnumChatFormatting.GOLD.toString();
      public static final String LIGHT_GRAY = EnumChatFormatting.GRAY.toString();
      public static final String GRAY = EnumChatFormatting.DARK_GRAY.toString();
      public static final String LIGHT_BLUE = EnumChatFormatting.BLUE.toString();
      public static final String BRIGHT_GREEN = EnumChatFormatting.GREEN.toString();
      public static final String BRIGHT_BLUE = EnumChatFormatting.BLUE.toString();
      public static final String LIGHT_RED = EnumChatFormatting.RED.toString();
      public static final String PINK = EnumChatFormatting.LIGHT_PURPLE.toString();
      public static final String YELLOW = EnumChatFormatting.YELLOW.toString();
      public static final String WHITE = EnumChatFormatting.WHITE.toString();
    
      public static final String OBFUSCATED = EnumChatFormatting.OBFUSCATED.toString();
      public static final String BOLD = EnumChatFormatting.BOLD.toString();
      public static final String STRIKETHROUGH = EnumChatFormatting.STRIKETHROUGH.toString();
      public static final String UNDERLINE = EnumChatFormatting.UNDERLINE.toString();
      public static final String ITALIC = EnumChatFormatting.ITALIC.toString();
      public static final String END = EnumChatFormatting.RESET.toString();
    EDIT: Fixed; the StringHelper names didn't match the EnumChatFormatting names.
     
    Last edited: Oct 3, 2014

Share This Page