diasurgical / devilutionX

Diablo build for modern operating systems
Other
8.11k stars 796 forks source link

Migrate from SimpleIni to our own implementation #7511

Closed glebm closed 2 weeks ago

glebm commented 3 weeks ago

Our implementation has a more modern interface and only supports the features that we care about.

It always outputs \r\n as newlines and does not output BOM.

The modern interface eliminates awkward c_str()/data() conversions.

This implementation preserves comments and the file order of sections and keys. New keys are written in insertion order.

We now also support modifying and adding default comments, which may be a useful thing to do for the especially tricky ini options (this PR doesn't add any but adds the ability to do so).

Sadly, this increases the RG99 binary size by 24 KiB. I'm guessing this is because the map implementation generates quite a bit of code.

Note that while it might seem that using std::string for every key and value would do a lot of allocations, most of these strings are small and thus benefit from Small String Optimization (= no allocations).

AJenbo commented 3 weeks ago

It always outputs \n as newlines

Considering INI files are a Windows thing and we extend diablo.ini maybe it should be \r\n?

AJenbo commented 3 weeks ago

I never was particularly happy with any of the ini libs. I like the cleaner options code, was this the main reason for the change?

glebm commented 3 weeks ago

Considering INI files are a Windows thing and we extend diablo.ini maybe it should be \r\n?

SimpleIni produced \r\n on Windows and \n on other platforms. I wanted to make it consistent on all platforms (whichever way we decide on).

Note that INI libraries on Windows support \n, and even Notepad displays \n correctly these days.

I like the cleaner options code, was this the main reason for the change?

Yeah, I wanted to clean that up a bit and figured writing an INI library from scratch won't be that difficult.

AJenbo commented 3 weeks ago

I like consistency, I think we should go for the Windows endings since it's often accessed my non technical users and it's what it's been for most users and we don't know how diablo reacts if it's files was changed to the other format.

glebm commented 2 weeks ago

Done, switched to \r\n