search

godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.13k stars 93 forks source link

Increase the number of arguments in `ClassDB::bind_method` to 6 by default #1229

Closed Xrayez closed 4 years ago

Xrayez commented 4 years ago

Describe the project you are working on: Godot Engine modules development

Describe the problem or limitation you are having in your project: Oftentimes, I need to bind just about 6 arguments for a method, but the current limit is 5 arguments:

Some my own usages or modules I created:

Godot's own use cases (some of them do not require exactly 6 arguments though):

Some other cases which require more than 5:

This list is to be updated.

Describe the feature / enhancement and how it helps to overcome the problem or limitation: Make a change in make_binders.py to support 6 arguments in ClassDB::bind_method. This will modify core/method_bind.gen.inc, and we'll get less use cases where we need to include core/method_bind_ext.gen.inc which supports 13-15 arguments currently. Which, to my presumptions, make the engine binary size bloat. It seems like the exact reason as to why core/method_bind.gen.inc and core/method_bind_ext.gen.inc have to exist is also not documented.

I understand the logic of "if we increase the limit, people would start demanding 7 arguments" etc. But you also have to agree that the more arguments we have, the less cases we're left where we have to manually include core/method_bind_ext.gen.inc. So, increasing the default number by 1 will not eliminate all cases, but at least make the experience for more common use cases a breeze.

I also understand the logic of "if you need to have more than the default, you need to reconsider your architectural decisions". But this is already the case for engine use cases, where we have to include core/method_bind_ext.gen.inc for RenderingServer (VisualServer in 3.2), PhysicsServer2D/3D etc.

My own rationale behind increasing this from 5 to 6 is that, we create systems with some parity in mind. To make something connect, you need to have a pair. The odd number does not contribute to this logic. See get_noise_6d as an example above.

Increasing the default limit could actually make the engine binary size decrease, because we're not going to include a whole lot of methods to support whopping 13-15 arguments in comparison. Again, I'm still not sure whether including those headers bloat the engine binary size, but that's likely the rationale behind such a split.

Another reason for increasing the default limit is that we've already increased the method_bind_ext.gen.inc limit from 13 to 15 in https://github.com/godotengine/godot/commit/9b0dd4f571ff431e23b9097e7f29746f4157be12, which somewhat signifies that there's a general need for increasing the limit.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams: That's up for a PR if we reach a consensus.

If this enhancement will not be used often, can it be worked around with a few lines of script?: You have to #include "core/method_bind_ext.gen.inc" each time you need to bind more than the default limit.

Is there a reason why this should be core and not an add-on in the asset library?: The idea is that the defaults should reflect the real life to cater more use cases. Feel free to add your own to justify this proposal.

Calinou commented 4 years ago

Wasn't ClassDB refactored to make use of C++17, which makes it possible to support more arguments while not bloating the binary as much?

Xrayez commented 4 years ago

This would certainly make the proposal not relevant anymore (maybe for 3.2). According to godotengine/godot#40487, even if it was implemented on some level, this is still limited in 4.0/master AFAIK.

I actually have an idea of making this configurable via SCons so that module developers are not necessarily restricted by the engine limit, I guess should be trivial to add if desired. Talking about adding some command-line option (it doesn't have to be visible to users if you're afraid to bloat the command-line API).

akien-mga commented 4 years ago

I don't understand what the problem is with including core/method_bind_ext.gen.inc when you need 6 arguments or more, in the very few locations where this is needed?

$ rg 'include "core/method_bind_ext.gen.inc"' -l
editor/editor_resource_preview.cpp
servers/physics_2d_server.cpp
servers/visual_server.cpp
servers/physics_server.cpp
scene/2d/canvas_item.cpp
scene/gui/graph_node.cpp
scene/3d/physics_body.cpp
scene/2d/tile_map.cpp
scene/2d/physics_body_2d.cpp
scene/animation/tween.cpp
scene/animation/animation_tree.cpp
scene/resources/font.cpp
scene/resources/texture.cpp
scene/resources/surface_tool.cpp
core/class_db.h
$ rg 'include "core/method_bind_ext.gen.inc"' -l | wc -l
15
$ rg 'ClassDB::bind_method\(' -l | wc -l
421

Another reason for increasing the default limit is that we've already increased the method_bind_ext.gen.inc limit from 13 to 15 in godotengine/godot@9b0dd4f, which somewhat signifies that there's a general need for increasing the limit.

That's not a valid reason. This change means that some methods need to bind up to 15 arguments. It doesn't mean that all methods as a whole tend to get more arguments and thus the baseline should be raised. Methods with more than 5 arguments are not user-friendly and often a sign of bad architecture. There are exceptions and sometimes it's needed, but I don't see the point in facilitating.

I also understand the logic of "if you need to have more than the default, you need to reconsider your architectural decisions". But this is already the case for engine use cases, where we have to include core/method_bind_ext.gen.inc for RenderingServer (VisualServer in 3.2), PhysicsServer2D/3D etc.

Yeah and the servers are exceptions, where a lot of information needs to be passed at once. If we could pass an info struct instead as done in the Vulkan API, it would be much better.

My own rationale behind increasing this from 5 to 6 is that, we create systems with some parity in mind. To make something connect, you need to have a pair. The odd number does not contribute to this logic. See get_noise_6d as an example above.

That doesn't make sense to me. Method arguments often represent completely different data structures and there's very little case to be made for "parity" here.

Random example from Tween:

247:    ClassDB::bind_method(D_METHOD("interpolate_property", "object", "property", "initial_val", "final_val", "duration", "trans_type", "ease_type", "delay"), &Tween::interpolate_property, DEFVAL(TRANS_LINEAR), DEFVAL(EASE_IN_OUT), DEFVAL(0));

If you want to find parity in this method, you could argue that (object, property) is a pair, (initial_val, final_val) is, but duration, trans_type, ease_type and delay aren't.

Another example from AnimationPlayer:

1642:   ClassDB::bind_method(D_METHOD("play", "name", "custom_blend", "custom_speed", "from_end"), &AnimationPlayer::play, DEFVAL(""), DEFVAL(-1), DEFVAL(1.0), DEFVAL(false));

There's no parity at all. Apart from the name which should be first, those arguments could be in any order. In AnimationNode:

423:    ClassDB::bind_method(D_METHOD("blend_node", "name", "node", "time", "seek", "blend", "filter", "optimize"), &AnimationNode::blend_node, DEFVAL(FILTER_IGNORE), DEFVAL(true));

Sure there might be a handful of classes where the method arguments are paired, but even then it might often be method(name, pair1_a, pair1_b, pair2_a, pair2_b), which supports a default of 5 arguments (or 7). But again, it's very far-fetched to me.

Increasing the default limit could actually make the engine binary size decrease, because we're not going to include a whole lot of methods to support whopping 13-15 arguments in comparison. Again, I'm still not sure whether including those headers bloat the engine binary size, but that's likely the rationale behind such a split.

That's the only thing which would make this proposal meaningful to me, so I suggest testing it and report on the findings. If it does make the engine size smaller to increase the max number of arguments in the non-ext method_bind.h, then that's an actual gain which makes the change meaningful. Otherwise there's no reason to change it if there's no gain.

akien-mga commented 4 years ago

Let's check what we're talking about for context:

echo "# 1 arg:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+"\)' | wc -l
echo "# 2 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+"\)' | wc -l
echo "# 3 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+", "[^"]+"\)' | wc -l
echo "# 4 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+"\)' | wc -l
echo "# 5 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+"\)' | wc -l
echo "# 6 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+"\)' | wc -l
echo "# 7 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+"\)' | wc -l
echo "# 8 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+"\)' | wc -l
echo "# 9 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+"\)' | wc -l
echo "# 10 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+"\)' | wc -l
echo "# 11 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+"\)' | wc -l
echo "# 12 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+"\)' | wc -l
echo "# 13 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+"\)' | wc -l
echo "# 14 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+"\)' | wc -l
echo "# 15 args:"
rg 'ClassDB::bind_method\(D_METHOD\("[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+", "[^"]+"\)' | wc -l

In 3.2:

# 1 arg:
3179
# 2 args:
944
# 3 args:
246
# 4 args:
112
# 5 args:
53
# 6 args:
20
# 7 args:
13
# 8 args:
6
# 9 args:
6
# 10 args:
1
# 11 args:
2
# 12 args:
1
# 13 args:
1
# 14 args:
0
# 15 args:
0

In master:

# 1 arg:
3287
# 2 args:
998
# 3 args:
261
# 4 args:
117
# 5 args:
48
# 6 args:
15
# 7 args:
14
# 8 args:
6
# 9 args:
12
# 10 args:
5
# 11 args:
6
# 12 args:
0
# 13 args:
0
# 14 args:
0
# 15 args:
0

Seems to me that the tendency is towards have less methods with more than 5 arguments, not more.

Edit: There should be a handful of bindings with >= 12 args, some added in godotengine/godot@9b0dd4f571 but not properly bound (the C++ canvas_item_add_nine_patch is registered with FUNC15, but the binding has only 10 or 11 named arguments, and somehow the method doesn't appear in the docs at all...? well at least this proposal as the merit of exposing a bug :)).

Xrayez commented 4 years ago

Thanks for taking the time to measure this!

I don't understand what the problem is with including core/method_bind_ext.gen.inc when you need 6 arguments or more, in the very few locations where this is needed?

I'm not necessarily talking about in-house, engine-related cases. There are custom C++ modules which rely on this limit. That's why I'm asking potential module developers out there to showcase their results. Perhaps The Godot Sandbox requires just about 5 arguments for their use cases by default, but module devs are more limited in what they can do with the engine internals, and may as well require less strict limits from the core side. I hope that the engine cares about those use cases.

That doesn't make sense to me. Method arguments often represent completely different data structures and there's very little case to be made for "parity" here.

Well that's more like a philosophical thoughts from my side rather than an argument, sorry for making it look like an actual reasoning. Perhaps you know my stance regarding intuition over logic. If game engines were built out of pure logic, we'd get very raw tools at our disposal which do not necessarily cater to peculiarities of human behavior. And I hope that those features is what makes Godot different from other engines.

Those statistics kinda prove my point in my eyes. The 6 args seems to be like the starting point of decreased usage, which is worth including as default, but of course not more than 6. That's a good compromise the way I see it.

Increasing the default limit could actually make the engine binary size decrease, because we're not going to include a whole lot of methods to support whopping 13-15 arguments in comparison. Again, I'm still not sure whether including those headers bloat the engine binary size, but that's likely the rationale behind such a split.

That's the only thing which would make this proposal meaningful to me, so I suggest testing it and report on the findings. If it does make the engine size smaller to increase the max number of arguments in the non-ext method_bind.h, then that's an actual gain which makes the change meaningful. Otherwise there's no reason to change it if there's no gain.

I hoped that the proposal is somewhat obvious to implement especially given it takes almost no effort (in contrast to writing this proposal and discussing it further), but I'll take the challenge and report, if this doesn't hold true I'll close this proposal. 😛

Xrayez commented 4 years ago

So I've done some testing by removing method_bind_ext.gen.inc from servers/physics_server.cpp and increasing the limit in core/make_binders.py by 1. To my ignorant surprise, I haven't noticed any difference in binary size.

I suspect that the reason behind existence of those is simply because of @reduz preferences regarding the source code debug symbols readability or similar. See what you get when you try to compile without method_bind_ext.gen.inc without increasing the limit manually:

D:\src\godot\core/class_db.h(267): error C2672: 'create_method_bind': no matching overloaded function found
servers\physics_server.cpp(363): note: see reference to function template instantiation 'MethodBind *ClassDB::bind_method<MethodDefinition,Dictionary(__cdecl PhysicsDirectSpaceState::* )(const 
Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)>(N,M,const Variant &,const Variant &,const Variant &,const Variant &)' being compiled
        with
        [
            N=MethodDefinition,
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(R (__cdecl T::* )(P1,P2,P3,P4,P5) const)': could not deduce template argument for 'R (__cdecl T::* )(P1,P2,P3,P4,P5) const' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(4746): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(void (__cdecl T::* )(P1,P2,P3,P4,P5) const)': could not deduce template argument for 'void (__cdecl T::* )(P1,P2,P3,P4,P5) const' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(4528): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(R (__cdecl T::* )(P1,P2,P3,P4,P5))': could not deduce template argument for 'R (__cdecl T::* )(P1,P2,P3,P4,P5)' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(4310): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(void (__cdecl T::* )(P1,P2,P3,P4,P5))': could not deduce template argument for 'void (__cdecl T::* )(P1,P2,P3,P4,P5)' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(4092): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(R (__cdecl T::* )(P1,P2,P3,P4) const)': could not deduce template argument for 'R (__cdecl T::* )(P1,P2,P3,P4) const' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(3874): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(void (__cdecl T::* )(P1,P2,P3,P4) const)': could not deduce template argument for 'void (__cdecl T::* )(P1,P2,P3,P4) const' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(3664): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(R (__cdecl T::* )(P1,P2,P3,P4))': could not deduce template argument for 'R (__cdecl T::* )(P1,P2,P3,P4)' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(3454): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(void (__cdecl T::* )(P1,P2,P3,P4))': could not deduce template argument for 'void (__cdecl T::* )(P1,P2,P3,P4)' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(3244): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(R (__cdecl T::* )(P1,P2,P3) const)': could not deduce template argument for 'R (__cdecl T::* )(P1,P2,P3) const' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(3034): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(void (__cdecl T::* )(P1,P2,P3) const)': could not deduce template argument for 'void (__cdecl T::* )(P1,P2,P3) const' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(2832): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(R (__cdecl T::* )(P1,P2,P3))': could not deduce template argument for 'R (__cdecl T::* )(P1,P2,P3)' from 'M'     
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(2630): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(void (__cdecl T::* )(P1,P2,P3))': could not deduce template argument for 'void (__cdecl T::* )(P1,P2,P3)' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(2428): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(R (__cdecl T::* )(P1,P2) const)': could not deduce template argument for 'R (__cdecl T::* )(P1,P2) const' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(2226): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(void (__cdecl T::* )(P1,P2) const)': could not deduce template argument for 'void (__cdecl T::* )(P1,P2) const' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(2032): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(R (__cdecl T::* )(P1,P2))': could not deduce template argument for 'R (__cdecl T::* )(P1,P2)' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(1838): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(void (__cdecl T::* )(P1,P2))': could not deduce template argument for 'void (__cdecl T::* )(P1,P2)' from 'M'     
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
[ 96%] D:\src\godot\core\method_bind.gen.inc(1644): note: see declaration of 'create_method_bind'
Compiling ==> scene\resources\rectangle_shape_2d.cpp
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(R (__cdecl T::* )(P1) const)': could not deduce template argument for 'R (__cdecl T::* )(P1) const' from 'M'     
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(1450): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(void (__cdecl T::* )(P1) const)': could not deduce template argument for 'void (__cdecl T::* )(P1) const' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(1264): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(R (__cdecl T::* )(P1))': could not deduce template argument for 'R (__cdecl T::* )(P1)' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(1078): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(void (__cdecl T::* )(P1))': could not deduce template argument for 'void (__cdecl T::* )(P1)' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(892): note: see declaration of 'create_method_bind'
                                                                                         D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(R (__cdecl T::* )(void)
 const)': could not deduce template argument for 'R (__cdecl T::* )(void) const' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(706): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(void (__cdecl T::* )(void) const)': could not deduce template argument for 'void (__cdecl T::* )(void) const' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(528): note: see declaration of 'create_method_bind'
D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(R (__cdecl T::* )(void))': could not deduce template argument for 'R (__cdecl T::* )(void)' from 'M'
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(350): note: see declaration of 'create_method_bind'rectangle_shape_2d.cpp

D:\src\godot\core/class_db.h(267): error C2784: 'MethodBind *create_method_bind(void (__cdecl T::* )(void))': could not deduce template argument for 'void (__cdecl T::* )(void)' from 'M'       
        with
        [
            M=Dictionary (__cdecl PhysicsDirectSpaceState::* )(const Vector3 &,const Vector3 &,const Vector<RID> &,uint32_t,bool,bool)
        ]
D:\src\godot\core\method_bind.gen.inc(172): note: see declaration of 'create_method_bind'
scons: *** [servers\physics_server.windows.tools.64.obj] Error 2
scons: building terminated because of errors.

So, I'm not sure whether my proposal is still relevant because:

  1. This doesn't seem to affect binary sizes in either case. Hence:
  2. It doesn't really hurt much increasing the default limit by 1 for readability stuff.

In either case, I doubt the limit will be increased because usability is preferred over anything else for Godot development according to godotengine/godot-docs#3788, so I'll just close this.

Again, thanks for taking the time to write this, I'm satisfied by this kind of interaction on GIP whatever the end result.

Xrayez commented 3 years ago

For anyone stumbling upon this, this is no longer a limitation in 4.x, see godotengine/godot#42826.