cgame: parse modelRotation for player models, fix modelScale

[illwieckz]

Bug fix:

The modelScale field from configs/classes/*.model.cfg was unused with skeletals models, unlike configs/buildables/*.model.cfg and configs/missiles/*.model.cfg.

Feature implementation:

The modelRotation field was not implemented in configs/classes/*.model.cfg, unlike configs/buildables/*.model.cfg and configs/missiles/*.model.cfg.

Clean-up:

A MODEL_ROTATION enum value was set but had no usage.

[slipher]

What is this needed for?

The cgame model code really sucks by the way. The code for different categories of thing (weapon/player/buildable/etc.) all randomly works differently for no reason. When using the same model as both a weapon and a missile I found that not only did they turn out different sizes; on one of them (but not the other) the faces were actually inside out and I had to set RF_SWAPCULL. So it would be unfortunate if we added a bunch of rotation code with no documentation as to what the angles are and resulting in a different effect in each place it is used.

[illwieckz]

1. Unifying file syntax and feature support

We support modelScale and modelRotation in .model.cfg files when stored in configs/buildables/ or configs/missiles/ but not in configs/models/, this is bad as the syntax is shared among those files and a contributor is expected to believe those features are there.

About modelScale, the lack of it in .model.cfg for models leds to the usage of another file that is models/<name>/<name>/character.cfg, which is also parsed by cgame, but whose data is sent to engine (!) to be rotated by the engine itself (!!!). It is better to unify the features, especially since the character.cfg method is less efficient. It's likely that the modelScale from character.cfg is applied every frame and that we may deleted per-frame engine code while keeping another valid way to rotate model. The character.cfg has no modelRotation feature, and I gave a quick look at it and the cgame/engine split implementation would probably break engine compatibility.

The code for different categories of thing (weapon/player/buildable/etc.) all randomly works differently for no reason. […]

The purpose is precisely to track down and reduce some of those discrepancies. It's a step forward unifying the features and code. At some point after doing steps like this after other steps like this we may become able to make reusable functions usable for all models kinds.

2. Compatibility with legacy assets

We have different models that have a different rotation in static position (when displayed without animation) than in animations. Basically it happens that some ouf our modelers, when they had to rotate their model to fit in the game, rotated the animation instead of the model.

The medistation is one of them, to be displayed properly in both netradiant and in game, it is orientated at asset build time in the way the base position (not animated) is correct, since netradiant doesn't play animations. But then it is rotated in-game using modelRotation to workaround the fact the animations are doing rotations over its axis.

On the left, static position (no animation), on the right, animation position (first frame):

On the left is how those models are displayed on every model editor/viewer not playing any animations, on the right is how it would display when animated in game if we don't workaround the model rotation in game.

I don't know why the medistation animation is rotated (we can do it in game), but the fact the human animation is rotated is very likely a bad workaround from the animator because of the lack of modelRotation in .model.cfg files.

We now have a tool that can rotate a IQM model with its skeleton and animations at build time, but we did not have this at the time. The last people having put his hands onto the model before inclusion in game was the one who did the animation, and he likely received a non-animated model from the modeler that was using another projection convention and rotated the model at this point to fit the need of the game. But being an animator, he rotated the animation in his animation editor.

The medistation rotation is likely because of the same reason but with the animator not knowing there was a modelRotation feature available. For the human model, there was no modelRotation feature available anyway, so an animator rotating an animation is not really the animator's fault at this point: he did what he could with what he had.

It is important that the static position in a model is the right one. For buildables, it is because NetRadiant displays the model for buildables entities. For human models, the need for display in NetRadiant is less common, but it was not rare to bake player models as decorations in maps (there are usages that fit the lore), and both NetRadiant and q3map2 rely on the static rotation for display and baking.

One can also temporarily import the models in map editor to check the map compatibility with the it, like the model size, this is why I make efforts that the static pose of our models in dpk archives we distributes are always using the correct scale and rotation, even if the game has to scale/rotate it back if the animator made a mistake and scaled/rotated the animations.

It would be better to fix the animation rotation and scale, while keeping the static rotation and scale unmodified, but this requires skills that are harder to find. And we're lacking this skill for years now, even myself don't know how to rotate them separately.

3. Compatibility with third-party assets

It's good to provide a way for a modder to be able to import a model and make it compatible with the game just by writing two lines in the .model.cfg, without requiring a skill in modeling we even don't have and are even ourselves struggling to acquire for a whole decade.

[DolceTriade]

The cgame model code really sucks by the way.

That's my fault. I didn't know proper coding practices back then.

[DolceTriade]

Need to address comments in CG_Corpse too

[DolceTriade]

You should use ci->modelScale

[illwieckz]

The ci->modelScale value comes from the character.cfg, not from the .model.cfg file.

The ci->modelScale value is then sent to the engine for the in-engine scaling.

Setting a non-1 scale in both character.cfg and cmc->modelScale will probably multiply the scales, we may do the multiplication in cgame and delete the scaling code in engine, but that is out of scope of this PR.

[DolceTriade]

The model code is messed up right now. I recommend sticking with the status quo for now instead of having multiplied scales. We can refactor and bring consistency later.

[illwieckz]

I'm not saying we want to multiply the scale, I'm just guessing what is happening if both mechanisms exist and are used.

Actually the modelScale in character.cfg looks like a hack. Quoting the human model character.cfg:

// modelScale is used to customize player models made originally for Quake4
// Quake4 player models are taller than Q3A player models and need to be sized down
// by a factor of 0.83 in each coordinate axis
// if modelScale is not specified it defaults to 1 1 1

There is no modelScale in ioq3 neither in etlegacy. It looks to be a quirk inherited from XreaL. Doing it in character.cfg is hacky, we better do it in .model.cfg files like buildables and missiles already do. The implementation itself is hacky.

Actually once this is merged I will probably go next by NUKING the character.cfg's modelScale feature from both cgame and engine (and then multiplication will not be a concern).

[DolceTriade]

Then let's bring zOffset too, since that is a model characteristic in the character.cfg

[DolceTriade]

Only if not skeletal. Skeleton scaling is done in CG_TransformSkeleton.

[slipher]

The purpose is precisely to track down and reduce some of those discrepancies. It's a step forward unifying the features and code. At some point after doing steps like this after other steps like this we may become able to make reusable functions usable for all models kinds.

How can copy and pasting the same code to several places be good from this point of view? It just adds to the stuff that would have to be cleaned up to make reusable code.