There are a few issues I can think of regarding how you parse the file intoadd/remove_from_file:

• Encoding. Non-ascii characters will be broken.
• End-line comments.
• Leading/trailing spaces.
• Non-normalized paths (//, \, /, , etc.).

I would also add a few items in regards to your use ofPath objects.

1. I would recommend usingPath.isfile() instead ofPath.exists(). Not that it's likely, butexists will returnTrue even if the path in question is a directory.
2. I would also preferGAME_PATH / file_path overGAME_PATH.joinpath(file_path), but that's really just a preference.
3. Another preference would be usingwith file_path.open('r') as file: instead ofwith open(file_path, 'r') as file:

With the last 2, I understand that they are slower, because they end up calling the other functionality internally. But, this isn't an operation that will be happening frequently, and the speed difference should be negligible.

* Encoding. Non-ascii characters will be broken.* Leading/trailing spaces.

Fixed.

* End-line comments.

Isn't that a tricky thing to do? As both "test #.vtf" and "test///test.vtf" are valid paths, we shouldn't allow end-line comments.

* Non-normalized paths (//, \, /, , etc.).

That's not a problem that should be solved by add_from_file/remove_from_file, but by Downloadables itself.
Also, if you normalize it,in/__contains__ won't work.

Besides, I think the downloadables string is a problem that should be managed by the plugin author/model distributor, and normalization will only add unnecessary problems.

1. I would recommend using `Path.isfile()` instead of `Path.exists()`.  Not that it's likely, but `exists` will return `True` even if the path in question is a directory.3. Another preference would be using with file_path.open('r') as file: instead of with open(file_path, 'r') as file:

This is intentionally to display an error, because there is nothing we can do as long as we don't know whether the specified file path starts with an absolute path, a relative path, or a path relative to GAME_PATH.

The reason I don't like Path.open is that it makes errors more confusing than they need to be. Instead of Path.open error, it now raises FileNotFoundError by default.

Isn't that a tricky thing to do? As both "test #.vtf" and "test///test.vtf" are valid paths, we shouldn't allow end-line comments.

This can be done with a simple regex. For example:

importrefrompathimportPathfrompprintimportpprintdefget_files(buffer):files=set()forlineinbuffer.splitlines():line=line.strip()ifline.startswith('//'):continuematch=re.match('(.*?)(?=\/\/ )',line)ifmatchisnotNone:line=match.group().rstrip()file=line.strip()ifnotfile:continuefiles.add(Path(file).normpath())returnfilespprint(get_files("""foo//bar/\\baz.mp3 // This is a cool song.maps/some map.bsp   // my_model.mdl    // Some empty linemyfile.txt    my     file.vtf // This material is trash.foo.wav // blah blah éééàà.mp3"""))

I discarded# because#foo.bar can be a valid file therefore should not be used to determine whether the current line is commented or not.

That's not a problem that should be solved by add_from_file/remove_from_file, but by Downloadables itself. Also, if you normalize it,in/__contains__ won't work.

Besides, I think the downloadables string is a problem that should be managed by the plugin author/model distributor, and normalization will only add unnecessary problems.

I don't think this should be the responsibility of theDownloadables class. That class is a straight bridge between the script and the table and the given strings should be added as is. However, by providing methods that add/remove strings for the script, you take responsibility and should provide consistency. Moreover, these methods are likely to be used to provides configuration files for the end users so we can't really assume scripters will properly format their files.

I discarded# because#foo.bar can be a valid file therefore should not be used to determine whether the current line is commented or not.

You're missing when a directory starts and ends with a space. And when there is no space after a comment.

"""foo//bar/\\baz.mp3 // This is a cool song.foo //bar/\\baz.mp3 // This is a cool song.foo// bar/\\baz.mp3 // This is a cool song.foo // bar/\\baz.mp3 // This is a cool song.foo//bar/\\baz.mp3 //This is a cool song.foo //bar/\\baz.mp3 //This is a cool song.foo// bar/\\baz.mp3 //This is a cool song.foo // bar/\\baz.mp3 //This is a cool song."""OutPut:{Path('foo'), Path('foo /bar/\\baz.mp3'), Path('foo /bar/\\baz.mp3 /This is a cool song.'), Path('foo/bar/\\baz.mp3'), Path('foo/bar/\\baz.mp3 /This is a cool song.')}

I don't think this should be the responsibility of theDownloadables class. That class is a straight bridge between the script and the table and the given strings should be added as is.

Remember thatDownloadables Class is also aSet. Anything added by add_from_file should be removed by remove(str), and vice versa.

However, by providing methods that add/remove strings for the script, you take responsibility and should provide consistency. Moreover, these methods are likely to be used to provides configuration files for the end users so we can't really assume scripters will properly format their files.

I simply thought it would be useful to be able to add and remove files in batches because some models have files in different locations in the directory hierarchy, so I didn't think that far ahead. But if that's the case, wouldn't it make more sense to normalize in all situations?

Why do you think normalization is necessary in the first place?\\ works fine on Linux/Windows without normalization, though.

If the downloadables don't work, they won't work after all, so I think it can only be the responsibility of the person who added them. For example, on Windows, lowercase and uppercase letters are treated as equal, so mixing them will work, but on Linux, it will not.

You're missing when a directory starts and ends with a space. And when there is no space after a comment.

That's because my regex doesn't have the right order. Should be(.*?)(?= \/\/) instead of(.*?)(?=\/\/ ) (or even better,(.*?)(?=\s\/\/)). The idea is that a file should not ever have a trailing space, meaning that if the pattern matchesfoo.mp3<space><fwdslash><fwdslash> this is a comment but if it matchesfoo.mp3<fwdslash><fwdslash> this is a continuation of the path.

Remember thatDownloadables Class is also aSet. Anything added by add_from_file should be removed by remove(str), and vice versa.

That's a great point, actually. Which lead me to agree with:

wouldn't it make more sense to normalize in all situations?

The goal is that paths are consistent and the resolution from the added strings are the same as the resolution of thefilesystem so yes, it would makes sense to have them normalized in theset itself so thatdl.add('foo/bar.baz') anddl.remove('foo\\bar.baz') handle the same path.

If you agree on the two changes I would be happy to merge into master.

I also wouln't add the possibility to add comments at the end. They have to be at the beginning of a line and that's it. No need to make it more complex.