sarbian / ModuleManager

177 stars 96 forks source link

MM cache is not compatible with \n #84

Closed Sigma88 closed 5 years ago

Sigma88 commented 6 years ago

example:

NODE
{
    description = FirstLine\nSecondLine
}

will be saved to the cache as

UrlConfig
{
    name = NODE
    type = NODE
    parentUrl = /test
    url = /test/NODE
    NODE
    {
        description = FirstLine
SecondLine
    }
}

which means that when running KSP without MM cache the description will be loaded as

FirstLine\nSecondLine

while when running KSP and loading the nodes from the MM cache, the description will be loaded as just

FirstLine

Sigma88 commented 6 years ago

pinging @blowfishpro in case he might know :)

sarbian commented 6 years ago

The cache is written with KSP stock ConfigNode.Save(path) so the problem is in there :( So we either have to escape everything before saving, write our own Save or cache in an other format (binary serialization ? protobuf ? Unity JSON serializer). That may improve perf a bit more. Or not.

Sigma88 commented 6 years ago

in the meantime I've circumvented the issue using &br; and .Replace("&br;","\n")

let me know if you need some tests

blowfishpro commented 6 years ago

Sounds like KSP is handling escaping properly when loading but not when saving. I'll see about letting Squad know about this

Sigma88 commented 6 years ago

the major problem about this is that the behaviour is not consistent between starts

if you start the game without the cache the result is different than with the cache.

if it wasn't for that I wouldn't have even noticed

blowfishpro commented 6 years ago

Interesting, apparently \n is only properly processed in the first place because of the localizer. So I guess that explains why it's not handled on save

jrossignol commented 6 years ago

Ugh, I hit this one too. Used to work before 1.3 because KSP wouldn't do anything with the \n.

Sigma88 commented 6 years ago

I haven't closed the issue just because the info might help people having this same issue, just make sure you use an alternative symbol (I use &br;)

jrossignol commented 6 years ago

The problem with changing to &br; is that I can't easily assess the impact - out of two dozen or so contract packs out there, they could all be using '\n' voraciously... or not using it at all. So I'd rather explore options that don't require that change until I have no other choice.

Sigma88 commented 6 years ago

ah yes, but the issue here (iirc) is that the localizer changes \n to newline so by the time mm gets the cfg it is already messed up

@sarbian @blowfishpro correct me if I'm wrong

blowfishpro commented 6 years ago

That's correct. The only thing I can think of to fix this on MM's side would be to rewrite the ConfigNode saving code and use that when writing the cache.

jrossignol commented 6 years ago

Agreed, it's not pretty, and even if it was I'm not sure if I'd want to be the one to have MM reformat every config node value before it goes the door in cache.

So @JPLRepo - since I can't easily ping you on the KSP tracker, any chance you (or someone else) can respond on #16315 to let us know if there's any chance for a fix in 1.4.2?

JPLRepo commented 6 years ago

You can always get me on IRC.

seanth commented 6 years ago

In some experiments with Kopernicus today, I was able to get line feeds using a syntax like "\\nn""

The internal "\n" is removed, but in doing so, it creates a valid "\n". I tested this using MM cache or fresh, and both worked in my hands.

Not a great solution, but could be a workaround.

blowfishpro commented 5 years ago

Fixed now

Sigma88 commented 5 years ago

Thanks @blowfish this is great!