search

aRTy42 / POE-ItemInfo

Item Info Script for Path of Exile
166 stars 224 forks source link

fixed some file encodings #54

Closed Eruyome closed 7 years ago

Eruyome commented 7 years ago

I'm not sure why this was only an issue for TradeMacro, but I had to change the defaults.ini file encoding (https://github.com/PoE-TradeMacro/POE-TradeMacro/issues/201). To make sure that there are no further problems I converted the entire ItemInfo data folder to UTF-8 (using UTFCast). Looking at the changes made to those files it seems some other files had issues, too (div card list, dark shrine effects for example).

aRTy42 commented 7 years ago

Uniques and divination cards are currently intentionally as ANSI because "Mjölner" and "Maelström of Chaos" cause trouble with utf-8. I can try utf-8 bom again though.

aRTy42 commented 7 years ago

Your config file does not work for me and I suspect that other files may cause issues too. If I'm not mistaken almost all files were in ANSI and I think that's the format we should continue to use.

Edit: Oh and I should add that GitHub somehow decides to alter encoding when you edit files in the browser and I think even in the desktop version.

Eruyome commented 7 years ago

It seems I have to look into this a bit more.

aRTy42 commented 7 years ago

The default.ini file had a weird format, this went unnoticed because the config.ini was included in the releases which is not really how it is supposed to work though. I deleted the config from the last release because I realised this when I wanted to update the default for ShowCurrencyValueInChaos, now that we have the poe.ninja method. The encoding didn't cause an error for me when I tested it, no I didn't notice that is was not as intended.

The scripts method to rewrite the default file seems to be broken and it is actually not needed from what I see. The default file should never be missing and even if it does then the script has hardcoded fallback values and a config file gets created with those values once the user accesses the user settings menu.

While we are at those config files anyway: What do you think of moving the default.ini into the lib folder? Now that we have it, the file seems rather misplaced in the data folder. We could consider moving the image files aswell.

Eruyome commented 7 years ago

Not sure, the lib folder isn't quite the right place for it, too. Maybe just a folder resources for images/icons, possibly with subfolders images and config for the default.ini and something-else for updates.txt and authors.txt.

Btw, I knew all that you just explained. I just didn't notice that the default.ini was pretty messed up when downloaded from the TradeMacro github repo. It was fine when downloaded from the ItemInfo repo.

4GForce commented 7 years ago

I'm just not sure about removing the CreateDefaultConfig() method, nothing is gained from doing so and in the rare case that an alien would delete default_config.ini the revert to default feature won't work anymore.