| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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.
|
There are a few issues I can think of regarding how you parse the file into add/remove_from_file:
|
Sorry, something went wrong.
|
I would also add a few items in regards to your use of Path objects.
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. |
Sorry, something went wrong.
Fixed.
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.
That's not a problem that should be solved by add_from_file/remove_from_file, but by Downloadables itself. 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.
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. |
Sorry, something went wrong.
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.
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. |
Sorry, something went wrong.
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.')}
Remember that Downloadables Class is also a Set. Anything added by add_from_file should be removed by remove(str), and vice versa.
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. |
Sorry, something went wrong.
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. |
Sorry, something went wrong.
That's a great point, actually. Which lead me to agree with:
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. |
Sorry, something went wrong.
| raise FileNotFoundError(f'No file found at "{file_path}"') | ||
|
|
||
| with file_path.open('r', encoding=encoding) as file: | ||
| lines = [line.strip() for line in file.readlines()] |
There was a problem hiding this comment.
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))
}
Sorry, something went wrong.
There was a problem hiding this comment.
I fully agree on the normalizing part. At the moment normalizing isn't that important as no users are involved. But I think it's better to normalize. Especially with this PR.
dl.add('foo/bar.baz') and dl.remove('foo\bar.baz') handle the same path.
Sorry, something went wrong.
| The paths to add to the downloadables. | ||
| """ | ||
| if items is None: | ||
| return |
There was a problem hiding this comment.
What is the point of the None check here? A Python set would throw an error:
x = set()
x.update(None)
Traceback (most recent call last):
File "<pyshell#3>", line 1, in <module>
x.update(None)
TypeError: 'NoneType' object is not iterable
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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: