Repository navigation
Added additional param_tuple macros - #1712
claymore-gamedev wants to merge 1 commit into
Conversation
|
I'm not really in favor of this, especially not without a very good justification of a use case. The reason why the arity is 14 right now is because some Godot APIs simply don't work without it:
It's already too much in my opinion, but we have no choice. Keep in mind that Rust's hacky emulation of variadic generics isn't free, and in the interest of compile time I'd rather not support more parameters. The argument becomes an arbitrary "one more doesn't hurt" at this point 🙂 |
|
Fair enough. I just needed to make a constructor for a custom refcounted with 16 arguments. |
|
Out of curiosity, what needs 16 parameters? It's also very easy to make off-by one errors (especially since neither GDScript nor Rust have named arguments... only IDEs may help a bit here). Splitting into structs, using properties after construction, builders, ... there are a few way to counter this 🤔 |
|
I have a rather fat network message that I send to new players when they join a lobby. I made the somewhat bad decision 10 months ago that I would manage all of my data in the Godot side so I have the need for sometimes calling functions with a lot of parameters from gdscript. In particular I have a refcounted that holds all the amounts of different rarities of fish and treasure that a player has gathered. That itself it already split into two smaller refcounted; one for the treasure, and one for the fish. Each of those smaller refcounted contain 15 parameters. Because there's no optional parameters, the builder pattern doesn't seem appropriate to me here. It's already split into the smallest logical groups I want to work with so more refcounted layers would just over complicate it. I have a signal somewhere else that emits 16 parameters among which are also several refcounted to group parameters together. That message is the information about an existing player you receive when you join a lobby. I would never surface so much rust to gdscript in a future project, it's just the price of my decision 10 months ago and my unwillingness to rewrite my 50,000 lines of gdscript into rust. |
|
I figured I would make a pull request in case anyone had the same problem as me. Guess not. |
|
Thanks a lot for the elaboration, and contribution. Maybe we can extend this one day, but we've recently done quite a bit of compile-time optimization (#1706 + 4 more PRs), and the main culprit were too many combinations of trait instantiations ("monomorphizations"). And the traits of this kind are unfortunately quite prone to multiply the combinations with each extra parameter. So I think it's not too bad to nudge the user towards fewer parameter lists, in their own interest 🙂 maybe at one point it could even be reduced to 10 or so (now it's dependent on Godot and not documented, so when Godot changes it may also change...). |
Minor change I made for myself to implement a few more param_tuple sizes up to 16 arguments.