Let us use the .new() constructor to fill in RaycastParam's properties

So we can do:

local raycastParams = RaycastParams.new(Enum.RaycastFilterType.Blacklist, ignoreList, false)
workspace:Raycast(origin, direction, raycastParams)
-- or even, if we so desired...
workspace:Raycast(origin, direction, RaycastParams.new(Enum.RaycastFilterType.BlackList, ignoreList, false))

Instead of:

local raycastParams = RaycastParams.new()
raycastParams.FilterType = Enum.RaycastFilterType.Blacklist
raycastParams.FilterDescendantsInstances = ignoreList
raycastParams.IgnoreWater = false
workspace:Raycast(origin, direction, raycastParams)

As well as being much more concise, allowing us to provide properties in the constructor also lets us build the RaycastParams inline instead of having to save it to a variable and type in an additional 3 steps to set the properties, then pass in the variable.

I know of no good reason for this constraint to exist. This functionality already exists elsewhere, for example in the TweenInfo object (where the constructor is actually useful and makes scripters’ lives easier).

Ironically, while this new API is meant to make raycasting cleaner and not force the developer to use a dumb object, the RaycastParams object somewhat revives that frustration by not letting us just give RaycastParams concisely and making us write boilerplate.

14 Likes

I was confused why Roblox made it so you can’t fill in properties for the constructor when TweenInfo already does it with its lengthy set of accepted parameters.

This feature is minor but worth looking at by Roblox.

I do think this new API still makes raycasting however cleaner since you can just have an instance to filter descendants from rather than getting the instance itself and its entire descendants with extra steps.

1 Like

TweenInfo should work like RaycastParams imo, there’s just too many properties to order. Same with CustomPhysicalProperties.

4 Likes

The point of the API is to be nicely extensible. Taking arguments is counter to that goal, because as soon as we add a new feature to it which a lot of people do want to use, we’re now back in a bad situation again:

  • You would have to fill in a bunch of default arguments that you didn’t want anyway to “get to” the one that you do want to fill in.

  • The code would now be really hard to read, because you have a long argument chain with no naming of which parameter is what, so it would be easy to make a mistake in setting it up.

  • The argument order wouldn’t make any sense, so you’d have to look it up every time.

TL;DR: This seems like a good idea right now but it doesn’t work long term.

Furthermore I think most of the annoyance should be cleaned up once the better code completion for the studio editor ships, since you’ll be able to fill in fields in the RaycastParams very easily.

EDIT: I should note, quenty is correct. TweenInfo would be done the same way as RaycastParams if it were introduced today. CustomPhysicalProperties can’t be a mutable object, otherwise you could write part.CustomPhysicalProperties.Property = value and be very surprised when it doesn’t have an effect on the part.

13 Likes

Not to mention… shudders DockWidgetPluginGuiInfo

@tnavarts would it be possible to make these datatypes mentioned have their properties not be read-only? What’s the reasoning behind for example DockWidgetPluginGuiInfo being that way while RaycastParams is not?

The person wrote the API simply followed the existing conventions set out by the existing types like Vector3 / CFrame of being immutables constructed from their arguments rather than considering other options. There was actually considerable internal discussion when I proposed this style for RaycastParams since it’s effectively a completely new data type in being a non-Instance mutable object.

I’m actually going to “fix” this type when I have time allowing it to be constructed the same way as RaycastParams (I would go so far as to remove the constructor arguments if backwards compatibility weren’t a factor).

3 Likes

Hmm ok, I guess. I’m not a fan of the verbosity but maybe I’m just too attached to doing things in one line.

It seems like you’re moving towards more of a struct style with datatypes which is a good thing imo.

Unfortunately there isn’t really an alternative. Raycasting is fundamentally a complex operation with a lot of configurability on exactly how it is performed.

The API I designed does give you close to the minimum verbosity in the long run since the verbosity of the Raycast call will be linearly proportional to the number of features of the API you want to use. The only way it could be shorter is if the Raycast call or the RaycastParams constructor took a table instead, which would let you fit things into a single statement, but at the cost of being much easier to misuse (accidentally misspelling one of the table fields would be a silent error)

2 Likes

Your point is really good actually and I might even seek out this approach if I made anything with the new constructor for a usable object.

How would it be a silent error? Wouldn’t the classic X is not a valid member of Y be thrown?

Presumably it would only check if specific fields were set, rather than going through all fields in the table and setting fields according to field names.

local function f(t)
    local x,y,z = t.x or 0,t.y or 0,t.z or 0
    -- do something with x,y,z
end
f{X=1,y=2,z=3}
-- default value used, X field goes unused, not very obvious there is a problem
-- because of no error

And if you did go through every field, then the order in which the fields wouldn’t be defined. (Which could be important in the future, maybe field X needs to be set before field Y)

local function f(t)
    for i,v in pairs(t) do
        print(i,v)
    end
end
f{x=1,y=2,z=3}
-- (output in roblox studio)
--> y 2
--> x 1
--> z 3

This would probably also end up being slower, as for every field it would have to do at least 1 string comparison to verify that it’s a valid field and which field it is.

1 Like

Digging through the table and trying to find all of the fields (which would be necessary to get that error) would be surprising behavior.

That wouldn’t support someone using a proxy object with a metatable as the argument, because such an object might not have any properties. It also wouldn’t support that table have any sort of additional developer created “class tag” or metadata in it. Of course, you could decide to just not support those cases. But due to stuff like that the semantics of using a table as an argument are generally fairly messy and open to interpretation, which is why existing APIs shy away from that.

2 Likes

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.