Deadrik / TFCraft

TerraFirmaCraft
GNU General Public License v3.0
204 stars 142 forks source link

Added crafting config sync from server to client + ... #788

Closed dries007 closed 9 years ago

dries007 commented 9 years ago

The fun parts

This pile of changes will allow you to change all config values on the fly via the ingame GUI config. (I couldn't get it to use the sliders for numbers without massive amounts of extra work, but oh well.) This includes world gen and crafting recipes! You can now also change the recipe options file on the server without having to tell your players to change there values, as it syncs on connect.

The boring parts

I did not squash all of the commits, because some of them are quite large.

There is a pile of optimizations and removal of unnecessary code in this too.

In Part 3 (f05a7cb75b9e70ea2e0b79127e0de44bfb6a71e6) I did optimize all imports as close as I could get them to what was there. This is the config I used:

java.*
javax.*

io.netty.*
net.minecraft.*
net.minecraftforge.*
cpw.*

org.lwjgl.*

all others

all static

Somewhat important:

Kittychanley commented 9 years ago

Edit: I just noticed your message about part 3. I literally can't see the important changes to the config file stuff because there's too many files edited with nothing more than reorganized imports.... reorganized imports that are going to get reverted the next time I or Bioxx edit that file because our save actions are set differently than yours. I would also prefer if you didn't remove any unnecessary code, as we may use it at a later date. I appreciate your efforts, but they really belong in a completely separate PR if you really want them merged, especially any optimization changes that are unrelated to the config syncing.

I've noticed there are a few file diffs that are nothing more than moving an import to a different line. Could you maybe get rid of those to help cut down on the number of files changed?

Yeah.. just scrolling through it looks like the majority of those 507 files are nothing more than reorganizing imports or removing whitespace. If you want to submit those as a separate PR that's fine, but they really don't belong in this one.

Kittychanley commented 9 years ago

Since there's conflicts on this PR now, as well as waaaay too much stuff that is out of scope for what we discussed, I'm just going to close this for now. You can submit another PR with just the stuff for the config syncing but please ask and actually talk with us before making massive changes to the entire repo.