Major Bug Fix, Character Appearance Fix

[karl-police]

When the Character Appearance changes, such as a Bundle, e.g. Head, Right Arm, etc.

It breaks the character, because it's never being refreshed.

This patch, fixes it though. And I do not know if I should add a setting for this, cuz idk if it's gonna break the Catalog Avatar Creator's person thing...

https://github.com/TheNexusAvenger/Nexus-VR-Character-Model/issues/34

[karl-police]

I was too lazy to squash the commits cuz I don't use git but Github Desktop, but I didn't want to download it all nor launch the Rojo CLI, so I did it the old classical way.

[karl-police]

For testing I used Adonis Admin System and the commands.

[karl-police]

I don't like how this is set up right now.

• This looks like a patch instead of a proper integration. The client script makes internal changes to the CharacterService instead of the CharacterService encapsulating that behavior.
• Is the RemoteEvent needed? It looks like it can be done all on the client without the server providing a RemoteEvent that can be triggered by a malicious client.
• Can the updating be done in a more efficient manner (I don't know enough about HumanoidDescription to know the answer). Completely clearing the character seems like a lot more work than is required.
• Like the formatting or not, what you provided is not consistent with the rest of the project. PascalCase is used instead of camelCase, underscores aren't used, spaces should not trail --, and full variable names (Humanoid, Player, Character) are used in favor of the shorthands (hum, ply, char).

RemoteEvent might be needed because I am unsure about StreamingEnabled.

It is indeed a patch. I can add a function into the CharacterService for the SOLID Principle. But all I would do is to just set the table to nil.

I think the updating can be done more efficient. But I don't have VR, nor could I get SteamVR and an emulator to work without Roblox crashing.

There's just one issue when I manually tried to replace certain body parts... There is something in NexusVR, that is really important. The Input updates, are really big matter. When I was making this fix, I'd notice them floating up in the sky. And clearing up the entire character like that, worked well as a patch.

[karl-police]

I don't like how this is set up right now.

• This looks like a patch instead of a proper integration. The client script makes internal changes to the CharacterService instead of the CharacterService encapsulating that behavior.
• Is the RemoteEvent needed? It looks like it can be done all on the client without the server providing a RemoteEvent that can be triggered by a malicious client.
• Can the updating be done in a more efficient manner (I don't know enough about HumanoidDescription to know the answer). Completely clearing the character seems like a lot more work than is required.
• Like the formatting or not, what you provided is not consistent with the rest of the project. PascalCase is used instead of camelCase, underscores aren't used, spaces should not trail --, and full variable names (Humanoid, Player, Character) are used in favor of the shorthands (hum, ply, char).

changed

[karl-police]

need to re-check something

[karl-police]

oh ye the characteradded needs to be moved up, but I am changing something

[karl-police]

The better way would be Character.new()

the advanced way would be... the meh... but it would work

Here are some screenshots while I was re-testing this again

Then after fixing that

also there is something strange with the bend direction, but that was before as well I also see that there's something for the legs

[karl-police]

You can also change one single arm

https://www.roblox.com/catalog/134082453/Headless-Horseman-Left-Arm

works after resetting the character as well, connection stays

[karl-police]

I don't like how this is set up right now.

• This looks like a patch instead of a proper integration. The client script makes internal changes to the CharacterService instead of the CharacterService encapsulating that behavior.
• Is the RemoteEvent needed? It looks like it can be done all on the client without the server providing a RemoteEvent that can be triggered by a malicious client.
• Can the updating be done in a more efficient manner (I don't know enough about HumanoidDescription to know the answer). Completely clearing the character seems like a lot more work than is required.
• Like the formatting or not, what you provided is not consistent with the rest of the project. PascalCase is used instead of camelCase, underscores aren't used, spaces should not trail --, and full variable names (Humanoid, Player, Character) are used in favor of the shorthands (hum, ply, char).

it is no longer clearing the entire Character

it's still using the RemoteEvent though

[karl-police]

Now it no longer uses RemoteEvent

[karl-police]

This is significantly better. It is pretty close to something I would accept with a few changes:

• The rest of the project's if statements don't use parenthesis under single conditions (like (self.AppearanceChangedConnection), (Humanoid), and (Child:IsA("HumanoidDescription"))).
• RefreshCharacterParts was used before in previous commits but isn't anymore. I don't see any reason to keep it.

• To nitpick the comments:

  • While the project's comments are very verbose, --Store this into the object is not a useful comment. Consider removing it and putting it below the call to SetupVRParts.
  • The rest of the comments in the project end with punctuation.
  • "Set up" is an action while "setup" is a noun. "Set up" should be used when an action is intended. This goes for changing SetupVRParts to SetUpVRParts and SetupAppearanceChanged to SetUpAppearanceChanged as well.
  • SetupAppearanceChanged is missing a simple docstring like the others.

Changed

[karl-police]

done