InsertService error case behaviour changes

[spotco]

We’re implementing a behaviour change to InsertService in its error case.

Specifically, to properly check for InsertService:LoadAsset failure, it should be wrapped in a pcall similar to other services.

local model
local success, err = pcall(function()
  model = game:GetService("InsertService"):LoadAsset(257489726)
end)
if success then
  model.Parent = workspace
  print("successfully inserted model")
else
  warn("unable to insert model because " .. err)
end

The “err” string will contain a brief description of why the request failed.
The previous fail behaviour was to create an empty model.

If your code previously checked for an empty model, this may be a breaking change.
We’ll be rolling out this API change in the coming weeks based on developer feedback.

[Usering]

Any reason why you guys have decided to add this change in and not keep it the way it was, besides making it consistent with the rest of your API?

Not saying I’m against it, just wondering.

[Seranok]

This is nitpicky, but would you please use conventional variable names in your example code? It took me a bit to realize that fnrtv stood for function return value.

local model
local success, err = pcall(function()
  model = game:GetService("InsertService"):LoadAsset(257489726)
end)
if success then
  model.Parent = workspace
  print("successfully inserted model")
else
  warn("unable to insert model because " .. err)
end

[einsteinK]

@Seranok

It took me a bit to realize that fnrtv stood for function return value.

I just never figured it out until you discovered it.

@Usering not sure if you saw this, but:
http://devforum.roblox.com/t/sigh--title-at-the-right/22944

[Usering]

No I want to know why all the sudden they decided to go back on this method and change its behavior.

@Usering

besides making it consistent with the rest of your API?

[spotco]

This is part of a general cleanup of InsertService internals. There may be other outward-facing changes in the near future.

This specific change was done for consistency, since all similar “services” behave the same way.
In addition, for a localscript “LoadAsset”, there wasn’t any way to know if it succeeded for failed (no error message in the console, had to check for empty model).