Anaminus / roblox-bug-tracker

Formerly an unofficial bug tracker for Roblox.
33 stars 24 forks source link

StarterGui children cloned before Character property set #787

Closed cntkillme closed 8 years ago

cntkillme commented 8 years ago

This is a really old problem, and I'm not sure why it still exists but it does. The children in StarterGui are cloned into a player's PlayerGui when their character respawns. But at that point the Character property is not set by Roblox yet (which is the sole reason why local scripts have to yield before they try indexing the character). Somewhat similar is that when CharacterAdded is fired, all children of the Character should exist at that point and the Character is expected to be in Workspace, which is not the case.

Reproduction:

-- put in a localscript in StarterGui
local char = game.Players.LocalPlayer.Character
print(char) -- nil

Reproduction:

-- put in a script in Workspace/SSS/...
game.Players.PlayerAdded:connect(function(x)
  x.CharacterAdded:connect(function(y)
    print(y.Parent) -- nil
  end)
end)
JodeRBX commented 8 years ago

Problems like this exist throughout the engine, and there are several good arguments against fixing them in general. I'll focus on just the two examples you bring up.

  1. It is more efficient to load everything all at once. If GUIs had to wait for the character to finish loading completely, then that'd slow processing time. This is even less justifiable when you consider that many GUIs might not even touch the character at all. It's much better to let scripts yield when they need the character, so that GUIs that don't can continue initializing themselves in meantime. Even waiting for LocalPlayer.Character to not be nil -- even if it isn't fully set up yet -- would likely introduce some delay. Consider when you first join a place. The client can clone in StarterGUI even before its character is set by the server (well, it can at least do this when Players.CharacterAutoLoads is true). This optimization would be impossible if GUIs needed to wait for the server to set the client's character.
  2. Consider what CharacterAdded does -- it is really just a Changed event that only fires for the Character property. If it wasn't, then CharacterAdded wouldn't fire for custom characters. Setting it up this way provides additional control over how characters are created and managed. If you were to make it a post-event -- that is, it fires after the character is set up, not before -- then not only would you lose much of that control, you would once again introduce unnecessary delays. Let's say of all the things in a character, you are only interested in changing the Humanoid's MaxHealth property. If CharacterAdded was a post-event, then you could safely do this: char.Humanoid.MaxHealth = 200. But since it isn't, you need to do this: char:WaitForChild("Humanoid").MaxHealth = 200. This is uglier, sure, but it is also more performant; you only care about the Humanoid, so that's all you wait for. If CharacterAdded was a post-event, then you'd have to wait for the entire character to load, even though you only care about just one part of it.

I've only really scratched the surface here, but I hope this gives you a sense of how much more complicated the issue is than it might initially seem.

cntkillme commented 8 years ago

Yeah those are pretty obvious points, but there's not much to load. Just the parts have to be created and parented into the character which is already what happens according to the wiki: CharacterAdded fires when the Character is added to the Workspace. The Humanoid, Torso, and other parts of a character exist, but clothing items like hats and shirts/pants may take a few seconds to be added to the character. so in a way what doesn't happen, should happen according to the wiki.

matthewdean commented 8 years ago

Character loading is not guaranteed to be asynchronous: in the case of the new R15 characters it may take several seconds. In fact, if CharacterAutoLoads is false, the character will never load but GUIs should.

cntkillme commented 8 years ago

The wiki should be updated to account for that here: http://wiki.roblox.com/index.php?title=API:Class/StarterGui and http://wiki.roblox.com/index.php?title=API:Class/Player/CharacterAdded

In the first link, it says that all the children get cloned to a Player's PlayerGui when the character spawns (which I believe implies CharacterAdded is fired) and in the second link remove the portion that states "CharacterAdded fires when the Character is added to the Workspace" (or maybe rephrase it to state when the Character property is set).