Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secondary Skin file format #2271

Open
LordSk opened this issue Oct 20, 2019 · 14 comments
Open

Secondary Skin file format #2271

LordSk opened this issue Oct 20, 2019 · 14 comments

Comments

@LordSk
Copy link
Member

LordSk commented Oct 20, 2019

0.7 introduced a new skin system, separating each body part to allow deeper customisation by combining them. This was a good decision I think, and most players love it.

The other day, @iTzMeRafa published their new website for sharing teeworlds tee skins, https://teeskins.de. I was excited to download new 0.7 skins, and... There are none. And it feels obvious why. We gained deeper customisation with the new system but lost the ability to quickly share skin files. One has to make and share several pngs and perhaps jsons, whereas before a simple png image would suffice.

I propose we implement a secondary way of loading skin parts, all in one file.

skin_sheet_no_bg
skin_sheet

Columns represent the different body parts : body, decoration, marking, eyes, feet, hands, in that exact order. The file can omit columns starting from the end. That way a file can only be composed of body, decoration, marking, for example. And of course each line is an item.

This should be relatively simple to implement, and help with the issue of 0.7 skin sharing.

EDIT:

  • The file can have an X amount of lines (like a table).
  • Once such a file is loaded, we could create a default skin combination using the first line of each item (this would help users test/equip their skins quicker).
@AssassinTee
Copy link
Contributor

I see a few problems there: the content of the jsons is bigger and can't be implemented into a single png file easily.

For example default colors, or using skin parts from other skins

@jxsl13
Copy link
Contributor

jxsl13 commented Oct 20, 2019

Good idea. How about omitting any skin part you want? it would maybe create bigger files, but would make this format more customizable.
like, the pngs would always have the same width, but grow from the top to the bottom.
One could maybe save the count of specific parts in the meta(exif?) data. Empty columns would stay transparent.

problem could be the names of the skin parts maybe?

@AssassinTee
Copy link
Contributor

Teeworlds could add an option to export and import skins (in zip format).
So you don't need an extra Format and sharing skins is easier.

@LordSk
Copy link
Member Author

LordSk commented Oct 20, 2019

I see a few problems there: the content of the jsons is bigger and can't be implemented into a single png file easily.

For example default colors, or using skin parts from other skins

The idea here is to only share skin parts. If one would also want to share a skin combination with it, they would have to share a .json. We actually could make a default one, with the first line of each part (but it would be "white").

Good idea. How about omitting any skin part you want? it would maybe create bigger files, but would make this format more customizable.
like, the pngs would always have the same width, but grow from the top to the bottom.
One could maybe save the count of specific parts in the meta(exif?) data. Empty columns would stay transparent.

problem could be the names of the skin parts maybe?

Growing from top to bottom (like a table) is the idea, yes. I'm not sure how omitting specific columns would translate in file size, but we could certainly test it.

@Dune-jr
Copy link
Member

Dune-jr commented Oct 20, 2019

Sounds great

The idea here is to only share skin parts. If one would also want to share a skin combination with it, they would have to share a .json. We actually could make a default one, with the first line of each part (but it would be "white").

Indeed, auto-creating a skin with the first line of the file would make sharing skins as simple as in 0.6 :)

@oy
Copy link
Member

oy commented Oct 22, 2019

Well, that's not a simple task, every skin part has to provide a name so other players can see/use them.
If you just want to share skin parts, providing the png's is already the simplest way.
If you want to provide a skin, like in 0.6 times, you can provide the json when using standard parts. For skins that use custom skin parts, we could add some kinda container. Similar to our maps, where we put the json(actual skin) and "external" skin parts used by that skin.

@AssassinTee
Copy link
Contributor

Well, that's not a simple task, every skin part has to provide a name so other players can see/use them.
If you just want to share skin parts, providing the png's is already the simplest way.
If you want to provide a skin, like in 0.6 times, you can provide the json when using standard parts. For skins that use custom skin parts, we could add some kinda container. Similar to our maps, where we put the json(actual skin) and "external" skin parts used by that skin.

e.g export zip <.<

@LordSk
Copy link
Member Author

LordSk commented Oct 22, 2019

Well, that's not a simple task, every skin part has to provide a name so other players can see/use them.

If the image name is, let's say oy.png as an example, each part could be named oy_body1.png, oy_body2.png, oy_eyes1.png, etc. Now if you mean that a specific part could be at a different location (and thus get a different name), but is the same as another part elsewhere, we had that problem before as well. Same skins with different names. We could alleviate this issue by generating a hash from the part's pixel data, and name it that.

If you just want to share skin parts, providing the png's is already the simplest way.
If you want to provide a skin, like in 0.6 times, you can provide the json when using standard parts. For skins that use custom skin parts, we could add some kinda container. Similar to our maps, where we put the json(actual skin) and "external" skin parts used by that skin.

This is an orthogonal direction I think. We would have to make a skin editor, which would only add steps to skin creation. It would help with skin sharing though, I agree on that.
But the simplicity of just sharing a .png has great value in my opinion.

e.g export zip <.<

We would need to add a zip library dependency then, do you have one in mind?

@AssassinTee
Copy link
Contributor

AssassinTee commented Oct 22, 2019

We would need to add a zip library dependency then, do you have one in mind?

Teeworlds already has a zlib dependency

@Dune-jr
Copy link
Member

Dune-jr commented Oct 23, 2019

KISS; plenty of people already have no idea how to deal with a .json, or even figure that it is a text file.

I don't even think we need several rows. You can even design that in the 0.6 format. bear.png will give the bear body part, the bear hand part, etc. Simple stuff.

It's not a big deal if there is some transparent space, 0.6 skins had a ton of redundancy (repeat default parts) and they were still tiny in size compared to the rest of the Teeworlds assets.

PS: skin hashes is #826 @LordSk

@oy
Copy link
Member

oy commented Oct 24, 2019

There already is a skin editor and you can save the skin you built. That will create the JSON file. Nobody has to create/edit some file by hand. Should be possible to automatically add/include the non-standard skin parts to make it as simple as possible for the user.
If everything is within a PNG, the auto part renaming isn't that good and doesn't match the original names. Of course hashes (#826) will come in handy though it probably should still use name+hash.
Another problem are the colours which are attached to every skin part which aren't part of the PNG either.

@Dune-jr
Copy link
Member

Dune-jr commented Oct 25, 2019

Either way, the current state is not desirable and limits greatly user content. I am not sure what is the best course of action, but I strongly believe something should be done to ease the creation and sharing of skins.

There is a loss of flexibility (naming schemes, attached colours) in @LordSk's proposed new format indeed, though standard skins would still be able to use the current, advanced skin format. I'd like to see any other proposal.

@oy
Copy link
Member

oy commented Oct 26, 2019

Well, options are: to convert the non standard skin part image data to base64 and include it in the JSON file. So a user who lacks that part could use that one.
Or: use a container like mentioned above. Either a custom one or a zip based one like @AssassinTee mentioned.

heinrich5991 pushed a commit to heinrich5991/teeworlds that referenced this issue Jun 19, 2020
2276: Fix segmentation fault when creating sql tables failed r=heinrich5991 a=Zwelf

`GameServer()->OnShutdown(true);` (which would wait on running threads to exit)
wasn't called if an error occured in the server init. Therefore `CServer` was
freed before the init-thread finished executing, causing a use after free from
`CSqlConnector::ms_ppSqlReadServers` pointing to `CServer->m_apSqlReadServers`.

Fixes teeworlds#2271 

Note that the server exits when creating tables failed. Since the tables are created by default, it has to be disabled explicitly (e.g. `add_sqlserver r teeworlds record teeworlds PASSWORD "localhost" 3306 0`) if the servers should start even when the database connection fails.

Co-authored-by: Zwelf <zwelf@strct.cc>
@Robyt3
Copy link
Contributor

Robyt3 commented Mar 27, 2022

My roadmap:

  • Base64 encoding/decoding
    • Base64 encoding adds around 33% overhead to the skin container file size, but it's easier to integrate as we already have json skin files.
    • Encoding/decoding gzip with zlib seems like a hassle so I didn't try. For windows users, the gzip format is also inconvenient, as they lack the default software to open gz files.
  • Embed base64 encoded skin parts in json files when saving
    • Only embedded non-vanilla parts. Automatically detect vanilla parts so the process is opaque for the user.
  • Load base64 encoded parts from json files, if the part could not be loaded by filename.
    • [?] Unpack embedded skin parts that a user doesn't have into their skin parts folders as png images. It's not clear to me whether you want this or not. Skins could be shared more easily either way, when the parts are embedded in the json files. While unpacking the skin parts would allow other players to more easily use parts of other skins, it would further raise the issue of skins parts with the same name but different hash. I think if users want to share skin parts and not whole skins, they should do just that and share the individual png files instead.
    • On the other hand, if skins parts are not unpacked into png files, then they need to be kept in memory to allow the user to save skins containing embedded parts again. Not sure if we want to do that. We could also allow embedded parts to only be used in the skin that they are embedded in and additionally disallow saving skins with embedded parts.

RFC!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants