Mod_LoadAllSkins - Bug Fix

At the start of Mod_LoadAllSkins:

  Code:

skin = (byte *) (pskintype + 1);

For single skins:

  Code:

Mod_FloodFillSkin (skin, pheader->skinwidth, pheader->skinheight);

For group skins:

  Code:

Mod_FloodFillSkin (skin, pheader->skinwidth, pheader->skinheight);

What will happen here is that for single skins, only the first skin in the model will get flood-filled; for group skins it's worse - nothing gets filled and instead it scribbles all over the memory for the group skin info (fortunately after reading the necessary info, otherwise it's a potential crasher) - it may also mess up the texel data.

Change single skins to:

  Code:

Mod_FloodFillSkin ((byte *) (pskintype + 1), pheader->skinwidth, pheader->skinheight);

And group skins to:

  Code:

Mod_FloodFillSkin ((byte *) (pskintype), pheader->skinwidth, pheader->skinheight);

So u mean that skin = (byte *)(pskintype + 1); is defined out of the loop and therefore doesnt increment to the next slot?

I realy havent seen any animated skingroups on alias models, but i do see the pskintype++ in the group portion, but should we also change
at the top

  Code:

skin = (byte *)(pskintype + 1);


to

  Code:

skin = (byte *)(pskintype);


?

So u mean that skin = (byte *)(pskintype + 1); is defined out of the loop and therefore doesnt increment to the next slot?

Yup.

...should we also change...

No need because that variable is no longer used.

Rook: I don't have time to look now, but IIRC, TF (MegaTF?) had at least one model with skin groups (one of the grenade types).

MH: It's great that you're posting these. Though I suspect it's one of the first things many fix, it might be good to mention the fun for loop bug in CL_UpdateTEnts (for (i = 0;...) { ... for (i = 0; ...)). That one took me 3 days of learning how to use gdb to fix (first real bugfix in QF). I found it when launching the cluster rocket near the lightning trap near the end of rogue's r1m4 (the earthquake/cave map: "Cave of Death").

I have to go to work or I'd post more details :/

It took me close on 8 years to even be aware of the for loop - I still have a check at the start of CL_UpdateTEnts that I suspect isn't necessary any more but I'm paranoid about removing...

A nice find for certain.

Wow, never knew of skingroups for alias models. Dug out the syntax from quakeforge git, modelgen.txt
See $skingroupstart. Wonder what $sync does?

  Code:

$modelname <name>         #output name
$base <name>            #basis model
$cd   <path>               #source path base
$sync
$origin <X> <Y> <X>         #model origin offset
$eyeposition <X> <Y> <Z>   #self explanatory :)
$scale <scale>            #scale factor
$flags NUM               #entity effects
   1   EF_ROCKET
   2   EF_GRENADE
   4   EF_GIB
   8   EF_ROTATE
   16   EF_TRACER
   32   EF_ZOMGIB
   64   EF_TRACER2
   128   EF_TRACER3
   4096 EF_GLOWTRAIL   (qf ext? nehahra?)
$frame <name>+            #frame grabbing
$skin <name> [interval=0.1]   #skin grabbing
$framegroupstart         #auto-animated frames
   $frame <name> [interval]
   $framegroupend
$skingroupstart            #auto-animated skins
   $skin
   $skingroupend

I discovered skin groups for alias models when a model Ceruix made used them.

(I also noticed the gl code in most engines doesn't seem to bother saving their skins for potential coloring, but I thought fixing that would be a waste of time as the feature is almost never seen.)

So Basically, its for multiple skins that can also be colored for teams?
Ive not seen a prob in CTF, or using a skin mod with skin slots.... :/ logically it looks bugged, i just never noticed.

So Basically, its for multiple skins that can also be colored for teams?
Ive not seen a prob in CTF, or using a skin mod with skin slots.... :/ logically it looks bugged, i just never noticed.

Skins groups aren't a model having multiple skins. That's somewhat common.

Skins groups are a series of up to 4 skins that represent one skin --- it is in effect an "animated skin". Like how some textures in a map that begin with "+" alternate.

If what I am describing is making you confused, it should. Odds are, you've never seen the feature used before. Ever.

[And it is difficult to come up with ever an idea on how using the feature would be beneficial, external replacement textures for them get all weird like player.mdl_0_2.tga, and considering the feature is tuned for 10 fps or something like that, it isn't going to be smooth unless someone invents texture animation intepolation (and what can you really do with 4 frames of texture animation? Maybe make a fire monster look a little fiery or a robot with blinky lights or something, but it won't look good chances are. And think of the maintenance for a feature no one actually uses.]

'up to 4' is a glquakeism
one of the mission packs has an eel. that's the only official use of skingroups that I can think of.
like framegroups, the frame used is non-linear with time. unless you're using glquake which is again broken.
if you're making a model that uses them, don't expect all engines to be as broken.

CuTF uses skingroups on the telepads to give a bit of animation. These entities also use colormaps, so colourmapping skin groups is indeed useful (albeit painful).
Tbh if its colourmapped you can get away with just ignoring the extra frames if it means you get colourmapping instead. Whether that's the 'correct' behaviour or not is a different matter.
I don't know about the grenades. For lights blinking on their sides it makes sense that they would have framegroups.

"Lights blinking on the sides" is exactly the effect that was done... Or so I thought. I can't find the model [edit] Maybe it was fullbrights.

However, the Rogue mission pack has skin groups in timecore.mdl, sphere.mdl, and p_shield.mdl.

That said, I did fix skin groups (both arbitrary limit and bogus timing) for gl quake back in 2001. No idea why other than "oi, that's wrong".

For the curious:

  Code:

commit 7d1401304155368c3d298e21ce1e605556c4898d
Author: Bill Currie <bill@taniwha.org>
Date:   Wed Nov 21 08:14:05 2001 +0000

    fix many, many bugs in the alias skin loading, transforming and rendering
    code. This fixes blather's `melted models' (sw), the nq alt player model
    skins (gl), the arbitrary limits on skins and skin groups in gl, and the
    incorrect timing of group skins (animated) in gl.


Nowadays there's nothing much you can do with skin groups that you couldn't otherwise do with multiple skins and some QC. It's a little more bandwidth usage for sure, but utterly minimal and gives the modder more control.

John Carmack's take on them: http://www.team5150.com/~andrew/carmack ... #d19970707

Anatomy of a mis-feature:

As anyone who has ever disected it knows, Quake's triangle model format is a mess. Any time during Quake's development that I had to go back and work with it, I allways walked over to Michael and said "Ohmygod I hate our model format!'. I didn't have time to change it, though. After quake's release, I WANTED to change it, especially when I was doing glquake, but we were then the proud owners of a legacy data situation.

The principle reason for the mess is a feature.

Automatic animation is a feature that I trace all the way back to our side-scroller days, when we wanted simple ways to get tile graphics to automatically cycle through animations without having to programatically each object through its frames.

I thought, "Hmm. That should be a great feature for Quake, because it will allow more motion without any network bandwidth."

So, we added groups of frames and groups of skins, and a couple ways to control the timing and syncronization. It all works as designed, but parsing the file format and determining the current frames was gross.

In the end, we only used auto-frame-animation for torches, and we didn't use auto-skin-animation at all (Rogue did in mission pak 2, though).

Ah well, someone might use the feature for something, and its allready finished, so no harm done, right?

Wrong. There are a half dozen or so good features that are apropriate to add to the triangle models in a quake technology framework, but the couple times that I started doing the research for some of them, I allways balked at having to work with the existing model format.

The addition of a feature early on caused other (more important) features to not be developed.

Well, we have a new model format for Quake 2 now. Its a ton simpler, manages more bits of precision, includes the gl data, and is easy to extend for a couple new features I am considering. It doesn't have auto-animation.

This seems like an easy case - almost anyone would ditch auto-animation for, say, mesh level of detail, or multi-part models. The important point is that the cost of adding a feature isn't just the time it takes to code it. The cost also includes the addition of an obsticle to future expansion.

Sure, any given feature list can be implemented, given enough coding time. But in addition to coming out late, you will usually wind up with a codebase that is so fragile that new ideas that should be dead-simple wind up taking longer and longer to work into the tangled existing web.

The trick is to pick the features that don't fight each other. The problem is that the feature that you pass on will allways be SOMEONE's pet feature, and they will think you are cruel and uncaring, and say nasty things about you.

While I agree that automatic animations aren't worth it for cpu usage (here, QC cpu usage), they still present a nice savings for network bandwidth. That said, CSQC can do that (eg, tightly sandboxed QC code embedded in the model, even).

Automatic animations might be a mis-feature, but we're "stuck" with them for q1 data and we might as well get the relevant code paths right but keeping them suitably separate from code paths for other formats (eg iqm).

BTW, QF's IQM support reuses some of the code used for alias models (lerp parameter calculations, triangle rendering in sw), but I'm sure those are areas about which JC is not so regretful.

For what it's worth, xmen's burnt skeleton has a skingroup too

I use a skingroup in vs97 for the chainsaw's v_ model to scroll the teeth. I also use it to make a pulsating effect on the beam cannon