godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.1k stars 69 forks source link

Add a system-agnostic way to split strings on new lines #9233

Open Heros-Tempus opened 5 months ago

Heros-Tempus commented 5 months ago

Describe the project you are working on

A spinning wheel for random string selection which reads in, and writes to, a plain text file.

Describe the problem or limitation you are having in your project

The difference between how godot reads multiline strings when debugging verses when packaged for windows prevented my program from working as expected and it took an embarrassingly long time for me to figure out why.

After spinning the wheel, if it was set to delete items from the item list, I had it read in the item list file, split it on \n to get separate items, then check each item for the first instance that matched what was spun, removed that first one from the list, then wrote the output to the file. It worked fine while I was editing the it, but when I packaged it for windows it stopped working.

The reason it stopped working is because, when packaged and running on windows, it would read the newlines as \r\n so when I split on \n it would leave the \r in the string making it unable to find the corresponding item.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

A platform agnostic way of splitting lines of text would completely eliminate this (admittedly small) stumbling block.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

I can think of a few ways of doing it. It could be solved by adding a simple function like:

func split_lines(string x):
    if OS.get_name() == "Windows":
        return x.split("\r\n")
    else:
        return x.split("\n")

Or the change could be baked into the split function by adding a condition to the start of the function like:

    if delim == "\n" and OS.get_name == "Windows:
        delim = "\r\n"

Or a new variable could be added to OS class representing whatever newline delimitator is appropriate for the system, like: var split_lines = x.split(OS.NEWLINE) Personally, if I had to pick which one was added I'd go for the last one.

If this enhancement will not be used often, can it be worked around with a few lines of script?

It can be worked around when working with GDScript. Or, if making a C# project, the issue doesn't exist because C# already has a platform agnostic way to split lines.

Is there a reason why this should be core and not an add-on in the asset library?

A core aspect of Godot is that it is cross-platform. I interpret that as meaning that core Godot should be platform agnostic in every aspect, or at least in all aspects that are technically viable.

AThousandShips commented 5 months ago

And just normalizing the string isn't sufficient? I.e. my_string.replace("\r", "")?

Nallebeorn commented 5 months ago

I don't think making the behaviour OS-dependent is a good idea, that would lead to weird bugs of its own. It's not that uncommon to end up with files with LF line endings even on Windows, and it would make save files, config files etc. not be portable between platforms -- and even more annoying if your game downloads text from a server to process it.

I think it's better for a split_lines function to always split on either \r\n or \n (this is also how it works in for example Python and Rust).

Having it built in would be nice though. Even if its very easy to write yourself if you need it it, it's easy to miss that you do need it, when split("\n") is right there as the obvious solution.

AThousandShips commented 5 months ago

You can also just get each line from the file with get_line, it skips \r

Calinou commented 5 months ago

PHP has a newline constant that is \r\n on Windows and \n on other platforms, but as said above, files with LF line endings are very common on Windows nowadays. Many text editors write LF line endings even on Windows, since Windows 10's Notepad can display them correctly now.

Heros-Tempus commented 5 months ago

You can also just get each line from the file with get_line, it skips \r

The most embarrassing thing about this is I had already wrote a load_file function that reads in a plain text file returned all lines in an array using get_line way back when I was just concerned with reading in the items. I completely forgot about it when I got around to making it remove items at 1am and lost my mind trying to figure out why split("\n") would only sometimes work.