JarateKing / CleanTF2plus

Clean TF2's sequel
MIT License
118 stars 14 forks source link

Linux refactor #29

Open maplepy opened 1 year ago

maplepy commented 1 year ago

I just skimmed over the bash script and good god... This seems very hard to maintain and not verbose at all. Do you want me to submit a PR to fix it ?

Also

FLAT_GEN() {
    echo would you like flat textures resized? Y/N/HELP
    read choice

    if [ $choice == "Y" ]; then
        flat=2
    elif [ $choice == "N" ]; then
        flat=1
    elif [ $choice == "HELP" ]; then
        echo resized flat textures appear mostly flat on sv_pure, non-resized appears as stock tf2 textures
        FLAT_GEN
    else
        echo invalid input
        FLAT_GEN
    fi
}

say the answer is no, so flat=1

elif [ $flat == 2 ]; then
    echo generating flat textures
    dev/generators/textures_flat.sh dev/lists/linux/flat.txt "../../tf2_textures_dir.vpk" 1
    dev/generators/textures_flat.sh dev/lists/linux/flat_hl2.txt "../../../hl2/hl2_textures_dir.vpk" 1
    echo flat textures \(resized\) >>dev/current_options.txt
    echo done
fi

why is this being done?

maplepy commented 1 year ago

Moreover, what is this supposed to do?

#!/bin/bash

BINPATH=$(dirname $0)/../../../../../bin
LD_LIBRARY_PATH=$BINPATH $BINPATH/vpk_linux32 $*
JarateKing commented 1 year ago

Greetings! I don't usually deal with batch/bash by trade and most of the bash version was largely @Pyogenics' effort porting my batch script. There isn't much architectural decision-making beyond "get it working." Feel free to refactor it! Any help is greatly appreciated.

For your specific question, the intention is that flat will be set by one of:

  1. the user doesn't want flat textures. They say no (in the FLAT() function), and get prompted for the next option (nohats). The flat variable is 0 in this case, because it hasn't changed from default.
  2. the user wants flat textures. They say yes, and then they get asked if they want to resize the textures. They say no, so textures don't get resizes and will look default in sv_pure 2. The flat variable is set to 1 in this case.
  3. the user wants flat textures. They say yes, and then they get asked if they want to resize the textures. They say yes again, because they do want 1x1 textures that'll look mostly flat in sv_pure 2. The flat variable is set to 2 in this case.

Then when we get to the end of the script, we check what value flat is and generate things based on that. If flat == 1, the dev/generators/textures_flat.sh script will be run with only the list of files to use. When flat == 2, we also pass in 1 as an argument to tell it to resize them to 1x1 images.

The other script you're talking about uses vpk_linux32 found inside tf/bin (which is a few directories up from the script) to turn the CleanTF2plus folder into a vpk file. This is primarily to make it easier to distribute and potentially faster to load (since folder mods are much slower for TF2 to load than vpk mods)

I'm not sure if there's something I'm missing or misunderstanding about your questions, so hopefully that clears things up.

Pyogenics commented 1 year ago

I tried to port the batch to bash as 1:1 as possible, so you'll even find comments in similar locations. Logic being it makes it easier to make changes to both batch and bash sections when needed, although I agree that the bash is probably not the best written in this style. If you make any changes to any prompts or messages + any additional output (I assume you'll add more, e.g. more verbose) you probably should update the batch side too if you can.

Pyogenics commented 1 year ago

Abit of a side note but it would possibly be a good idea to have multiple processes running in parallel, like with flat textures step. Would speed up generation considerably, here's me running flat textures on my machine with a few other things open. Sat 22 Jul 13:44:50 BST 2023

Pyogenics commented 1 year ago

I was thinking about it but perhaps the entirety of this mod can be ported to powershell? Powershell runs both on linux and windows so we can have one set of scripts for both, and powershell is considerably more powerful than bash or batch. Thoughts?

maplepy commented 1 year ago
  • the user doesn't want flat textures. They say no (in the FLAT() function), and get prompted for the next option (nohats). The flat variable is 0 in this case, because it hasn't changed from default.

  • the user wants flat textures. They say yes, and then they get asked if they want to resize the textures. They say no, so textures don't get resizes and will look default in sv_pure 2. The flat variable is set to 1 in this case.

  • the user wants flat textures. They say yes, and then they get asked if they want to resize the textures. They say yes again, because they do want 1x1 textures that'll look mostly flat in sv_pure 2. The flat variable is set to 2 in this case.

Alright I'll probably simplify this so it is easier for you to maintain in the long run if you want

When flat == 2, we also pass in 1 as an argument to tell it to resize them to 1x1 images. Does that change anything from the default config or is it not doing anything in the real world ? (just trying to understand so the script is more efficient)

The other script you're talking about uses vpk_linux32 found inside tf/bin (which is a few directories up from the script) to turn the CleanTF2plus folder into a vpk file. This is primarily to make it easier to distribute and potentially faster to load (since folder mods are much slower for TF2 to load than vpk mods)

Alright I thought it was something to go back into the usr/bin or something which would break if users have the game on another drive but I understand the goal now :)

I have already fixed a few things in the main script to make it easier to maintain, will work on the rest today probably so I can submit that PR soon :)

maplepy commented 1 year ago

I tried to port the batch to bash as 1:1 as possible, so you'll even find comments in similar locations. Logic being it makes it easier to make changes to both batch and bash sections when needed, although I agree that the bash is probably not the best written in this style.

I am now using functions so you don't have to maintain tons of duplicate messages and logic.

What I thought of doing is display the help message straight away and make the answer easier (y or Y or Enter would accept the changes, n or N would deny the changes) image It could look like this what do you think? (see above screenshot)

To explain how it works:

  1. In the first question I pressed enter
  2. Pressed n
  3. Pressed y

If you make any changes to any prompts or messages + any additional output (I assume you'll add more, e.g. more verbose) you probably should update the batch side too if you can.

I am not sure if I know batch well enough to do this, also I don't have a Windows machine so I wouldn't be able to test it for now

maplepy commented 1 year ago

Abit of a side note but it would possibly be a good idea to have multiple processes running in parallel, like with flat textures step.

Sure, you used multiple scripts for this reason right?

Pyogenics commented 1 year ago

You'd have to ask JarateKing for that one, might be because of batch being like it is so it's easier to use separate scripts like functions.

maplepy commented 1 year ago

I was thinking about it but perhaps the entirety of this mod can be ported to powershell? Powershell runs both on linux and windows so we can have one set of scripts for both, and powershell is considerably more powerful than bash or batch. Thoughts?

Well I have never really used it whether that's on Windows back when I still used that O.S. nor on Linux as I find bash pretty easy to write and maintain but if you know Powershell sure :)

Pyogenics commented 1 year ago

I was thinking about it but perhaps the entirety of this mod can be ported to powershell? Powershell runs both on linux and windows so we can have one set of scripts for both, and powershell is considerably more powerful than bash or batch. Thoughts?

Well I have never really used it whether that's on Windows back when I still used that O.S. nor on Linux as I find bash pretty easy to write and maintain but if you know Powershell sure :)

I've done some powershell before, more of programming language than batch or bash, plus it will allow the use of C# assemblies so we can maybe even ditch dependencies on seperate binaries like hlextract to do the work and instead just have a library to do it, meaning we don't have a split between windows and linux code sections.

maplepy commented 1 year ago

I was thinking about it but perhaps the entirety of this mod can be ported to powershell? Powershell runs both on linux and windows so we can have one set of scripts for both, and powershell is considerably more powerful than bash or batch. Thoughts?

Well I have never really used it whether that's on Windows back when I still used that O.S. nor on Linux as I find bash pretty easy to write and maintain but if you know Powershell sure :)

I've done some powershell before, more of programming language than batch or bash, plus it will allow the use of C# assemblies so we can maybe even ditch dependencies on seperate binaries like hlextract to do the work and instead just have a library to do it, meaning we don't have a split between windows and linux code sections.

I haven't used C# yet so I'm afraid I wouldn't be able to help for the Powershell part :/

JarateKing commented 1 year ago

Abit of a side note but it would possibly be a good idea to have multiple processes running in parallel, like with flat textures step. Would speed up generation considerably, here's me running flat textures on my machine with a few other things open.

Agreed, that sounds like a great improvement.

I was thinking about it but perhaps the entirety of this mod can be ported to powershell? Powershell runs both on linux and windows so we can have one set of scripts for both, and powershell is considerably more powerful than bash or batch. Thoughts?

I wouldn't oppose it, but I'm even more unfamiliar with powershell than I am batch or bash.

Does that change anything from the default config or is it not doing anything in the real world ? (just trying to understand so the script is more efficient)

It changes two things:

Most people probably want it resized. But I don't want to make that decision for them, and it's easy enough to support both when they generate them themselves.

What I thought of doing is display the help message straight away and make the answer easier (y or Y or Enter would accept the changes, n or N would deny the changes)

That's a good idea to me, I don't know why I didn't do that the first time

Sure, you used multiple scripts for this reason right?

That was mostly just to separate things. generate.bat is already a fairly long script as it is. If it had to also include all the generator scripts, it would be very unwieldy.

I've done some powershell before, more of programming language than batch or bash, plus it will allow the use of C# assemblies so we can maybe even ditch dependencies on seperate binaries like hlextract to do the work and instead just have a library to do it, meaning we don't have a split between windows and linux code sections.

I did intentionally choose existing executables to use as dependencies, so that people didn't need to build them themselves but if they were worried about anything malicious they could easily check them against official sources.

fart.exe is just a utility to find and replace text (hence the name "fart") and would be really easy to code by hand. Imagemagick utilities are overkill for what they're used for, which is pretty much just for finding the average color of an image and setting an image to a single color. But I went with them because I didn't want your average user to have to compile anything or install python or anything like that, and I didn't want security-minded users to have to trust a random exe from a random person.

I'm not fixed on that though: if there are good reasons to use something else (like C#) for speed and maintainability reasons, I think that outweighs the other concerns.

Splarkszter commented 1 year ago

powershell

No powershell please, i ditched windows for a reason!!! (Also it isn't included by default on distros so it would just complicate things and add bloat to people's systems)

Pyogenics commented 1 year ago

powershell

No powershell please, i ditched windows for a reason!!! (Also it isn't included by default on distros so it would just complicate things and add bloat to people's systems)

Well we already depend on wine and imagemagic, they aren't included by default. Plus powershell is opensource and using it we can probably also, atleast, remove the wine dependency.

Pyogenics commented 1 year ago

I tried to port the batch to bash as 1:1 as possible, so you'll even find comments in similar locations. Logic being it makes it easier to make changes to both batch and bash sections when needed, although I agree that the bash is probably not the best written in this style.

I am now using functions so you don't have to maintain tons of duplicate messages and logic.

What I thought of doing is display the help message straight away and make the answer easier (y or Y or Enter would accept the changes, n or N would deny the changes) image It could look like this what do you think? (see above screenshot)

To explain how it works:

  1. In the first question I pressed enter
  2. Pressed n
  3. Pressed y

If you make any changes to any prompts or messages + any additional output (I assume you'll add more, e.g. more verbose) you probably should update the batch side too if you can.

I am not sure if I know batch well enough to do this, also I don't have a Windows machine so I wouldn't be able to test it for now

I might be able to port changes to batch? Would need to get a windows VM for some other windows dev stuff anyway. But batch is abit static and old, maybe atleast the windows side could be powershell? We'll see.

maplepy commented 1 year ago

yeah maybe on the windows side use powershell? I know I won't code it for Powershell myself though, I can do bash but that's pretty much it

Pyogenics commented 1 year ago

I could do the powershell side, port your changes into it for windows. If jarateking approves of powershell for the windows side, that is.

JarateKing commented 1 year ago

I see no reason not to

maplepy commented 1 year ago

Been pretty busy this weekend for my bday and will probably be a bit busy this week so i'll send the pr when i can :)

Pyogenics commented 1 year ago

Heya, just checking in. Its been a while but how is the refactor going?

maplepy commented 1 year ago

Hey! I have been having some personal complications lately so I couldn't really work on it and I haven't played tf2 since then so I will see if I can draft a PR in the future days even if its incomplete, just so you have something cleaner