DarkPlacesEngine / DarkPlaces

The DarkPlaces Quake engine, created by LadyHavoc. Official Git repository (replaces SVN).
https://icculus.org/twilight/darkplaces/
GNU General Public License v2.0
273 stars 39 forks source link

Current beta - gamedir change corrupts cvars #101

Closed Baker7 closed 7 months ago

Baker7 commented 11 months ago

This relates to attempts to build on Windows with current darkplaces master. As I understand it, building on Windows is not really supported at this time. And I knew that going in ...

Current beta - gamedir change corrupts cvars.

If I type "gamedir undergate", the menu_progs cvar is reset to "default" which is corrupted data.

I built current DarkPlaces beta using Visual Studio 2022 and this is running on Windows 10 with a GeForce GTX 960M. . . . .

Since this is not the preferred method to build, this could be my fault for several different reasons including that the .vcxproj seemed to be out of date and I had to mess with it some, it could be missing option or compiler flags.

void Cvar_RestoreInitState(cvar_state_t *cvars) if (c->defstring) Z_Free((char *)c->defstring); c->defstring = Mem_strdup(zonemempool, c->initstate->defstring); if (c->string) Z_Free((char *)c->string); c->string = Mem_strdup(zonemempool, c->initstate->string);

(The documented msys2 method to compile on Windows works, but the .exe will not start and has the message "Application failed to start" ... I expect I can get it to produce a functional .exe with some messing around.)

Baker7 commented 11 months ago

I've tried left and right to get current DarkPlace beta master to compile either in Visual Studio or using mingw64 through CodeBlocks mirroring the same cflags in the makefile.

It does compile. But with certain kinds of serious run-time issues for certain things.

On gamedir change in client, like "gamedir undergate", cvar strings are trashed if -ffinite-math-only is removed. Mingw64 does not appear to trash the cvars on gamedir change in client if -ffinite-math-only is present ...

However ... no combination of mirroring the flags in the makefile that I could assemble seemed to produce an .exe didn't have some sort of issue whether screwing up ssqc/csqc SendEntities.

(Visual Studio, the same but a bit worse. Seems to have no exact equivalent for -ffinite-math-only, no way to avoid the trashed cvars. I don't know the specifics of why they get trashed, I focused on trying different build methods to mimic the compiler options.)

With issues like this, it could be a compiler flag or even my processor or an obscure sizeof issue ...

What I was hoping to do is carefully recode most of the changes made in the Zircon engine with the current DarkPlaces beta so DarkPlaces could acquire some/most/all of them.

Are there instructions on how to compile a Windows .exe on Linux?

hemebond commented 11 months ago

Are there instructions on how to compile a Windows .exe on Linux?

Have a look at .travis.yml and other travis files for info on how to do it.

Baker7 commented 11 months ago

I've tried a number of different ways to make a functioning a windows .exe from DarkPlaces master and the DarkPlaces apr 19 version.

Some methods do make an .exe that runs, ...

All of them get some sort of memory corruption on gamedir change.

The "menu_progs" cvar gets it's value messed up and so do many others. "menu_progs" is volunteering itself as the litmus test to see if memory got messed up.

I've used many methods, the MSYS2 method, etc.

I can make a Linux binary that functions just fine without any memory corruption issues that I have seen and by all accounts the Linux binary I build is just fine.

(Why the April 19 version ... as far as I can tell, the collision stuff in the current DarkPlaces beta causes droptofloor issues on a map I made, my gamedir impaired builds of DarkPlaces beta April 19 do not have issue.).

Other random notes: vid_mouse 0 is described as disabling the mouse in windowed mode. On Linux at least, it does not fully disable the mouse, I can still look around with it.

On any fullbright map --- a map with no light data -- I see a lot of solid black textures. AFAIK, only the sky and Quake textures with fullbright pixels render.

I post pics later. 1) droptofloor issues (not present on April 19 build) 2) textures in fullbright maps (this exists in the April 19th build)

Baker7 commented 11 months ago

Here at least is a partial fix ..

In Cvar_RestoreInitState, c->initstate is a memory copy of the cvar struct.

c->initstate->string is not a copy of the string, it is pointer to the same string.

Cvar_RestoreInitState is freeing the original string (c->string) and then allocating a copy from c->initstate->string --- which is the same freed string as c->string.

This partial fix addresses the above.

            Con_DPrintf("Cvar_RestoreInitState: Restoring cvar \"%s\"\n", c->name);
            const char *s_copy_str = NULL;
            const char *s_copy_defstr = NULL;

            if (c->string)      s_copy_str = Mem_strdup(zonemempool, c->initstate->string);
            if (c->defstring)   s_copy_defstr = Mem_strdup(zonemempool, c->initstate->defstring);
            if (c->defstring)
                Z_Free((char*)c->defstring);
            c->defstring = s_copy_defstr;//   Mem_strdup(zonemempool, c->initstate->defstring);
            if (c->string)
                Z_Free((char*)c->string);
            c->string = s_copy_str; // Mem_strdup(zonemempool, c->initstate->string);
        }
        c->flags = c->initstate->flags;
        c->value = c->initstate->value;
        c->integer = c->initstate->integer;
        VectorCopy(c->initstate->vector, c->vector);
        cp = &c->next;

And also this further down ...

        for (cp2 = &cvars->hashtable[hashindex]->cvar;(c2 = *cp2);)
        {
            if (c2 == c) {
                if (cvars->hashtable[hashindex]->next)
                    *cp2 = cvars->hashtable[hashindex]->next->cvar;
                else *cp2 = NULL; // Baker: there is no next ..
                break;
            }
            else
                cp2 = &cvars->hashtable[hashindex]->next->cvar;
        }

This seems to get DarkPlaces to completely survive one gamedir switch.

However, the problems start to occur on the 2nd gamedir switch, now that I think about it updating c->initstate->string / c->initstate->defstring to c->string / c->defstring after the mem_strdup might fix because those are stale.

Maybe Linux is rather lax with free and you can get away with doing a strdup of previously freed memory if you do it right away, Visual Studio in debug mode trashes the stale memory right away, perhaps to make bugs like these hurt immediately.

Also gamedir switching is not something everyone does.

No reason to do it in Xonotic or Doombringer, I imagine. Or maybe I'm wrong.

But for single player Quake, far more common to want to switch to a different episode or such.

Baker7 commented 11 months ago

updating c->initstate->string / c->initstate->defstring to c->string / c->defstring after the mem_strdup might fix because those are stale.

No, there is still something else going on.

cvars->hashtable issue with null pointer to cvar on 2nd gamedir change.

Baker7 commented 11 months ago

These 2 changes together appear to make it so unlimited gamedir changes work.

Part 1

    if (c->initstate)
    {
        // restore this cvar, it existed at init
        if (((c->flags ^ c->initstate->flags) & CF_MAXFLAGSVAL)
            || strcmp(c->defstring ? c->defstring : "", c->initstate->defstring ? c->initstate->defstring : "")
            || strcmp(c->string ? c->string : "", c->initstate->string ? c->initstate->string : ""))
        {
            Con_DPrintf("Cvar_RestoreInitState: Restoring cvar \"%s\"\n", c->name);
            const char *s_copy_str = NULL;
            const char *s_copy_defstr = NULL;

            if (c->string)      s_copy_str = Mem_strdup(zonemempool, c->initstate->string);
            if (c->defstring)   s_copy_defstr = Mem_strdup(zonemempool, c->initstate->defstring);
            if (c->defstring)
                Z_Free((char*)c->defstring);
            c->defstring = c->initstate->defstring = s_copy_defstr;//   Mem_strdup(zonemempool, c->initstate->defstring);
            if (c->string)
                Z_Free((char*)c->string);
            c->string =  c->initstate->string = s_copy_str; // Mem_strdup(zonemempool, c->initstate->string);

        }

Part 2 (I think this need tweaked some more, I'm not checking if we are the head ... and I doubt this code is quite right, the important part to me is that it doesn't crash with unlimited gamedir changes so that means getting the following code exactly right will stick a nail in this problem)

        for (cp2 = &cvars->hashtable[hashindex]->cvar; (c2 = *cp2); )
        {
            if (c2 == c) {
                if (cvars->hashtable[hashindex]->next)
                    *cp2 = cvars->hashtable[hashindex]->next->cvar;
                else cvars->hashtable[hashindex] = NULL; // Baker: there is no next ..
                break;
            }
            else
                cp2 = &cvars->hashtable[hashindex]->next->cvar;
        }

After both adjustments, we seem to survive unlimited gamedir changes.

Needs more testing.

Cloudwalk9 commented 11 months ago

Building on Windows is supported. I just haven't bothered writing the instructions for Visual Studio in the README yet.

Baker7 commented 11 months ago

Hey Cloudwalk, thanks for the reply. Kleshby wrote a tutorial which is pretty good and walks through the NuGet / SDL2.

https://quake.by/tutorials/?id=52

I want to point out, I was able to build using MinGW64 using CodeBlocks but neither the MingGW64 compiler that comes with MSYS2 or CodeBlock 20.03 works with current DarkPlaces beta. (My CodeBlocks adapted from the Xonotic's .cbp).

I had to use x86_64-8.1.0-release-posix-sjlj-rt_v6-rev0\mingw64 compiler, which I got from the bowels of the MinGW64 site somewhere.

And I got it to work after getting all the cflags to mirror the MSYS2 compile. Also the SDL2 is very sensitive to the order of linker params on Windows. One order yields a .exe that will not start, another order makes one that runs.

Baker7 commented 11 months ago

Visual Studio 2022 hates the pre-existing freetype-6.dll for Windows and throws an exception in debug with ntdll.dll.

The following freetype.dll made by the FreeType project in Visual Studio does not throw an exception:

https://github.com/ubawurinna/freetype-windows-binaries/tree/master/release%20dll/win64

That page is linked from FreeType at: https://freetype.org/download.html at "Windows DLLs of FreeType compiled with Visual Studio can also be downloaded directly from a github repository."

bones-was-here commented 7 months ago

I rewrote much of the cvar state restoration and fixed old gamedir bugs in 32c99c7b0a58545b273fa713d81d6bbfb3b8ebe6 5db369bea3d0cdd3dbd3b5b8038a399f0dbf84b6 7b7009740d891a5454c39106459ec9c63b74aa10 so it should be reliable in all supported games now.

The best maintained way to build for Windows is MinGW because that's what Xonotic uses, and because unlike Microsoft, GCC supports current C standards. You can find x86_64 binaries including DLLs in the engine zip at https://beta.xonotic.org/autobuild/ which are all cross compiled using MinGW and updated when commits are pushed to master.