cc3768 / UniversalCoinsMod

Universal Coins mod for Minecraft, using Minecraft Forge.
MIT License
9 stars 5 forks source link

UC price files in wrong location #17

Closed Higgs1 closed 9 years ago

Higgs1 commented 9 years ago

Hello!

I'd like to suggest a feature and/or bug fix: Could you please move the UC price files to Forge's config directory? ({gamedir}/config/universalcoins instead of ./config/universalcoins)

I recently recieved a mod pack from a friend, and thought the prices were way off but it turns out it wasn't reading from his price files, since he had them in {gamedir}/config/universalcoins, but my Minecraft instance was looking for them at ./config/universalcoins (where it generated the default price files).

Thanks :D

notabadminer commented 9 years ago

The config files should be in {gamedir}/config/universalcoins. I've run on Windows and Linux without issues. What OS and Java version are you using? Are you using a custom launcher?

Higgs1 commented 9 years ago

I'm on Linux, OpenJDK, and yes a custom "launcher". But in the code... https://github.com/notabadminer/UniversalCoinsMod/blob/master/java/universalcoins/util/UCItemPricer.java#L42

private static String configDir = "." + File.separatorChar + "config" + File.separatorChar + "universalcoins" + File.separatorChar;

It kinda points specifically to ./config/universalcoins, and not {gamedir} {forge config dir}. Other mods put their config files in {gamedir}/config {forge config dir} somehow, so I thought it would be a fairly simple change that would be a really great addition to your mod.

Mods that use {gamedir}/config {forge config dir}:

Mods that use ./config:

Edit: I think Forge even has a specific directory to put config folders in, not just {gamedir}/config

Higgs1 commented 9 years ago

Hi, me again! I looked it up and there is a specific directory Forge has for dumping config things in and apparently it's FMLPreInitializationEvent.getModConfigurationDirectory()

In the Java Docs: FMLPreInitializationEvent.getModConfigurationDirectory()

And then you can

File configDir = new File(event.getModConfigurationDirectory(), "universalcoins")

Would the forge devs want you to use your modid instead of a string (if this is how to get the modid, I have no idea...)?

File configDir = new File(event.getModConfigurationDirectory(), event.getModMetadata().modId)
notabadminer commented 9 years ago

With the unmodified launcher, "." is the minecraft folder. Which is why using the code config/universalcoins was used. I wasn't aware of any issues as I haven't used any other launchers. I will change the code to match how NEI gets the config dir and it should resolve the issue. NEI uses FMLInjectionData.data()[6] which returns the Minecraft home dir.

The new line will be: private static String configDir = FMLInjectionData.data()[6] + "/config/universalcoins/";

Higgs1 commented 9 years ago

Thanks! :+1: That code looks pretty nifty. Now I can just drag and drop one config directory when I get an update from my friends.

Now I'll just need to message the AW2 devs. Maybe they can even use that line of code too.