Standalone md5 model support

[NHogweed]

Now, if there is a md5 model, but no mdl with the same name, the engine will use the former instead of returning an error. This ignores r_loadmd5models and r_md5models.

[vsonnier]

I @NHogweed, thanks for this. Now, the code around the selection of which model to load it getting too messy for my taste, especially with the goto addition :) I'm going to move the model selection to a separate function to be more explicit and readable.

No need to touble yourself further though, I'll take care of the rest from this.

[NHogweed]

OK. I'm keeping my code until that though, just for any case.

[vsonnier]

Hi @NHogweed finally I merged your modification as-is. I wanted to be pedantic at first and redo this, but hey, I'm lacking time and as we say home : "le mieux est l'ennemi du bien" or if you prefer here Practicality beats Purity :)

Seriously though, your change was not that big, and is still easy enough to follow.

Thanks for your contribution !

[NHogweed]

I think it would be better for compatibility (at least with FTEQW and QSS) purposes to revert my changes to Mod_LoadModel (addition to Mod_Extradata_CheckSkin is still needed) and add lines 452 - 454 from QSS instead. The normal way to use standalone md5meshes in other engines is to refer to them as themselves in QuakeC. They can be referred to as mdls, but this is cvar-dependent.

[vsonnier]

Please provide another PR, because I don't see how the changes you mention could work and still be compatible with the

if (r_loadmd5models.value) {
... 
}

branch ?

[NHogweed]

Let's compare gl_model.c right before the latest change and what I propose now.

[vsonnier]

I think the previous md5_forced mod was fine, this last change do not work for MD5 standalone anymore.

We must keep the automatic override / standalone support irrespective of the real QuakeC name, else we loose the ability to use MD5 models packs simply.

Right, I think we can have both override and explicit MD5, I'll do something about it.

[NHogweed]

this last change do not work for MD5 standalone anymore

This is not what I saw after its compilation.

else we loose the ability to use MD5 models packs simply

Just tried RME for hipnotic, and it worked as usual.

You need to protect against loading MD5 twice

Really? As far as I understand,
char newname[MAX_QPATH];
q_strlcpy (newname, mod->name, sizeof (newname));
char *extension = (char *)COM_FileGetExtension (newname);
if (strcmp (extension, "mdl") == 0)
checks if the loaded file has .mdl extension. And
case (('M'<<0)+('D'<<8)+('5'<<16)+('V'<<24)):
Mod_LoadMD5MeshModel(mod, buf);
is a check for the loaded file's content. So md5 replacement won't be loaded for anything but a .mdl file, and if it is indeed an alias model, the corresponding option in the switch (mod_type) section will be used. I can imagine double md5 loading if md5's extension was changed to .mdl, but I doubt this is a reasonable thing to do.

I fail to see what is wrong with the new version of standalone md5 loading code, unless you think it is important to keep the older one. But even then, they can be combined.

[vsonnier]

Just tried RME for hipnotic, and it worked as usual.

Can't be, the MD5 of this package are invalid, ( andrei-drexler/ironwail#356 (comment) ) unless you have a particular version ?
They still crash on me, copying contents into QuakeEnhanced directory (a.k.a Remaster from GOG)

Now for the code, I think you are right:

The switch(mod_type) is used to dispatch to the appropriate loader but indeed md5_loaded can only be true if mod->name was a .mdl, so indeed we call Mod_LoadMD5MeshModel() at that place only for a true MD5, that was never called in the .mdl replacement option.
To be clearer, I'll rename md5_loaded into md5_replacement_loaded.

Following that logic, you proposed change should bring MD5 standalone support.

The previous code md5_forced allowed to load a standalone MD5 (without .mdl companion) while in QuakeC the specified model was .mdl.

That's how I tested your change using Q2M (#761) but this is not a valid situation, obviously the .mdl is supposed to be present if specified in QuakeC ! and the same thing applies for MD5.

[NHogweed]

Can't be, the MD5 of this package are invalid, ( andrei-drexler/ironwail#356 (comment) ) unless you have a particular version ?

I removed problematic models (proxbomb and wetsuit). Everything else worked fine. (UPD: well, actually there are 3 more models with broken anim files.)

To be clearer, I'll rename md5_loaded into md5_replacement_loaded.

Maybe it's a good idea to rename md5-related cvars too?

If you want to see the md5 option from switch (mod_type) in action, try launching the folder in the attached archive as a mod with r_loadmd5models and r_md5models disabled. There is a progs.dat file compiled from a remaster source (included) with links like "progs/v_weapon.mdl" changed to "progs/v_weapon.md5mesh" (in weapons.qc and world.qc). This works only in the game itself, not in demos from id1\pak0.pak.
test.zip

[vsonnier]

Thanks for your test example it works !
Added the change in commit 8dfa015

Maybe it's a good idea to rename md5-related cvars too?

Yes r_loadmd5models was replaced in r_allow_replacement_md5models, default to 1 but NOT saved, because by default we want replacement possible, AND in fact this is only taken into account at each new game / level, so changing it dynamically do not have the expected effect. So let's consider disabling replacements (0) as a debug-only feature.

r_md5models stays the same and is controlled by Models > Remastered / Classic.

Now if we provide MD5-only models, the Remaster / Classic setting makes no difference, as intended.