search

nickbabcock / Pdoxcl2Sharp

A Paradox Interactive general file parser
MIT License
39 stars 13 forks source link

Potential newline issue in ParadoxSaver.cs? #35

Closed keegandent closed 3 years ago

keegandent commented 3 years ago

Apologies if I'm missing some step that converts back into \n, but it looks like the current code tries to save the pdox files with \r\n line endings. As far as I can tell, EU4, CK2, and Stellaris all use \n.

If I change the FileTest.txt to LF, the unit test fails (which makes sense).

$ grep -C 3 -REn "\\\r"
ParadoxSaver.cs-15-        public ParadoxSaver(Stream output) 
ParadoxSaver.cs-16-            : base(output)
ParadoxSaver.cs-17-        {
ParadoxSaver.cs:18:            Writer.NewLine = "\r\n";
ParadoxSaver.cs-19-        }

$  Failed Pdoxcl2Sharp.Test.FileSave.SaveNoChange [32 ms]
  Error Message:
   Assert.Equal() Failure
                          ↓ (pos 16)
Expected: date="1641.6.11"\nplayer="ALG"\n1=\n{\n\tname="Stoc��olm"\n\tcit···
Actual:   date="1641.6.11"\r\nplayer="ALG"\r\n1=\r\n{\r\n\tname="Stoc��olm"\r···
                          ↑ (pos 16)
  Stack Trace:
     at Pdoxcl2Sharp.Test.FileSave.SaveNoChange() in /home/keegan/Projects/Pdoxcl2Sharp/tests/Pdoxcl2Sharp.Test/FileSave.cs:line 45

I'd be more than happy to submit a PR, assuming there's no Pdox game that uses CRLF for which compatibility must be preserved.

nickbabcock commented 3 years ago

Wow, all these years no one has questioned the line endings. I could sworn that \r\n is what the games used, but maybe that was only EU3 or maybe I wanted the files that people opened in Notepad to "look right" 😉 .

You're right that they solely use \n now. You're not seeing any problems arise from using \r\n, right? I have a couple scripts that produce files with \r\n and EU4 and CK3 appear to be loading them file.

It seems reasonable to change our behavior to match paradox's.

keegandent commented 3 years ago

I verified it doesn't seem to impact Stellaris saves either. So much for what the wiki says. Loading and re-saving a file in Stellaris with CRLF endings does produce a file with LF endings. I'll open the PR anyway.