-
Notifications
You must be signed in to change notification settings - Fork 19
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
Cleanup of the skin changing system (WIP) [OLD] #63
Conversation
Important notes: - Haven't tested multiplayer - There's currently only 1 skin and it's a hardcoded pathname (nothing random) - Haven't tested the reset skin button - Haven't tested the fallback mechanism - Old classes moved to 'unused' folder because they contain logic I'll still need once I implement it (excuse my english) How it works as fast as possible: - The capability has an object 'SkinChangeHandler' containing the pixel data on both and RL's on the client - This object is synced normally, if the pixel data changes a new texture is generated & set (this happens in deserilizeNBT, synchronization always happens server -> client so this is the perfect place for it) - For client-side skin choosing (by clicking the reset button or changing skin packs in the future) there's a SkinChange packet that's sent to the server with the new data. The server calls SkinChangeHandler.updateServerside which sets the new data and synchronizes it to all clients. While writing this commit message I noticed that I'm not 100% I check *which* player is updated, I assume it takes the player it received the packet from (I might have stolen this code from you).
@@ -153,7 +153,7 @@ | |||
|
|||
@Config.LangKey("config.regeneration.skins.model_preference") | |||
@Config.Comment("ALEX = 'Give me Alex Skins only', STEVE = 'Give me Steve Skins only', EITHER = 'Give me either!!'") | |||
public SkinChangingHandler.EnumChoices prefferedModel = SkinChangingHandler.EnumChoices.EITHER; | |||
public SkinUtil.ModelPreference prefferedModel = SkinUtil.ModelPreference.EITHER; //NOW this probably has to be a capability-level setting (server) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Configs are overwritten when playing on a server
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No they aren't
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait what. I always thought they were as it seems logical, as well as never having issues when playing with friends.
Was this done manually by the mods I played with? How can you prevent cheating if this is the case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or wait is it that the serverside logic uses the servers config instance and the client-side logic the client's? That would actually make a lot of sense 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup
|
||
public void randomizeSkin(Random rand) { | ||
if (!cap.getPlayer().world.isRemote) | ||
throw new IllegalStateException("Randomizing skin on server"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the skin decided on the server if the clients have the skins?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait I misread this, my bad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read that line again...
RegenerationMod.LOG.error(e.getMessage()); | ||
} | ||
} | ||
Minecraft.getMinecraft().getSoundHandler().playSound(new MovingSoundPlayer(cap.getPlayer(), RegenObjects.Sounds.REGENERATION_2, SoundCategory.PLAYERS, true, () -> cap.getState().equals(RegenState.REGENERATING))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhhhhhhh, make sure this get's changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops that's a conflict thingy
public static void sendSkinResetPacket() { | ||
NetworkHandler.INSTANCE.sendToServer(new MessageUpdateSkin(new byte[0], isSlimSkin(Minecraft.getMinecraft().player.getUniqueID()))); | ||
NetworkHandler.INSTANCE.sendToServer(new MessageSkinChange(new byte[0], Minecraft.getMinecraft().player.getSkinType().equals("slim"))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why didn't I think to do that? haha
File[] files = skins.listFiles(IMAGE_FILTER); | ||
|
||
if (files.length == 0) { | ||
//createDefaultImages(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do re-implement this of course, it stops kids deleting the skins directory and then going "WAAAAAAA WHY DID I CRASH"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course.
if (skinInfo.getSkintype() == SkinInfo.SkinType.ALEX) { | ||
SkinChangingHandlerOLD.setPlayerModel(renderPlayer, SkinUtil.ALEX_MODEL); | ||
} else { | ||
SkinChangingHandlerOLD.setPlayerModel(renderPlayer, SkinUtil.STEVE_MODEL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this hashmap will be moved to SkinUtil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know it's no longer needed. I'm not even sure it was needed in the first place, what was it used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Player logs in
They're not in the map
Game: "Oh hey, he hasnt been altered, lets sort that then mark him as altered"
Without it:
Players logs in
Player keeps their Mojang skin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So basically just to make sure everyone gets their skin when they log in? Why would you need the hashmap for that, you could just set the skin in onLogin
right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're saying that the information get's loaded from the Capability on login and set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so
How it works as fast as possible
SkinChangeHandler
containing the pixel data on both sides andResourceLocation
s on the clientdeserilizeNBT
, since synchronization always happens server → client so this is the perfect place for it)SkinChange
packet that's sent to the server with the new data. The server callsupdateServerside
on the capabilities skin handler, which sets the new data and synchronizes it to all clients.TODO
TOTEST