Enhanced Downloadables.

[CookStar]

Changed Downloadables to accept iterable on initialization.

Fixed Downloadables.update to correctly add downloadables to stringtable.

Added add_from_file/remove_from_file functions to Downloadables.

Examples:

from stringtables.downloads import Downloadables

downloadables = Downloadables(("downloadable_path_1.vtf", "downloadable_path_2.vtf"))

downloadables.update(("downloadable_path_3.vtf", "downloadable_path_4.vtf"))

downloadables.add_from_file("file_path.txt")

downloadables.remove_from_file("file_path.txt")

[jordanbriere]

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

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

[satoon101]

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

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.
2. I would also prefer GAME_PATH / file_path over GAME_PATH.joinpath(file_path), but that's really just a preference.
3. Another preference would be using with file_path.open('r') as file: instead of with 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.

[CookStar]

* 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.

[jordanbriere]

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:

import re
from path import Path
from pprint import pprint

def get_files(buffer):
    files = set()
    for line in buffer.splitlines():
        line = line.strip()
        if line.startswith('//'):
            continue
        match = re.match('(.*?)(?=\/\/ )', line)
        if match is not None:
            line = match.group().rstrip()
        file = line.strip()
        if not file:
            continue
        files.add(Path(file).normpath())
    return files

pprint(get_files("""
foo//bar/\\baz.mp3 // This is a cool song.
maps/some map.bsp
   // my_model.mdl
    // Some empty line
  
myfile.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 the Downloadables 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.

[CookStar]

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 the Downloadables class. That class is a straight bridge between the script and the table and the given strings should be added as is.

Remember that Downloadables Class is also a Set. 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.

[jordanbriere]

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 matches foo.mp3<space><fwdslash><fwdslash> this is a comment but if it matches foo.mp3<fwdslash><fwdslash> this is a continuation of the path.

[jordanbriere]

Remember that Downloadables Class is also a Set. 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 the filesystem so yes, it would makes sense to have them normalized in the set itself so that dl.add('foo/bar.baz') and dl.remove('foo\\bar.baz') handle the same path.

[jordanbriere]

Please use core.ConfigFile to parse the file. E.g.

lines = {
    Path(file.strip()).normpath()
    for file in set(ConfigFile(file_path, as_strings=True))
}