Add properties on GameProfile for restore skin on MC 1.7.8 -1.7.9

[ralmn]

No description provided.

[mbax]

The versions of the client which disconnect upon receiving any 'invalid' skin data are still able to connect to 1.7.9 servers.

[ralmn]

the goal is to display the real skin of each player instead of a "steve" skin when their head name is coloured

[mbax]

Sure, but then you're disconnecting every player from your server who didn't update to the non-mandatory 1.7.9.

[kyriog]

We've just tried with vanilla 1.7.2 and 1.7.4 clients, and we have no problem to connect to our server, and we're not disconnected if somebody joins.

[mbax]

Try 1.7.6, 1.7.7, 1.7.8 with this PR.

[kyriog]

Those 3 versions have been retired by Mojang. They are only usable if we check the "snapshot" checkbox. I think those versions couldn't be considered as releases, don't you?

[kyriog]

Moreover, those 3 versions aren't anymore downloadable from official launcher, even if the "snapshot" checkbox is checked! They are playable only if you have previously downloaded them.

[mbax]

They are playable only if you have previously downloaded them.

Plenty of users manually change versions, and only do so when their favorite server changes (eg. your favorite server stayed back on 1.7.2 for a bit, and then let you know when to update, so you had it manually on 1.7.2 and then manually changed to a later 1.7). Thus, I suspect there are plenty of players still on those versions because there was no forced update via protocol break. Please test those versions if you want this PR getting through.

[kyriog]

I just make some tests using those client versions:

• 1.7.2: Works, no problem.
• 1.7.3: Never been released.
• 1.7.4: Works, no problem.
• 1.7.5: Works, no problem.
• 1.7.6, 1.7.7: Not tried, as they aren't downloadable anymore.
• 1.7.8: Broken, you're kicked right after joins (this version isn't downloadable anymore).
• 1.7.9: Works, no problem.

I don't think there are a lots of server who are using 1.7.6 -> 1.7.8, as there are too many bugs with skins.

[mbax]

My concern isn't servers running 1.7.6-8, it's clients on those versions. :(

[kyriog]

Well, I think you could agree that skins are an essential feature of Minecraft, right?
The fact is that with the current version, TagAPI is pretty much useless as changing players' names will display "Steve" skins.
So I think both servers and players will agree to update their Minecraft to get skins back, don't you think?

[ralmn]

Now it works on 1.7.8 version, I don't have 1.7.6 and 1.7.7 version to test

[mbax]

@kyriog I agree that skins are an essential feature, but the client disconnect made it pretty clear that skins are a bad idea. You unfortunately can't tell a player to upgrade unless you can determine their exact version before they join the game, as they'll just get disconnected with an irritating message. They may choose to visit another functioning server instead.

If the 'works on 1.7.8' code works, then that may not be a concern. Will check it out tonight if I get the chance.

[ralmn]

We have tested it sucessfully on 1.7.6 and 1.7.7, here are the client jars if you want to test it yourself :
https://s3.amazonaws.com/Minecraft.Download/versions/1.7.7/1.7.7.jar
https://s3.amazonaws.com/Minecraft.Download/versions/1.7.6/1.7.6.jar

[Byteflux]

Is this still a thing? I haven't been able to get this working in anything other than 1.7.2, 1.7.4 and 1.7.9, however, I know that the inbetween versions have mostly been phased out.

Mod support within the community is at 1.7.2 and 1.7.4 so that's a large chunk of players, while most other players will be using "latest game version" in their launcher profile which is 1.7.9.

Mojang fixed the whole skin thing with 1.7.9, so I feel strongly in the spirit of moving forward TagAPI shouldn't try to remain backwards compatible with pre-1.7.9 considering a negligible amount of players use any version between 1.7.4 and 1.7.9.

Regarding this PR, is it necessary to fetch the skin data remotely? I have modified my own fork to copy the properties from the old profile into the new one and it seems to work the same as this.

[mbax]

I kinda dropped the ball on testing. There were a few things I didn't like, that I'll need to re-review. For situations where it's the same name except color I suspect it'll work fine by just copying @Byteflux , but what about if you're actually changing the name, like changing everyone into Notch?

[Byteflux]

@mbax Just tested this and in all three mentioned versions, changing name to an uncolored and colored "Notch" worked without any errors or crashing.

In 1.7.2 and 1.7.4 the skin that's displayed is also a Notch skin. 1.7.9 sees the actual player's skin regardless of name.

[Byteflux]

Works in 1.7.5 too. 1.7.7 and 1.7.8 appear to be the only client versions obtainable from the launcher that get kicked.

[Byteflux]

So I've been running my fork (simply copies the properties from the old profile to the new one) all day today and have not experienced any issues nor have I seen a decrease in my player count which leads me to believe my conclusion regarding client version distribution is mostly correct.

This seems like a promising solution if you decide to pursue it. My repository is a bit mangled at the moment so I haven't actually made this into a PR but the code is pretty much a few lines:

In DefaultHandler:

if (newName != null && !newName.getName().equals(oldName)) {
    GameProfile oldProfile = (GameProfile)this.gameProfileField.get(p);
    GameProfile newProfile = new GameProfile(newName.getUUID(), newName.getName());
    for (Map.Entry<String, Property> oldProperty : oldProfile.getProperties().entries()) {
        newProfile.getProperties().put(oldProperty.getKey(), oldProperty.getValue());
    }
    this.gameProfileField.set(p, newProfile);
}

[vemacs]

Been running @Byteflux's fork for a week with no issues (and somehow not having people complaining about client incompatibility). I think with almost nobody using 1.7.6-8 clients, this really should be merged. It just provides a better experience.

[mbax]

Hi @vemacs ! Byteflux's fork works great as long as you want all players to always have their skin. I'm contemplating doing this solution instead, where I only set that skin if the color-stripped name is the same: https://github.com/KittehOrg/TagAPI/compare/fixskins

Alternately, there is the approach in this original PR, but I'm hesitant to do it for a series of reasons. That approach requires waiting on external URL connections. I could solve that by providing API for plugins to play with. However, I have no idea what route Mojang is going to take and am a bit worried about creating a pile of dead-within-a-month (or whenever they do 1.8) API. Still looking for input, and I still haven't made a final decision. At this point I am ok with murderously disconnecting 1.7.6-8 clients, though. At best I'll give a config option to disable skin setting.