TDM Packer - a python script for packing TDM missions

[Skaruts]

I've just created a little python script to make packing missions a bit more convenient. It zips everything into the pk4 for you, while also excluding unwanted files that you list in a .pkignore file.

https://github.com/Skaruts/tdm_packer

It's in an experimental state. It should work fine, but there's probably things missing that I couldn't think of. 

 

Edited July 25 by Skaruts

[OrbWeaver]

Is it using the 7zip module just to create a better-compressed ZIP, or is it actually creating a 7z-formatted file using the LZMA algorithm?

If it's an actual 7z file, will the game actually load it? The packaging section on the wiki suggests that only ZIP format is supported (although it could be out of date).

[Skaruts]

Hmm  let me investigate. I packed my BTC mission with 7z and it worked fine, but I confess I haven't tested any of the archives created by this script. I didn't think this could be a problem.

Gonna check...

(And yes, the goal was better compression.)

Edited July 7 by Skaruts

[Skaruts]

@OrbWeaver, indeed 7z LZMA compressions don't work. I'll remove it from the script then.

[stgatilov]

On 7/7/2024 at 3:46 PM, OrbWeaver said:

Is it using the 7zip module just to create a better-compressed ZIP, or is it actually creating a 7z-formatted file using the LZMA algorithm?

If it's an actual 7z file, will the game actually load it? The packaging section on the wiki suggests that only ZIP format is supported (although it could be out of date).

Actually, it only matters on your local machine today.
And if you share your pk4 with someone directly.

When you release a mission, you send archive to e.g. @nbohr1more, he unpacks all contents and commits them into mission SVN. Then TDM server automatically packs it into pk4 and this pk4 goes into mirrors.

[Skaruts]

I renamed this tool, to be a bit more specific. 

I've been working on a GUI based version of this, which already has more functionalities, and it's easier to use.

[OrbWeaver]

A few initial observations of the code:

The README says you can just run the script as a command ("dmpak .") however this won't work on Mac or Linux. In order for it to work the script needs to start with an interpreter specification line like #!/usr/bin/env python3 and the script needs to be made executable by the user with chmod +x dmpak.py. After this the user can run it as ./dmpak.py but it needs both the "./" and the file extension.

Instead of using functions like os.path.join you should use the new PathLib module.

class Task:
	type = ""
	arg = ""
	def __init__(self, type, arg=""):
		self.type = type
		if arg:
			self.arg = arg

You have defined class variables type and arg which serve no obvious purpose, and are shared with all instances of the class (equivalent to static member variables in C++). Python does not require instance variables to be declared in the class body (and there is no way to do so), so if these are just intended to be instance variables, you should remove the class variable assignments and just assign the instance variables in the __init__ method directly.

Also note that type is a built-in function in Python and generally should not be used as a variable name.

	i = 0
	while i < argc:
		string = argv[i]
		... 
		i += 1

This is how a loop would be written in FORTRAN or C, but is not idiomatic Python. It would be more usual (and much shorter) to write it like this:

	for string in argv:
		... process string

However, you shouldn't write your own CLI argument parsing function at all, instead you should use the argparse module which will parse the arguments, detect errors and generate the help text for you automatically.

	if len(tasks) == 0 and fm_path != "":
		add_task(PACK_FILES)

Do not use len just to test if a list is empty, it is inefficient (as it must iterate over the whole list just to count the items). Instead, use the fact that an empty sequence is false, with:

if fm_path and not tasks:
	...

As the above code shows, you can do the same with strings, since a string is a type of sequence.

[Skaruts]

Thanks for the feedback. 

3 hours ago, OrbWeaver said:

The README says you can just run the script as a command ("dmpak .")

... on some systems.  

I thought I read somewhere that the interpreter comment wasn't needed anymore in python 3. Maybe I'm remembering wrong. I don't use python often enough to remember things...

If this is actually required, then I'll add it in, ASAP.

 

3 hours ago, OrbWeaver said:

Instead of using functions like os.path.join you should use the new PathLib module.

Thanks. I didn't know that module existed.

 

3 hours ago, OrbWeaver said:

You have defined class variables type and arg which serve no obvious purpose, and are shared with all instances of the class (equivalent to static member variables in C++).

Thanks again. This was force of habit from coding in GDScript in Godot. Tasks aren't really being useful in this script (at least not yet), as there's only ever one task, so maybe that's why I didn't notice any issues.

(I think tasks should've been a queue, as well.)

Tbh, I don't really care about using built-in functions as member variables, or in small ephemeral scopes. Under those circumstances it should never cause any conflicts (unless python has some pitfall that I'm not aware of that makes it so).

 

3 hours ago, OrbWeaver said:

This is how a loop would be written in FORTRAN or C, but is not idiomatic Python.

But it was the way I needed it, in order to support more complicated args. This is code I brought in from another script where the index is advanced manually depending on the parameters of each argument.

But yea, I might as well simplify it in this script, though. I'm intending to keep this tool simple to use.

I've been aware of argparse, but since I never do anything too complicated, I figured it would take me longer to learn it than it would to parse args with some simple code. I confess I'm not too patient to learn python, since I don't use it much.  

I've been intending to dive into argparse at some point...

 

3 hours ago, OrbWeaver said:

Do not use len just to test if a list is empty, it is inefficient (as it must iterate over the whole list just to count the items).

I didn't remember that about len. Although the string part is intentionally explicit. I personally don't like having empty string comparisons hidden. I prefer explicit code overall. If there's a more explicity alternative to `not tasks`,  would prefer it.

 

[OrbWeaver]

.

[OrbWeaver]

1 hour ago, Skaruts said:

... on some systems.  

I don't know of any system on which your Python script could just be invoked as "dmpak". Mac and Linux will not remove file extensions because there is no such concept; the extension is just part of the name and there is no equivalent of ".exe" to indicate an executable file. As far as I know Windows will not run a Python script by just typing the file name without an extension, but I don't develop on Windows so I could be out of date.

1 hour ago, Skaruts said:

I thought I read somewhere that the interpreter comment wasn't needed anymore in python 3. Maybe I'm remembering wrong. I don't use python often enough to remember things...

The interpreter line has nothing to do with Python or the Python version. It is there to indicate to the shell what command to run in order to parse your script. Without that line the shell will have no idea that your script is a Python 3 script, and the Python 3 interpreter will never even be started. The interpreter line is not needed if you always start the script explicitly with "python3 dmpak.py"; it's only there to allow you to run the script directly as an executable from the shell.

1 hour ago, Skaruts said:

Tbh, I don't really care about using built-in functions as member variables, or in small ephemeral scopes. Under those circumstances it should never cause any conflicts (unless python has some pitfall that I'm not aware of that makes it so).

Well it's up to you. If you don't care about syntax highlighting randomly highlighting some of your variables in a different color (or you don't use an editor/IDE with highlighting at all), and you don't ever intend to call the conflicting function, I guess it doesn't matter (and it won't cause a Python error in this case). But note that a public member variable is very far from a "small ephemeral scope", since it could be referred to by any amount of downstream code which uses the class.

1 hour ago, Skaruts said:

I've been aware of argparse, but since I never do anything too complicated, I figured it would take me longer to learn it than it would to parse args with some simple code.

Maybe, but the resulting code will be larger, more complicated and bug-prone if you needlessly re-invent the wheel, so the effort of learning argparse is hardly wasted. Getting a simple ArgParser up and running is a few lines; far fewer than the dozens of lines you already have to manually parse arguments, split strings, write out a large help text (which needs to be manually kept in sync with the actual arguments), etc.

The only time I wouldn't use argparse is if the script is so simple it's just taking a single argument, e.g. "process.py /path/to/inputfile". But your script is already way past that level of complexity.

1 hour ago, Skaruts said:

I personally don't like having empty string comparisons hidden. I prefer explicit code overall.

I'm not sure what you think is "hidden" — the Python semantics are very clear, a non-empty sequence is True and an empty sequence is False. But you're right that it is just a stylistic issue, and some people do think that "len(seq) == 0" is clearer in expressing intent. Just remember that the performance considerations do exist, although in your case the size of the sequence is small so it is unlikely to make a difference (it's more of an issue of you have a list of a million points and you want to check that it's empty after processing each point, or something like that).

Another advantage of "if not x:" is that it will also correctly handle the case where x is None, whereas "len(None)" will cause an immediate crash. Again, probably not an issue in your case but it's something to be aware of.

[Skaruts]

2 hours ago, OrbWeaver said:

As far as I know Windows will not run a Python script by just typing the file name without an extension

At least Windows 7 does. It's how I've always ran python scripts. I wasn't specific in the readme just because I don't know what other systems -- if any -- can do that too. I just wanted to keep the instructions as clear as possible.

So if I'm understanding correctly, that comment allows you to omit the explicit python call on Linux?

(Btw, I think I was confusing it with another comment that people used back in python 2, which iirc was no longer needed in python 3, so nevermind that.)

 

2 hours ago, OrbWeaver said:

Well it's up to you. If you don't care about syntax highlighting randomly highlighting some of your variables in a different color

Yea, the syntax highlighting can be a little annoying, but when it's a small thing like that, it can bother me more to have to rethink it, than to just go with it. Also by ephemeral scope I just meant the init function. Being a member variable means you need to do *task.type*, so I don't see it ever being a problem.

There is a part of me, however -- the highly responsible lua coder part of me -- that is telling me I shouldn't open lazy exceptions, especially in a language with too many pitfalls, like python...  

 

2 hours ago, OrbWeaver said:

Getting a simple ArgParser up and running is a few lines; far fewer than the dozens of lines you already have to manually parse arguments, split strings, write out a large help text (which needs to be manually kept in sync with the actual arguments), etc.

That's actually very true.  One of these days I'll give it a go, then.

 

2 hours ago, OrbWeaver said:

I'm not sure what you think is "hidden" — the Python semantics are very clear,

I'm thinking in terms of what is instantly clear to the eyes and requires no brain effort to parse. When possible, I like to write code that way. If I see `== ""`, I don't have to spend a moment determining what exactly is the condition being evaluated, or in some cases what the variable even is. Especially when I look at the code in six months. 

In this case the difference is probably negligible, but it's a personal convention. I think the only conditions I ever use implicitly are the booleans.

Edited July 25 by Skaruts

[OrbWeaver]

15 hours ago, Skaruts said:

So if I'm understanding correctly, that comment allows you to omit the explicit python call on Linux?

Correct. The user still has to set the executable flag on the script using chmod +x, but once they've done that, the shell will run it as an executable if it has the interpreter specification line.

[Skaruts]

Added the interpreter comment. Also fixed the built-in function shadowing thing, because it started bugging me since you got me thinking about it.  

The rest of the stuff I'll eventually get to it.

Edited July 26 by Skaruts