cc @Grapjas

That's an interesting patch. Don't forget the XML headers in the group files.

Don't forget to also update checkrefs.py.

Would be nice to have tests for those refs https://trac.wildfiregames.com/ticket/6029

Optionally it would be nice to have a script that move duplicated (say 3 groups identical) into a group file. Or at least lists them. This way we can update mods.

I don't really like idea of the easier way to increase the number of variations. And because our art contributors are inexperienced in terms of perfomance (and we don't have art reviewers) it's very easy to significantly decrease the game performance. We already have a lot of bad helmets and draw call nightmire of rice fields.

How does this make it easier to add more variation? If anything it reduces duplication and makes it easier to ensure consistency in actors. Also makes diff easier to review since less files get touched

In D4821#205347, @Stan wrote:

How does this make it easier to add more variation?

Adding a variant in a single group adds it in every reference. Another problem is that it might be nested: actor > group > group > group, which might lead to unknown consequences.

If anything it reduces duplication and makes it easier to ensure consistency in actors.

As I said "the easier way to increase the number of variations.".

Also makes diff easier to review since less files get touched

It doesn't make sense for me if nobody does a review for art.

Well yeah kinda the point, you group things to update at the same time.

Not unknown. It's a crash, whether it's actors, props and variants, and there groups.

Well it's easier to add variation maybe but that also means since they are the same consistently it's easier to do instancing too cause props are more likely to be in the same in the general case. Eg a group with all animations means I'm less likely to have different tools per anims.

Sure. But there are mods that might want to be stricter about those. And the biggest use case for those is much less terrible updates if they rely on our actors like grapejuice.

In D4821#205351, @Stan wrote:

Not unknown. It's a crash, whether it's actors, props and variants, and there groups.

In case of actor > group > group > group it's easier to add a bug. As you don't see all references at once. And you can't always reproduce the bug because it happens only for some particular seed.

it's easier to do instancing too cause props are more likely to be in the same in the general case. Eg a group with all animations means I'm less likely to have different tools per anims.

It's true when the number of different combinations noticeably lower than the number of instances. Instancing for the low number of actors won't give you performance (might even take it for some drivers).

Sure. But there are mods that might want to be stricter about those. And the biggest use case for those is much less terrible updates if they rely on our actors like grapejuice.

I don't mind to help modders, but at the same time players complain about performance.

Indeed but we have checkrefs for that case :)

Then we can decide that the rule is only for animation groups and I can yell at artists if they break it. I think the proposed is okay. I wouldn't do it for textures or meshes.

Btw @Silier you are also missing the group rnc file :)

In D4821#205353, @Stan wrote:

Indeed but we have checkrefs for that case :)

I suppose it doesn't work for that, because it can't check incompatible material + actor, or overflowed mesh.

Then we can decide that the rule is only for animation groups and I can yell at artists if they break it. I think the proposed is okay. I wouldn't do it for textures or meshes.

It's not hard to add the validation in the LoadGroup.

Btw @Silier you are also missing the group rnc file :)

Also broken CC and indentations.

It is hard because animation variants contain tools props. You could restrict mesh and textures nodes but then you can't have bloodied corpses. Which you should override I suppose.

materials are not in groups.

I suppose one can check overflowed mesh by parsing dae, but that's out of the scope.

In D4821#205355, @Stan wrote:

It is hard because animation variants contain tools props.

Another way to have a performance drop.

You could restrict mesh and textures nodes but then you can't have bloodied corpses. Which you should override I suppose.

It should be restricted.

materials are not in groups.

Meshes are, so you might get a crash that will appear only some time after a commit.

In case of actor > group > group > group it's easier to add a bug

That can be solved by allowing only "actor > groupref > group" or "actor > group" so no group chain can be created

In D4821#205362, @Silier wrote:

That can be solved by allowing only "actor > groupref > group" or "actor > group" so no group chain can be created

Yeah, we can, though it won’t solve the lost context.