[3.x] Increase VARIANT_ARG_MAX to 8

[Rubonnek]

Fixes:

• Unable to configure functions with more than five parameters in AnimationTrack #54185
• UndoRedo's do/undo methods can't go further than 5 arguments #26483

Aside from the issues above, the other reason for increasing VARIANT_ARG_MAX to 8 is because it was done on master already to fix similar issues.

See:

• Increase the number of arguments accepted by UndoRedo methods #50206.

[Rubonnek]

Updated a code block in AnimationTree that also depends on VARIANT_ARG_MAX.

I should say that this pull request now also allows users to call TileMap.set_cell successfully through an AnimationPlayer and AnimationTree, given that the we are now able to pass the number of parameters required to properly call such functions.

[akien-mga]

We discussed this in a PR review meeting and we're not sure it's worth changing the whole VARIANT_ARG_MAX just for this - the limit on animation code doesn't seem related to VARIANT_ARG_MAX and could likely be lifted to allow any number of arguments, no?

[Rubonnek]

In this code the user is modifying a key within a method-type track of an Animation, right? The number of arguments for a method-type track in an Animation is bound by the maximum number of arguments the AnimationPlayer itself supports when playing this track. VARIANT_ARG_MAX is related. I fail to see how it could not. Keep an eye on Animation::TYPE_METHOD.

When the AnimationPlayer attempts to play the method-type track, it will run the following code (only pay attention to the first line, the last line, and params near the bottom):

godot/scene/animation/animation_player.cpp

Lines 527 to 548 in b197de6

	case Animation::TYPE_METHOD: {
	if (!nc->node) {
	continue;
	}
	if (p_delta == 0) {
	continue;
	}
	if (!p_is_current) {
	break;
	}
	List<int> indices;
	a->method_track_get_key_indices(i, p_time, p_delta, &indices);
	for (List<int>::Element *E = indices.front(); E; E = E->next()) {
	StringName method = a->method_track_get_name(i, E->get());
	Vector<Variant> params = a->method_track_get_params(i, E->get());
	int s = params.size();
	ERR_CONTINUE(s > VARIANT_ARG_MAX);

The very last line of that code block has a direct reference to VARIANT_ARG_MAX. That check is there because further below AnimationPlayer reconstructs the method call extracted from the track:

godot/scene/animation/animation_player.cpp

Lines 556 to 573 in b197de6

	if (method_call_mode == ANIMATION_METHOD_CALL_DEFERRED) {
	MessageQueue::get_singleton()->push_call(
	nc->node,
	method,
	s >= 1 ? params[0] : Variant(),
	s >= 2 ? params[1] : Variant(),
	s >= 3 ? params[2] : Variant(),
	s >= 4 ? params[3] : Variant(),
	s >= 5 ? params[4] : Variant());
	} else {
	nc->node->call(
	method,
	s >= 1 ? params[0] : Variant(),
	s >= 2 ? params[1] : Variant(),
	s >= 3 ? params[2] : Variant(),
	s >= 4 ? params[3] : Variant(),
	s >= 5 ? params[4] : Variant());
	}

The params array holds the number of arguments previously saved on the track through the AnimationTrackEditor. Everytime VARIANT_ARG_MAX is updated, that last code block needs to be updated to support the maximum number of arguments.

[Rubonnek]

@akien-mga is there anything holding back this PR?

For what is worth, sure, we could remove the limit on this referenced code but that doesn't mean the AnimationPlayer can execute the saved tracks -- that's what ties VARIANT_ARG_MAX to the code you've referenced.

[akien-mga]

Missing p_arg8.

[Rubonnek]

Fixed,

[akien-mga]

Discussed in PR review meeting, we're slightly concerned about the potential performance impact but given the places where it's used, it should be fine.

For Godot 4.0, this needs to be changed to use variadic templates so we can dehardcode things properly.

[akien-mga]

Thanks!