So much old mods that can be broken by these sort of changes. 

I saw an interesting bug report on quaddicted by NoNameUser, here:

Also, "r_lerpmodels" is set to 1 by default, but the weapon models lose their smooth animations outside the start map; monsters are ok, though, and everything else seems fine too. I've had this problem with the weapons animations with other mods too (Warp Spasm, The Horde of Zendar, and others).

I was able to reproduce it in two big maps (warpd.bsp, zendar.bsp) on various Fitz engines (0.85, QS 0.90.0, MarkV). What was happening was, this condition in CL_ParseClientdata was firing every frame, even when I wasn't changing weapons:

if (cl.viewent.model != cl.model_precache[cl.stats[STAT_WEAPON]])
{
cl.viewent.lerpflags |= LERP_RESETANIM; //don't lerp animation across model changes
}


The cause was, the above check was done before handling the SU_WEAPON2 byte, so only the bottom 8 bits of cl.stats[STAT_WEAPON] were set, which would cause the test to always fail if the weapon model was > 255. I just moved the if() statement to the end of the function, seemed to fix up the issue; here's the diff.

Does that look sensible? The lerping if() block used to be inside
if (cl.stats[STAT_WEAPON] != i), but I think it should be safe to move it outside of that, because the lerping if() is also testing that the weapon changed. 

Good catch. Your fix makes sense to me. 

Sounds like a great catch. 

Is there a way to code in so modders can choose which models can and cannot cast shadows, and which weapons/enemies are lerped?

I am using a bunch of models in one of my maps and they cast unwanted shadows. 

Fitz/Quakespasm/Mark V all have r_nolerp_list.

Mark V has r_noshadow_list too. 

Huh... how do you add to this list? Rc file? Cfg file? 

They're just cvars so you set their values the same as any other.

It would probably be a usability improvement to have commands to add individual items to these lists rather than having to require the user (or moder) to respecify the entire list. 

yeah, it's not very user or modder friendly, but for modders adding it to quake.rc is the best idea. (alternate idea: use stuffcmd)

In the case of modders, you should be able to create a full list of what should have shadows and what should interpolate (since you wrote the mod!)

As a player you end up creating a really long list that applies to multiple mods, since most mods do not provide their own lists.

The ideal would be an extension to the model format or new fields on quakec entities for the modder to define the behavior for each model or entity. All of those things require engine support, plus either a tools change or protocol change.

Hence, the hacky config file method. 

who's idea was it to add all this stuff to console commands anyway? can we change this to some text file where we just put a full path in from the mod dir (eg: progs/flame.mdl) and anything in that text file is not lerped?

config files are infinitely easier for modders and users alike. 

necros, it already works like that, you can set up the list in your config file. 

Darkplaces has a EF_NOSHADOW flag (4096) for the 'effects' entity field that you can set from qc, but it won't work in protocol 666 which only has 1 byte for 'effects'.

r_noshadow_list sounds like a decent compromise, thinking of adding it to Quakespasm too. 

We got a bug report for QS that the changelevel triggers in Xmen TC are harder to hit than with winquake. The changelevel triggers are point entities with an mdl model set on them.

What's happening is in Fitzquake, calling setmodel() also sets the mins/maxs based on the contents of the mdl, while winquake just sets them to '-16 -16 -16' '16 16 16'.

The Xmen code was calling setmodel() as the last thing in the setup code for this entity (xmen_teleport), so they were relying on the hacky behaviour of vanilla quake to set a 32x32x32 bbox.


Anyway - just thought I would document this for future reference; I suppose there could be a sv_gameplayfix cvar to revert to Winquake compatible behaviour, but it doesn't seem very urgent - lol. 

Oh... That would explain some seriously annoying issues I had with qc in the past 

sv_gameplayfix_setmodelrealbox
if 1, uses the model's bbox. if 0, uses +/- 16 for mdls.
just using +/-16 for *.mdl means that dedicated servers need not bother loading mdls at all, note that the vanilla quakeworld server does not support loading .mdl at all.
FTE uses +/-16 purely based upon the filename extension, while uses the real size in other cases. This means that replacement content still gets a suitable size with things that were originally .bsp (where they always got the bsp's size) while .mdl entities get their +/-16, and convieniently sidesteps issues that are so problematic in DP with replacement content.
Depending upon the filename extension like that is a bit of a hack, but it does indicate the original expectation much better than anything else.

quakerally is similarly (and severely) broken by mdls not getting size +/-16. 

That would explain some seriously annoying issues I had with qc in the past

Same! I figured it out after vtos() printing sizes of misbehaving things in desperation and getting non-power-of-2 dimensions. I started always calling setsize after setmodel assuming it was another QC quirk everyone knew about but me, and not something specific to Fitzquake. 

Haha, ages ago Spike and some QuakeC modders discussed that.

I did notice that anomaly in the FitzQuake source code last year when carefully comparing versus WinQuake.

I thought about it 10 seconds, and decided no one ever complains about FitzQuake.

This may be the first time someone has spotted a behavior difference in the wild and brought to someone's attention. 

Personally I'm baffled that engines still use this style of MDL bbox handling when all the code needed to handle them properly - including full rotation support - is there in Quake 2 - https://github.com/id-Software/Quake-2/blob/master/ref_gl/gl_mesh.c#L373

It's not hugely difficult to port that to Q1 and all of these problems just go away. 

I don't QuakeC much, but I've listened to plenty of conversations of QC.

I always thought in QuakeC, you were *always* supposed to do setmodel and then setsize.

But QRally doesn't, it would seem. And perhaps above comments from Necros and Lunaran indicate they don't always do setmodel and setsize too? And other mods may have made similar mistakes.

@metslime ...

Could you describe what FitzQuake was solving with the sizing the model?

Spike advocates all engines using the -16,-16,-16 to 16,16,16 but ...

I know you *never* implemented anything without a reason.

You made one of the most conscientious and thorough engines typically having a broad set of knowledge from a great number of sources. (The change wasn't accident or an oversight, but rather to solve something).

Was the issue for place_model, misc_model, mapobject_custom type of entities? 

iiuc it fixes glquake's model culling.

on the other hand, software rendering has more complicated culling, hence how it got away with hacking the size. I ought to verify that because frankly its an assumption.

and yes, qrally is NOT nice code. it violates other obscure rules too.

using +/-16 means that the server can avoid even loading the mdl in the first place. that's the biggest advantage. :) 

I am also curious about Baker's question.

Spike mentioning culling made me think.. if the server-side bbox is the +-16 units default, but the MDL is really huge (e.g. vermis), you could have a situation where the player can see the tip of the MDL, but the entity origin is outside of the player's PVS so the server culls it (wrongly).

It does seem like the main advantage of fitz's approach is for mapobject_custom type things, so very large MDL's can be supported without the mapper having to manually input the bbox size. 

I meant client-side frustum culling, rather than serverside pvs culling. 

Look at Quake 2's alias model culling code. It's easily adaptable (the key difference is that Quake 2 stores a bbox per frame) and handles rotation better than the FitzQuake code.

If it looks weird on first encounter then I'd encourage you to cross-check with modelgen.c, specifically the code it uses to scrunch the position data down to bytes. 

If you're talking about Mod_CalcAliasBounds the purpose is to calculate the bounds for the client.

GLQuake has a bug where large models are culled incorrectly, for example a dead shambler can disappear if you turn to a certain angle even though his arm should still be on screen. So my goal was to fix that.

e->model->mins/maxs are used by client for frustum culling, and that is what my change affects. ent->v.mins/maxs are used by server for PVS culling, and that is controlled by PF_setsize which I did not modify. 

@metslime -- the change isn't setsize actually, it's setmodel. I triple checked and ... well ... it is different.

I'd like to go the Spike route of -16,-16,-16,16,16,16 since it is standard Quake default.

But if even one FitzQuake targeted single player release would act up ... *sigh*

DarkPlaces defaults to original behavior with that sv_gameplayfix_setmodelrealbox 0 ... but I think many people if they heard a "DarkPlaces don't work right" with this 5 year old release would automatically blame DarkPlaces.