|
Post by knightmare on Apr 16, 2019 18:05:43 GMT -5
The multiple shutdown calls are something I've tangled with in the active development branch, and it's simply not worth it. I remember causing a crash once by removing one of them. They're there in the vanilla Q2 source as well and don't break anything, so I'm not going to bother changing this in the update branch. I may experiment again with cleaning this up in my dev branch, but it's not a priority.
Sys_Quit() is atually called in Sys_ConsoleProc() when the close/quit buttons are clicked, not a second time in Sys_Error().
The "colored text" you found being pinted in CL_ParseServerData() is actually the STX char (002) in the console before the map title. Colored text codes are already filtered out in the Sys_ConsoleOutput() function, but I could add the STX char as well. Fliltering the colored text codes out of the logfile and console dumps isn't needed for presentation purposes, as those are solely for debugging, and it's good to know that they're there.
The texture loading in r_model.c uses its own layer of failed caching because the R_FindImage() tries 4 formats for every call to it that specifies a .wal file. If the texture load completely fails, each of those failed texture files (tga, jpg, png, wal) has been added to a failed list. So it's faster to have a failed list in the world model loader that completely skips R_FindImage() if it's known that all possible formats for a texture can't be loaded. Otherwise it would be checking for 4 entries in a failed list instead of 1. The image name is still checked in the off chance of a hash collision.
That WM_VSCROLL handling code was in the original dedicated console code I borrowed from Quake2Evolved. I haven't audited every single thing it does.
Good find on the LoadGameCallback() for the autosave specifing a non-existent saveshot.
|
|
|
Post by MaxED on Apr 18, 2019 5:33:52 GMT -5
Thanks for the clarifications.
In my branch, I've removed Mod_CheckTexFailed logic, changed R_CheckImgFailed logic to use the hash of filename without extension, rewritten JPG/PNG/TGA loading logic to use stb_image library (YamagiQ2 also uses this) and rewritten R_FindImage to work without recursion. These changes allowed to reliably report missing images from R_FindImage (and that's how I found the bugs in saveshot loading logic). Another bug: "s_teamplay_box" value is not set in DMOptions_MenuInit.
|
|
|
Post by MaxED on Apr 22, 2019 3:24:45 GMT -5
In CL_RequestNextDownload():
// confirm existance of .tga textures, try to download any that don't exist if (precache_check == texture_cnt+2) { // from qcommon/cmodel.c extern int numtexinfo; extern mapsurface_t map_surfaces[];
if (allow_download->value && allow_download_maps->value && allow_download_textures_24bit->value) { while (precache_tex < numtexinfo) { char fn[MAX_OSPATH];
Com_sprintf(fn, sizeof(fn), "textures/%s.tga", map_surfaces[precache_tex++].rname); if (!CL_CheckOrDownloadFile(fn)) return; // started a download } } precache_check = texture_cnt+3; //precache_tex = 0; <-- Unlike similar blocks for downloading wal/jpg/png textures, this line is missing. }
|
|
|
Post by knightmare on Apr 23, 2019 18:41:26 GMT -5
Those last two are ones that were already fixed in the development branch, the first by the menu system rewrite. But it's good to know so that I can fix them in my stable branch.
|
|
|
Post by MaxED on Apr 24, 2019 3:39:28 GMT -5
At the end of CL_FootSteps():
if (loud) { if (volume == 1.0) S_StartSound (NULL, ent->number, CHAN_AUTO, stepsound, 1.0, ATTN_NORM, 0); else volume = 1.0; } S_StartSound (NULL, ent->number, CHAN_BODY, stepsound, volume, ATTN_NORM, 0); Is the sound supposed to be played twice when loud = true and volume = 1.0?
|
|
|
Post by MaxED on Apr 24, 2019 10:12:34 GMT -5
aliasLightDir used in R_LightAliasModel() is never assigned a value.
|
|
|
Post by knightmare on Apr 24, 2019 13:50:17 GMT -5
Yes, the footstep is played twice on on different channels to double the effective volume.
I added an assignment of {0,0,0} to the declaration of aliasLightDir.
|
|
|
Post by MaxED on Apr 27, 2019 17:39:09 GMT -5
SURF_DRAWSKY flag is never assigned, yet the code in RecursiveLightPoint and R_CreateSurfaceLightmap checks for it. In vanilla Q2 this flag is assigned only in software mode version of Mod_LoadFaces.
In some setups this can result in incorrect weapon lighting. For example, you can get this ( close to the ground, hovering slightly higher) at the beginning of the first map of Q2MP2 (rmine1). Looks like Rogue devs used brushes with "light", "nodraw" and "sky" flags to light the maps, and the code in RecursiveLightPoint can pick surfaces form such brushes.
I'm not entierly sure what's the correct way of handling this setup is. I've added checks for uninitialized lightmap size and SURF_NODRAW flag to RecursiveLightPoint, but I'm not sure whether to add SURF_DRAWSKY flag to surf->flags in Mod_LoadFaces.
|
|
|
Post by MaxED on May 2, 2019 7:35:11 GMT -5
In CL_Connect_f():
if (Com_ServerState ()) { // if running a local server, kill it and reissue SV_Shutdown (va("Server quit\n", msg), false); //mxd. How putting "msg" cvar into a string without a specifier character is supposed to work? }
|
|
|
Post by MaxED on May 3, 2019 16:02:21 GMT -5
file_from_pk3 var is never used.
|
|
|
Post by MaxED on May 4, 2019 17:19:19 GMT -5
FS_AddGameDirectory(): the directory name is added first to the search path (which means it's searched last). This results in incorrect maps.lst file (the one from pak0.pak instead of baseq2/maps.lst) loaded when using patched version of Q2 (like Quake II: Quad Damage from GoG). This also prevents overriding (KM)Q2 assets by placing files in baseq2 directory. Also, there's this bit in FS_AddGameDirectory():
if ( ( dirnames = FS_ListFiles( findname, &ndirs, 0, 0 ) ) != 0 ) { for ( j=0; j < ndirs-1; j++ ) //mxd. Why does this count to ndirs-1? { // don't reload numbered pak files ... } ... }
|
|
|
Post by knightmare on May 5, 2019 12:14:20 GMT -5
That's how Q2's pak files are supposed to work- they override all loose files in the directory. I'm not changing that. But I did add maps.lst to the blacklist of files that are ignored inside pak files.
I'll have to look into that sky brush lighting issue when I have time to experiment with it.
|
|
|
Post by MaxED on May 6, 2019 15:41:10 GMT -5
I did add maps.lst to the blacklist of files that are ignored inside pak files. And now, maps.lst won't be loaded for Rogue/Xatrix/any other mod with maps.lst in a .pak...
|
|
|
Post by knightmare on May 6, 2019 19:35:20 GMT -5
Ah crap, I forgot that the missionpacks included that file in the paks, and that an external maps.lst was only added in the "full" Q2 patches. I removed conflicting files from the Rogue and Xatrix .paks (leaving them loose in the mod folders) and moved their paks into baseq2 years ago.
After reviewing the official Q2 patches I have, vwep models and some CTF skins were included inside a pak2.pak as well for v3.18 beta and v3.19.
I'll need to add a new pakitem flag to try loading certain files externally first.
EDIT: that Yamagi hack blocks pak loading of those files under baseq2. I desire an approach that simply reorders searching. That will be tricky.
|
|
|
Post by knightmare on May 7, 2019 13:02:26 GMT -5
As far as the ndirs-1 thing, it skips the last entry because FS_ListFiles includes a null entry as a list terminator:
char **FS_ListFiles (char *findname, int *numfiles, unsigned musthave, unsigned canthave) { char *s; int nfiles = 0; char **list = 0;
s = Sys_FindFirst( findname, musthave, canthave ); while ( s ) { if ( s[strlen(s)-1] != '.' ) nfiles++; s = Sys_FindNext( musthave, canthave ); } Sys_FindClose ();
if ( !nfiles ) { *numfiles = 0; return NULL; }
nfiles++; // add space for a guard- this is a null terminator. *numfiles = nfiles;
list = malloc( sizeof( char * ) * nfiles ); memset( list, 0, sizeof( char * ) * nfiles ); . . . return list; } [/ocde]
|
|