Texture Packer page indexes

[Frosty-J]

Don't be deceived by the number of changed lines. Only lines 204 and 238 in TexturePacker.java actually change any functionality. But those lines make this a breaking change! At least, I think they do - the linked issue was labelled as breaking.

[That opening paragraph is now outdated. It is no longer a breaking change (meaning we're stuck with the old behaviour by default FOREVER) and now a couple more lines do stuff.]

It aims to fix #6239 by adding an index to the first texture page. Just like previously, an index isn't added if the atlas is only one page long.

No-one asked for this, but I also made the index start at 0 instead of 1. My reasoning behind this is it's easier to iterate over your textures and all that this way - no need to have + 1 all over the place. It wouldn't be worth changing this on its own, but seeing as this would still be a breaking change without it....

There's also a little bit of tidy-up to reduce warnings about redundancy and whatnot, but it's a very minimal effort.

P.S. I actually have Git configured properly now - there'll be no more "Frosty-J and Frosty-J committed" business.

P.P.S. Now it's configured even better. Verified commits!

[tommyettinger]

I think this doesn't have to be a breaking change if it is made configurable.

[Frosty-J]

@tommyettinger Did you know there's a legacyOutput option? This defaults to true and isn't documented on the wiki. Results in a smaller atlas file. I guess you already do, since you commented at #6316.

I'm going to add that legacyPages flag and might revert some of the other changes - depends if anyone else chimes in on the matter, and if they don't it's a figurative coin flip.

I should go to bed - marking this as draft so someone doesn't go merging it in its current state, even if I end up opening it again tomorrow.

[tommyettinger]

@tommyettinger Did you know there's a legacyOutput option? This defaults to true and isn't documented on the wiki. Results in a smaller atlas file. I guess you already do, since you commented at #6316.

Indeed I do know, I set legacyOutput to false for the large atlases of Dawnlike tiles! I should probably edit that wiki.

[Frosty-J]

Tested as working. Marking as draft was overkill, in hindsight.

[NathanSweet]

Needs parenthesis to make it clear.

[NathanSweet]

Sorry, but I don't really see the usefulness of this setting. It's not difficult to read files with the naming before this PR. Most people use TextureAtlas to do the reading anyway, so the file names are unimportant.

IMO for any feature, ideally default behavior should be found so a setting is not needed. Settings should only be added where necessary. This simplifies using a tool and forces putting in the effort to make the tool work well without a user needing to understand hundreds settings. Look at Maya or similar software for how not to do it.

Just like previously, an index isn't added if the atlas is only one page long.

The packer doesn't know how many page images there will be. It may pack one page the first time, then later be run again, append to the atlas file and pack additional pages.

No-one asked for this, but I also made the index start at 0 instead of 1.

When naming files I prefer image1.png, image2.png, etc. image0.png just looks ugly to me personally.

It's common that an atlas has only one page image, in which case image1.png looks worse than image.png. However, I would vote to not merge this PR and to close #6618 as a non-issue.

[Frosty-J]

Settings should only be added where necessary.

I would prefer to not use a setting, but that would make this a breaking change and not everyone is a fan of that. Meanwhile, I'm not a fan of discovering that we're already using a legacy output mode by default just for the sake of ensuring everything keeps ticking along unchanged.

The packer doesn't know how many page images there will be.

If you saw what this paragraph said previously, forget about it. I've now tested running it after adding and removing images, and the previous output gets deleted. I don't see under what circumstances future runs could pose an issue.

image0.png just looks ugly to me personally.

Could fix this with another setting. Haha, just kidding. Yeah, it's divisive. Trivial to change back to being one-indexed if that's the consensus.

It's common that an atlas has only one page image, in which case image1.png looks worse than image.png.

Agreed. In the event it only has one page, it would be image.png either with or without this pull request.

[NathanSweet]

The packer doesn't know how many page images there will be.

TexturePackerFileProcessor processes a directory hierarchy of image files. It first deletes all output files, then creates and runs a TexturePacker for each directory:
https://github.com/libgdx/libgdx/blob/master/extensions/gdx-tools/src/com/badlogic/gdx/tools/texturepacker/TexturePackerFileProcessor.java#L282-L285
Each run appends to the output of the previous runs. Checking the number of pages in the current run is insufficient to know if the final atlas will have only one page. We'd need to make that determination before running the packer the first time.

I greatly appreciate your enthusiasm, but I'm not convinced this needs changing. Does it really matter if the images are named image.png, image2.png, etc? If we can have image.png for one image and image1.png, image2.png, etc when there are multiple images, then I could agree that is an improvement. Doing that will take more effort though, given the above. Is it worth it? Can it be done easily? If it can, I'd prefer not to have a setting. We could have the new logic only when legacy is false, which probably affects fewer people.

[Frosty-J]

Thanks for the code link and explanation. You're right - this does require more thought. And due to how it goes along one subdirectory at a time like that, I can't currently see how to solve this.

It doesn't need changing, though 5 people agree with the original issue with no disagreements, so it would certainly be nice to have.