EngineerSmith / Export-TextureAtlas

Extends the Runtime-TextureAtlas library and allows it to be exported as file
MIT License
6 stars 1 forks source link

Report and Requests #1

Closed flamendless closed 2 years ago

flamendless commented 2 years ago

So far everything works good in my side (Linux). Awesome!

Here is my bash script

function create_atlas()
{
    exported_path=./res/exported
    exported_dirs=(intro kitchen living_room outside storage_room utility_room)
    out_dir=./res/exported/out/
    eta_path=./libs/ExportTextureAtlas/

    for in_dir in "${exported_dirs[@]}"; do
        love $eta_path \
            $exported_path/$in_dir \
            $out_dir/$in_dir \
            -removeFileExtension \
            -throwUnsupportedImageExtension \
            -padding 4 \
            -template "./scripts/atlas_template.lua"
    done
}

Also, I found out that if the input dir has a .lua file inside it, it is ignored. Is that the correct behavior given the -throwUnsupportedExtension flag?

With this, it opens a lot of love window per iteration of the loop. Might I suggest the following (might be my nitpick and subjective to my usage, feel free to discourage if mine is not good):

EngineerSmith commented 2 years ago

Sadly I can't do much about the window opening, as a window is required to use love.graphics. I'll look into solutions to it, such as changing the size of the window from default and it's position to attempt to hide it.

I'll look into throwUnsupportedExtension, anything that isn't a file extension that cannot be loaded by love.graphics.newImage should be thrown. It was added at the start of the project, so I hadn't test it since then.

I'll look into adding input and output flags, then a numerous of directories or file paths can be added afterwards, and the required arguments won't be needed (but still can be used). I'll change things around so the 2nd argument is no longer called outoutDir too as you'd be able to specify the name of the outputted file. An equal number of inputs and atlas outputs must be defined. e.g. both will work love . <inputDir> <outputAtlas Dir or file path> -outputQuadData <dir or file path> if outputQuadData is excluded it will be added to the same directory as the atlas output.

love . -input <dir> <dir> <...> -outputAtlas <dir or file path> <dir or file path> <...> -outputQuadData <dir or file path> <dir or file path> <...> (Note, you'll be able to specify more than one template too, if only one is supplied it use the same one throughout) (- outputQuadData ./foo/data.* * would be replaced with the template's extension)

These changes will allow to bake more than one atlas, but also pick file names than having them all as atlas.png and quad.*.

Finally, -ignore <dir or file path> <...> can be added, but it would ignore it for all input directories. I'll add it and make it noted in the documentation. *.foo would ignore all files with the extension of foo, or bar.* all files named bar, no matter their extension. dir/ ignores all in directory named dir (must end in a / or \), (if subdir within other directories called dir, it too would be ignored, ./dir/ would ignore it from the top level of the input directory)

I won't be able to add these changes instantly as I only have free time in the evenings on weekends, so these should be in by Monday morning.

flamendless commented 2 years ago

Sadly I can't do much about the window opening, as a window is required to use love.graphics. I'll look into solutions to it, such as changing the size of the window from default and it's position to attempt to hide it.

I am aware of that so i suggested passing in array of inputs and outputs so it will only opened once. Perhaps it's okay to set the width and the height of the window to 1px as a hack? haha

I'll look into throwUnsupportedExtension, anything that isn't a file extension that cannot be loaded by love.graphics.newImage should be thrown. It was added at the start of the project, so I hadn't test it since then.

Tbh this is fine, i think that other extensions should just be ignored even with the flag. Maybe just instead of throwing unsupported, just use the valid image extensions that are supported?

I won't be able to add these changes instantly as I only have free time in the evenings on weekends, so these should be in by Monday morning.

No rush! Im grateful for you taking the time in writing this lib for love framework. I'd be happy to test it always and integrate it in my project so there's a showcase for it in a complex and large game.

Other points youve said are good for me and i agree with them.

EngineerSmith commented 2 years ago

I'll look into throwUnsupportedExtension, anything that isn't a file extension that cannot be loaded by love.graphics.newImage should be thrown. It was added at the start of the project, so I hadn't test it since then.

Tbh this is fine, i think that other extensions should just be ignored even with the flag. Maybe just instead of throwing unsupported, just use the valid image extensions that are supported?

Yeah, it's mostly there because the flag used to be called handleUnsupportedExtension which would stop it from throwing when it came across an unsupported file. I might just remove it as you suggested and let unsupported files pass by and only throw if love.graphics.newImage throws. It is rather a long ugly flag that I can't imagine being used now that -ignore will be added

EngineerSmith commented 2 years ago

Updates: 2d4862437e10acd20b9ab14e51a9b40eed702b83

To dos:

flamendless commented 2 years ago

The window is now hidden too! partying_face (Tested on Windows)

works on Linux as well.

Question: how does one do multiple ignore?

Submitted a PR #4 for ignore not working properly with absolute paths (no wildcard)

EngineerSmith commented 2 years ago

Question: how does one do multiple ignore?`

-ignore ./foo/ ./bar/foo.png /meta/ should work for multiple ignores. Although you could theoretically do -ignore ./foo/ -ignore bar.png -ignore /metadata/ but I've never tested it, but my code should support it.

Submitted a PR #4 for ignore not working properly with absolute paths (no wildcard)

Thank you for the fix

flamendless commented 2 years ago

Okay my bad, ive tried the first one earlier but both of the files are inside the same string. Separating into multiple strings works.

Ive tested multiple -ignore flags but only last one got stored in the table internally.

EngineerSmith commented 2 years ago

Fixed the args, so they should work in multiples now

flamendless commented 2 years ago

Okay thanks. Will test later.

Maybe for inputs and outputs we could follow the

-in "a" "b" "c"
-out "a2" "b2" "c2"
//Or
-in "a" -in "b" -in "c"
-out "a2" -out "b2" -out "c2"

Convention for uniformity with ignore flag? 🤔

EngineerSmith commented 2 years ago

Yeah it would automatically work like that since my arg script splits and pulls together all the given arguments by itself. I'll also update my arg script to support " so directories with white spaces in them are supported. It's a useful script that I use for my personal project so my game can have flags and options for the client and server.

EngineerSmith commented 2 years ago

Updates: bde0824871da21235eaa363c52f55666a6f10ad1

Directories for output must end in / or \ or it will be mistaken as a file name. See last two examples below

    love . -input <dir1> <dir2> -output ./foo/
becomes
    ./foo/atlas1.png + ./foo/data1.lua,  ./foo/atlas2.png +  ./foo/data2.lua

    love . -input <dir1> <dir2> -output ./foo
will error due to not being able to use a file as an output directory for multiple inputs

    love . -input <dir1> <dir2> -output ./foo ./bar.png
becomes
    ./foo.png + ./data1.lua, ./bar.png + ./data2.lua

    love . <inputDir> ./foo
becomes
    ./foo.png + ./data.lua -- Note how by missing the forward slash, it changes the directory and file completely

    love . <inputDir> ./foo/
becomes
    ./foo/atlas.png + ./foo/data.lua

To dos:

flamendless commented 2 years ago

Awesome. Will test later. Works good, awesome. It's nicer to see just one love window opening up for the entire process.

Found a problem with the outputs with this command (formatted nicely for easier reading)

love ./libs/ExportTextureAtlas/
    -input
        ./res/exported/intro/
        ./res/exported/kitchen/
        ./res/exported/living_room/
        ./res/exported/outside/
        ./res/exported/storage_room/
        ./res/exported/utility_room/
    -output
        ./res/images/atlases/intro/
        ./res/images/atlases/kitchen/
        ./res/images/atlases/living_room/
        ./res/images/atlases/outside/
        ./res/images/atlases/storage_room/
        ./res/images/atlases/utility_room/
    -ignore
        ./res/exported/intro/intro.png
        ./res/exported/kitchen/kitchen.png
        ./res/exported/living_room/living_room.png
        ./res/exported/outside/outside.png
        ./res/exported/storage_room/storage_room.png
        ./res/exported/utility_room/utility_room.png
    -removeFileExtension
    -padding 4
    -template ./scripts/atlas_template.lua

the outputs are in separate directories as intended with the -output flags but the filenames inside each directory has a number suffix like ../intro/atlas1.png + ../intro/data1.lua, ../kitchen/atlas2.png + ../inro/data2.lua, and so on.

Hmmm i dont think relying on slash at the end of the arg to determine its type is a good thing. Seems too confusing and complex. Might i suggest either:

I think both are easier to maintain in the long run as well 🤔 but then again maybe it's just my nitpick. The latter is how, i believe, CLI in Unix handles it 🤔

EngineerSmith commented 2 years ago

I'm unsure how to fix the issue with the appending the number on the files. There's numerous problems with it, and ways I could fix it; however, I think its fine as a basic backup. Once -outputData is a flag, I image that it won't be an issue that shows itself as the file names can be defined by the user.

EngineerSmith commented 2 years ago

Update: b1f5060f4c069191f13a5c6e8203cdcba5417180

-dataOutput works exactly like -output but for the quad data. You can specify a single directory to redirect all data<i>.lua to that directory, or to individual directories, or give individual file names.

To dos Ideas:

flamendless commented 2 years ago

Yep, the changes work! Awesome.

flamendless commented 2 years ago

In my opinion the lib right now is very robust and powerful for complex projects such as mine. It's really awesome and helpful to have this added to my workflow and build script. Thanks a lot!

EngineerSmith commented 2 years ago

In my opinion the lib right now is very robust and powerful for complex projects such as mine. It's really awesome and helpful to have this added to my workflow and build script. Thanks a lot!

Np, I learned about how to template files/strings in Lua, and improving my runtime texture atlas for my own project. So it's been beneficial for me too. If something does come up, just open a new issue

flamendless commented 2 years ago

just gonna post my script for generating atlases if ever anyone needs a reference:

#build.sh
#run with ./build.sh create_atlas
function create_atlas()
{
    eta_path=./libs/ExportTextureAtlas/
    output_path=./res/images/atlases
    source_path=./res/exported #exported from aseprite files

        #from ./res/exported/intro, ./res/exported/kitchen, ... and so on.
        #improvement will be to get each subdirectories in ./res/exported/ and automatically populate the array
    exported_dirs=(intro kitchen living_room outside storage_room utility_room)

    input_dirs=()
    output_dirs=()
    data_dirs=()
    ignores=()

    for cur_dir in "${exported_dirs[@]}"; do
        in_dir=$source_path/$cur_dir/
        out_dir=$output_path/$cur_dir.png
        data_dir=./src/atlases/atlas_$cur_dir.lua
        ignore=$source_path/$cur_dir/$cur_dir.png

        input_dirs+=($in_dir)
        output_dirs+=($out_dir)
        data_dirs+=($data_dir)
        ignores+=($ignore)

        echo "gen atlas: '$in_dir' -> '$out_dir' + '$data_dir' !'$ignore'"
    done

    love $eta_path \
        -input "${input_dirs[@]}" \
        -output "${output_dirs[@]}" \
        -dataOutput "${data_dirs[@]}" \
        -ignore "${ignores[@]}" \
        -removeFileExtension \
        -padding $padding \
        -template "./scripts/atlas_template.lua"
}