legobmw99 / Allomancy

Brandon Sanderson's Allomancy, now in Minecraft
https://minecraft.curseforge.com/projects/allomancy
GNU General Public License v3.0
23 stars 18 forks source link

Added Key to toggle Hud (default: h) and config option to save state. #47

Closed Nick1st closed 3 years ago

Nick1st commented 3 years ago

I did this cause of #46. I also added a German translation.

legobmw99 commented 3 years ago

Thanks for doing this.

I'll take a look later, it seems like our Java formatters disagree slightly which is making the diff hard to read on mobile.

If it all looks good I will happy accept the PR, but I think I will set the key to unbound by default to avoid clutter in large packs.

Nick1st commented 3 years ago

Not sure about the last commit, but I think it's better to read a boolean each time the overlay is rendered instead of reading a config option.

legobmw99 commented 3 years ago

I'm not sure it ultimately matters, but you'd need to also update it in the refresh method in AllomancyConfig.

This may end up being more work than it is worth for a client side only thing.

Is there any noticeable difference in performance with the caching? If F3 (possibly with the graph) doesn't show any difference I think it would be better to keep simple. The render methods already check several config values and I don't think it's a performance problem

Nick1st commented 3 years ago

No, for me it doesn't matter, but I have an high end pc

Nick1st commented 3 years ago

I've tested it with my very bad Laptop and it doesn't make any difference.

legobmw99 commented 3 years ago

I'd say go ahead and revert that last commit then

Nick1st commented 3 years ago

Done