squeek502 / VeganOption

A Minecraft mod that seeks to add vegan alternatives to all Minecraft mob/animal products
The Unlicense
45 stars 10 forks source link

Improve WAILA/TOP localization as discussed in #69 #70

Closed elifoster closed 7 years ago

elifoster commented 7 years ago

There are a couple cases where it may have made more sense to use Math.round instead of int typecasting, but I figured it's such an insignificant detail (68º vs 69º really is not crucial for composting effectively) that the significant performance hit (for Java 6 and below, see What is the most efficient way to round a float value to the nearest integer in java?) was not worth it.

I double checked and, although I think some of the formatting to begin with was wonky (weird spaces before and after : and %), it looks identical to how it did before.

squeek502 commented 7 years ago
elifoster commented 7 years ago

IIRC the int cast was intentionally chosen over rounding for display purposes, as it better matches how things work (0.5 degrees below max temp shouldn't show as max temp, since it won't behave as max temp until it actually gets there). I guess Math.floor could be used instead but I doubt it matters much.

Math.floor only provides different results from type casts when the value is negative, and I don't recall ever getting a negative temperature in the composter.

Waila uses a space before and after the colon separator, so I was just following its conventions there. Would need separate strings for TOP if it needs different formatting.

Oh I dunno. I doubt it matters a whole lot. I never really used WAILA, and I haven't played much MC since 1.7 so I don't know what TOP's standards are. If that is WAILA's standard then WAILA's standard is dumb :P From a translator perspective it would be infuriating if you had to translate all these tooltips twice just for different placement of colons.

squeek502 commented 7 years ago

Could do some weird shared localization stuff like:

VeganOption.waila.format=%s : %s
VeganOption.top.format=%s: %s

VeganOption.context.composting.title=Composting
VeganOption.context.composting.value=%d%%

And then do:

// waila
LangHelper.translate("waila.format", LangHelper.translate("context.composting.title"), LangHelper.translate("context.composting.value", (int) (tag.getFloat("Compost") * 100F))));

// top
LangHelper.translate("top.format", LangHelper.translate("context.composting.title"), LangHelper.translate("context.composting.value", (int) (composter.getCompostingPercent() * 100F))));

Could make that less verbose by adding a helper method like:

public static String contextString(String formatName, String contextName, Object... valueParams)
{
    return LangHelper.translate(formatName + ".format", LangHelper.translate("context." + contextName + ".title"), LangHelper.translate("context." + contextName + ".value", valueParams)));
}

// example usage
LangHelper.contextString("top", "composting", (int) (composter.getCompostingPercent() * 100F));
squeek502 commented 7 years ago

Following up on what you said on IRC:

I'm still stewing on that context string thing. It seems strange to me but I can't pinpoint exactly where my issue lies with it. I'm guessing I'll come to the conclusion that it's just because Java's string formatting is crap and doesn't allow named parameters

because of some language needed it to be value : title, they couldnt do it because %s and %s are treated equally and the order of the parameters passed to format is in-code and not translated :P

This could all be addressed by doing a few things:

  1. Add the shared strings + format thing I outlined here
  2. Explicit argument indexes can be used to reverse the order of the format (%2$s : %1$s would create value : title). This information can be added as a comment in the lang file to help with translation.
  3. If we want translators to have complete control, we could also add optional override keys for the entire string, (e.g. if (LangHelper.exists("waila.composting")) then only translate that key and pass in the value parameters, otherwise use the format + shared strings).

I think 1 and 2 are worth doing. 3 might be overkill, and could be added later if someone has a need for it.

elifoster commented 7 years ago

Ah thanks for pointing out the index thing! Wasn't aware of that. I'll work on this today. I agree that 3 is overkill.