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
290 stars 42 forks source link

Items that get "crushed" by moving platforms can not be moved again #155

Open hemebond opened 7 months ago

hemebond commented 7 months ago

Trying to debug why the silver key in Zerstorer map zer1m3 never rides the platform up, I found that the map has a tiny func_door that moves down through the item_key1, squashing it, and preventing any other brush entity from moving it; the platform just moves through the key.

Before: zer20240330174847-00

After (you can see the tiny blue bounding box of the door at the bottom): zer20240330174920-00

In other Quake engines the item_key1 can still be moved so this doesn't actually break the map for them.

Here is a small test map. Use r_showbboxes 1 to see the behaviour. I can actually get it to work again if I reset the mins and maxs fields:

prvm_edictset server 7 maxs "16 16 32"
prvm_edictset server 7 mins "-16 -16 -24"

until it is crushes again.

I'm also confused as to why the bounding box is so damn big; it should only be 32x32x56

Baker7 commented 7 months ago

The culprit is in SV_Pushmove .. sv_phys.c

        if (PRVM_serveredictfloat(check, solid) == SOLID_NOT_0 || 
            PRVM_serveredictfloat(check, solid) == SOLID_TRIGGER_1)
        {
            // corpse
            PRVM_serveredictvector(check, mins)[0] = PRVM_serveredictvector(check, mins)[1] = 0;
            VectorCopy (PRVM_serveredictvector(check, mins), PRVM_serveredictvector(check, maxs));
            continue;
        }

I don't understand why anything except QuakeC has the right to change the mins/maxes of an entity, but perhaps I am overlooking something.

If I were to guess, I would think DarkPlaces might have did this at some point to accomodate blowing up fallen zombies. (Do zombies become solid_not for a while when knocked down? perhaps? I don't know.)

I'm not sure the purpose of changing the mins/maxs of a trigger area either.

My gut instinct would be to encase that with an if like ...

if (sv_gameplayfix_blowupfallenzombies.integer != 0)

And then go from there.

I looked through a classic Quake engine SV_PushMove and see no evidence that it messes with mins or maxs if it is not solid or a trigger.

sv_gameplayfix_blowupfallenzombies defaults 1 in the engine, but is ALWAYS set to 0 where it sets the defaults for GAME_NORMAL, so it is always off for Quake during normal operation.

Baker7 commented 7 months ago

Extra thoughts:

  1. Nice catch, Hemebond. I personally like to know about these DarkPlaces mysteries and this one was a good one.
  2. But your zip only a had .map (no .bsp), so I had to compile myself a bsp for your little test map.

"I'm also confused as to why the bounding box is so damn big; it should only be 32x32x56"

The size was always 32x32x56 during testing? I never saw the size change.

hemebond commented 7 months ago

If I were to guess, I would think DarkPlaces might have did this at some point to accomodate blowing up fallen zombies. (Do zombies become solid_not for a while when knocked down? perhaps? I don't know.)

They become non-solid. Their bounding box doesn't change. Changing sv_gameplayfix_blowupfallenzombies doesn't seem to have any effect on their bounding boxes.

  1. But your zip only a had .map (no .bsp), so I had to compile myself a bsp for your little test map.

Just habit. Normally helps if others can see how the map is built and make changes.

"I'm also confused as to why the bounding box is so damn big; it should only be 32x32x56"

The size was always 32x32x56 during testing? I never saw the size change.

I guess I'm just referring to what is rendered by r_showbboxes 1. It's much wider than it should be.

Baker7 commented 7 months ago

Well, I tested encasing it in an if clause.

However, the pusher in vanilla Quake returns.

Even with this, the pusher stops moving?

Your little test map is a gift that keeps giving.

hemebond commented 7 months ago

Even with this, the pusher stops moving?

No the func_train will go through the item_key1, causing it to fall through.

The .solid field doesn't seem to change:

server EDICT 7:
modelindex     32
absmin         '225 -31 -0.96875'
absmax         '287 31 57.03125'
movetype       6
solid          1
origin         '256 0 24.03125'
classname      item_key1
model          progs/w_s_key.mdl
mins           '-16 -16 -24'
maxs           '16 16 32'
size           '32 32 56'
touch          key_touch()
think          PlaceItem()
items          131072
netname        silver key
flags          768
noise          misc/medkey.wav
mdl            progs/w_s_key.mdl
]prvm_edict server 7

server EDICT 7:
modelindex     32
absmin         '241 -15 -0.96875'
absmax         '271 15 1.03125'
movetype       6
solid          1
origin         '256 0 24.03125'
classname      item_key1
model          progs/w_s_key.mdl
mins           '0 0 -24'
maxs           '0 0 -24'
size           '32 32 56'
touch          key_touch()
think          PlaceItem()
items          131072
netname        silver key
flags          768
waterlevel     -1
watertype      -1
noise          misc/medkey.wav
mdl            progs/w_s_key.mdl
Baker7 commented 7 months ago

I'm pushing the right button only at this time.

When I can get that to act like Quake, I'll push more buttons like the left one, haha.

Baker7 commented 7 months ago

Extra info for topic:

"sv_gameplayfix_blowupfallenzombies", "1", "causes findradius to detect SOLID_NOT_0 entities such as zombies and corpses on the floor, allowing splash damage to apply to them"

hemebond commented 7 months ago

This actually seems to fix it quite nicely by preventing the bounding box from being resized.

@@ -1931,11 +1932,12 @@ static void SV_PushMove (prvm_edict_t *pusher, float movetime)
                        // fail the move
                        if (PRVM_serveredictvector(check, mins)[0] == PRVM_serveredictvector(check, maxs)[0])
                                continue;
                        if (PRVM_serveredictfloat(check, solid) == SOLID_NOT || PRVM_serveredictfloat(check, solid) == SOLID_TRIGGER)
                        {
                                // corpse
-                               PRVM_serveredictvector(check, mins)[0] = PRVM_serveredictvector(check, mins)[1] = 0;
-                               VectorCopy (PRVM_serveredictvector(check, mins), PRVM_serveredictvector(check, maxs));
+                               // PRVM_serveredictvector(check, mins)[0] = PRVM_serveredictvector(check, mins)[1] = 0;
+                               // VectorCopy (PRVM_serveredictvector(check, mins), PRVM_serveredictvector(check, maxs));
                                continue;
                        }
Baker7 commented 7 months ago

The "continue;" part is important it seems. Now it acts like Quake so far for me.

Just want to point out that from my perspective it should be if blow up fallen zombies, LadyHavoc did a lot of work to make it so you blow up corpses and stuff -- at least as an option and not necessarily in Quake, but are at least able to do that in mods.

It is possible stuff like Quake 1.5 uses it? (Checked, it appears to use it.)

hemebond commented 7 months ago

Just want to point out that from my perspective it should be if blow up fallen zombies, LadyHavoc did a lot of work to make it so you blow up corpses and stuff -- at least as an option and not necessarily in Quake, but are at least able to do that in mods.

I tested the sv_gameplayfix_blowupfallenzombies 1 in e1m3 and it still worked with these lines commented out. The fallen zombie is non-solid, but still gibs when a grenade explodes next to them.

The only artifact that I can see from removing this code is that corpses are no longer ignored and will teleport onto platforms when crushed. If there is no space above the pusher, such as a crusher, they bounce a little when the crusher moves back up.

Baker7 commented 7 months ago

Corpses are probably SOLID_NOT_0, the key was SOLID_TRIGGER_1.

You might consider seeing what happens if you kept the old behavior for solid not?

hemebond commented 7 months ago

If we only continue on SOLID_NOT, the key blocks the train from moving further. If we only continue on SOLID_TRIGGER then corpses block the train from moving further.

Baker7 commented 7 months ago

I was suggesting flatten, the continue must stay. For certain in all cases for both solid not and solid trigger (based on what I can tell so far and off your experiments ...)

However, I want to point out if I pick up a weapon / key / whatever ... it goes to SOLID_NOT until regen .. and this flattening thing occurs to the intangible unseen ghost in deathmatch and sometimes coop ..

What I am seeking to do personally ... 1) Have blowupfallenzombies 1 (current behavior to retain full compatibility for DarkPlaces 2014 like with Quake Combat + and Quake 1.5 and whatever else).

2) Figure out how it should behave with Quake and default that for Quake.

3) And have this new 3rd behavior be able to be turned off and go back to current behavior for testing convenience in case there is something elusive not being considered.

So I want to play with corpses and lifts, probably E1M1 soldier is good place to look for now.

(Case in point, I thought the fiend fix in DarkPlaces Beta was "safe", implemented in Zircon as cvar. Seemed to play right ... then someone said "corpses are drifting in Quake Combat Plus" and fiend jump fix was reason ... so ...)

Baker7 commented 7 months ago

I'm sorry, I messed up ...

Quakespasm is doing this too ... (and it is in the original Quake source code release).

Quakespasm is flattening X and Y and centering the Z.

I was trying to locate the origin of this code in DarkPlaces and it seemed to always have it and so did FTE ... so I had to double check everything ...

    if (block)
    {   // fail the move
        if (check->v.mins[0] == check->v.maxs[0])
            continue;
        if (check->v.solid == SOLID_NOT || check->v.solid == SOLID_TRIGGER)
        {   // corpse
            check->v.mins[0] = check->v.mins[1] = 0; <-----------------------------
            VectorCopy (check->v.mins, check->v.maxs);
            continue;
        }

        VectorCopy (entorig, check->v.origin);
        SV_LinkEdict (check, true);

        VectorCopy (pushorig, pusher->v.origin);
        SV_LinkEdict (pusher, false);
        pusher->v.ltime -= movetime;

        // if the pusher has a "blocked" function, call it
        // otherwise, just stay in place until the obstacle is gone
        if (pusher->v.blocked)
        {
            pr_global_struct->self = EDICT_TO_PROG(pusher);
            pr_global_struct->other = EDICT_TO_PROG(check);
            PR_ExecuteProgram (pusher->v.blocked);
        }

I never was aware of this flattening until today! I had no idea such a thing was normal Quake behavior.

hemebond commented 7 months ago

With just this change the key will get squashed, but will be still be pushed around, making it work just like Quakespasm with one small difference: when the platform moves down again Quakespasm pushes it through the floor, whereas DarkPlaces does not.

@@ -1846,8 +1855,8 @@ static void SV_PushMove (prvm_edict_t *pusher, float movetime)
                        //trace = SV_TraceBox(PRVM_serveredictvector(check, origin), PRVM_serveredictvector(check, mins), PRVM_serveredictvector(check, maxs), PRVM_serveredictvector(check, origin), MOVE_NOMONSTERS, check, checkcontents);
                        if (!trace.startsolid)
                        {
-                               //Con_Printf("- not in solid\n");
-                               continue;
+                               Con_Printf("- not in solid\n");
+                               // continue;
                        }
                }

If I change the platform size to only cover half the key, like this:

image

In Quakespasm the platform stops interacting with it after it's crushed, but the horizontal train can still push it. DarkPlaces will nudge the key off the platform when it gets crushed, since it's only half on the platform.

I also tested this change with squashing disabled and the behaviour was quite odd. So far I think disabling the squashing gives the best behaviour.

hemebond commented 7 months ago

@bones-was-here Which solution would you prefer? The squashed bbox like other engines, or non-squashed box (my preference since it makes it still behave well in my limited testing).

Baker7 commented 7 months ago

Changing this is super dangerous unless you are doing extensive compatibility testing.

It is almost a guarantee this will break something.

LadyHavoc tried to limit the risk by making changes into sv_gameplay fix cvars so they could be turned on and off.

hemebond commented 7 months ago

It is almost a guarantee this will break something.

It's technically already broken since it behaves differently to regular Quake. Making it squashed but solid matches other engines.

Baker7 commented 7 months ago

LadyHavoc made this kinds of alterations toggle with a sv_gameplayfix_different_squish 1 or 0.

This helped testing and you could turn it on or off to test the effect.

That was a solid engine coding practice, and making a cvar toggle is easy.

hemebond commented 7 months ago
@@ -1846,8 +1855,8 @@ static void SV_PushMove (prvm_edict_t *pusher, float movetime)
                        //trace = SV_TraceBox(PRVM_serveredictvector(check, origin), PRVM_serveredictvector(check, mins), PRVM_serveredictvector(check, maxs), PRVM_serveredictvector(check, origin), MOVE_NOMONSTERS, check, checkcontents);
                        if (!trace.startsolid)
                        {
                                //Con_Printf("- not in solid\n");
-                               continue;
+                               // continue;
                        }
                }

This change causes horizontal movers/pushers, like doors, to drag the player along if they're touching it.

Not an issue when disabling squash entirely. Also tested checked e3m2 (the silver key is lowered through a hatch as the player approaches) and while it's moved a little it still goes down as expected.

I had a look at adding a gameplayfix cvar but they're all over the show.

Baker7 commented 7 months ago

You just put each component of the change in an if block, like

if (sv_gameplayfix_this_one.value) ...

That way you can toggle it. Obviously, you also register a cvar too.

hemebond commented 7 months ago

You just put each component of the change in an if block, like

if (sv_gameplayfix_this_one.value) ... That way you can toggle it. Obviously, you also register a cvar too.

That's the easy part. Registering the cvar was the complex part. I had a quick look but I can't be bothered right now.

Baker7 commented 7 months ago

Make a cvar like copy this line in sv_main.c with the new one right below (or find a place alphabetical to insert it), then name it as desired.

cvar_t sv_gameplayfix_downtracesupportsongroundflag = {CF_SERVER, "sv_gameplayfix_downtracesupportsongroundflag", "1", "prevents very short moves from clearing onground (which may make the player stick to the floor at high netfps), fixes groundentity not being set when walking onto a mover with sv_gameplayfix_nogravityonground"};

Register it as such ...

Cvar_RegisterVariable (&sv_gameplayfix_downtracesupportsongroundflag);

Goto server.h and add it to extern list ... because in this case it is likely to be needed by a different source file.

extern cvar_t sv_gameplayfix_downtracesupportsongroundflag;

Baker7 commented 7 months ago

I'm adding this as a cvar to Zircon Engine as sv_gameplayfix_hemebond_pushmove and turning it on -- while testing.

I'll try some different stuff including rotating doors and some mods and see how it goes.

Baker7 commented 7 months ago

Observation: With sv_gameplayfix_hemebond_pushmove 1

If I am standing beside a func_plat going up (I'm not standing on it), I get bounced a little.

I'll upload pic here later on

Baker7 commented 7 months ago

I get somehow "pulled up" a little if I stand within a "meter" of the platform moving up.

hemebond_pulled_up

If I turn off the cvar I made, "v_gameplayfix_hemebond_pushmove 0" it is just me standing still on the ground next to a platform moving up.

(The entirety of the change is removing the continue statement? Is that correct?

I did this ...

            if (!trace.startsolid) {
                // Con_PrintLinef ("- not in solid\");
                if (sv_gameplayfix_hemebond_pushmove.value) {
                    // continue - don't continue
                } else {
                    // DarkPlaces
                    continue;
                }
            }
hemebond commented 7 months ago

(The entirety of the change is removing the continue statement? Is that correct?

That's the change that results in doors and horizontal movers pulling you along, so I don't think it's a good solution.

This is the workaround I think I would prefer, where the entity is not squashed at all. There was only the minor issue with the silver key in e3m2 and some cosmetic corpses under platforms being teleported onto it.

@@ -1934,8 +1934,8 @@ static void SV_PushMove (prvm_edict_t *pusher, float movetime)
                        if (PRVM_serveredictfloat(check, solid) == SOLID_NOT || PRVM_serveredictfloat(check, solid) == SOLID_TRIGGER)
                        {
                                // corpse
-                               PRVM_serveredictvector(check, mins)[0] = PRVM_serveredictvector(check, mins)[1] = 0;
-                               VectorCopy (PRVM_serveredictvector(check, mins), PRVM_serveredictvector(check, maxs));
+                               // PRVM_serveredictvector(check, mins)[0] = PRVM_serveredictvector(check, mins)[1] = 0;
+                               // VectorCopy (PRVM_serveredictvector(check, mins), PRVM_serveredictvector(check, maxs));
                                continue;
                        }
hemebond commented 7 months ago

Raised a PR for it. Devs can decide if it's worth using.

bones-was-here commented 6 months ago

@bones-was-here Which solution would you prefer? The squashed bbox like other engines, or non-squashed box (my preference since it makes it still behave well in my limited testing).

By default we should squash because Quake did, but we can change the default for mods that work better without it.

hemebond commented 6 months ago

@bones-was-here Which solution would you prefer? The squashed bbox like other engines, or non-squashed box (my preference since it makes it still behave well in my limited testing).

By default we should squash because Quake did, but we can change the default for mods that work better without it.

Unfortunately I couldn't get it to squash like other engines. If I removed the if (!trace.startsolid) check then the player can get dragged by movers like doors.

bones-was-here commented 6 months ago

Quake has that check too, and the current squashing looks the same as Quake to me.

hemebond commented 6 months ago

Quake has that check too, and the current squashing looks the same as Quake to me.

For some reason squashed entities in DarkPlaces, unlike other engines, stop interacting with the world. I couldn't figure out how to fix that.

bones-was-here commented 6 months ago

If other engines allow it to be pushed (or lifted specifically?) even when it has a squashed bbox then we need to implement that behaviour (even if we also want to optionally disable squashing).

I'm also confused as to why the bounding box is so damn big; it should only be 32x32x56

probably this cvar_t sv_legacy_bbox_expand = {CF_SERVER, "sv_legacy_bbox_expand", "1", "before linking an entity to the area grid, decrease its mins and increase its maxs by '1 1 1', or '15 15 1' if it has flag FL_ITEM (this is the Quake/QuakeWorld behaviour); disable to make SVQC bboxes consistent with CSQC which never does this expansion"};

hemebond commented 6 months ago

I had another go at trying to find what was preventing it just the code in that function is all undocumented macros and I have no idea what is going on.

MarioSMB commented 2 weeks ago

Should this really be considered resolved by the inclusion of a nosquash workaround? I do believe bones brought up a valid concern regarding the behaviour of DarkPlaces compared to other engines.

divVerent commented 2 weeks ago

I do think it makes sense to be able to turn off the squashing, as it's an odd behavior. But of course the squashing can also be "fixed" by other means (e.g. QC can check if it's safe to re-expand the bounding box every frame, and do so once possible).

So yeah, let's reopen this - this issue is not fixed yet anyway, as we still need to set the cvar, even if nothing else.

Baker7 commented 2 weeks ago

hemebond, does your new cvar and changes --- do they fix Zerstorer playability?

hemebond commented 2 weeks ago

hemebond, does your new cvar and changes --- do they fix Zerstorer playability?

It does, yes.

image

Baker7 commented 2 weeks ago

hemebond, does your new cvar and changes --- do they fix Zerstorer playability?

It does, yes.

image

Interesting.

Baker7 commented 2 weeks ago

hemebond, does your new cvar and changes --- do they fix Zerstorer playability?

It does, yes.

image

Hemebond, two quick questions ...

1) Anyone figure out why DarkPlaces had this "squashing"?

2) Are there any known examples of "bad" behaviors or broken things that happen when your "no squashing" is activated? Like are Quake 1.5 or Xonotic or another DarkPlaces specific work negatively impacted?

Sounds like you've done a great job solving this one!

MarioSMB commented 2 weeks ago

Can't comment on the possible breakage as I will have to test with this new option, but I can note that squashing of items has caused issues in Xonotic maps and that its QuakeC has a few checks in place to resize items that get squashed.

For example the CTF flag code has this:

    // sanity checks
    if(this.mins != this.m_mins || this.maxs != this.m_maxs) { // reset the flag boundaries in case it got squished
        tracebox(this.origin, this.m_mins, this.m_maxs, this.origin, MOVE_NOMONSTERS, this);
        if(!trace_startsolid || this.noalign) // can we resize it without getting stuck?
            setsize(this, this.m_mins, this.m_maxs);
    }

So Xonotic, unless cases are found that cause issues, will likely prefer to not squash items.

As for why DarkPlaces has it, this is probably actually a "bug" from the original Quake engine: https://github.com/id-Software/Quake/blob/master/WinQuake/sv_phys.c#L523
Note that it also states this above: corpses are SOLID_NOT and MOVETYPE_TOSS, so I don't know why they included SOLID_TRIGGER (generally items) in the check.

Baker7 commented 2 weeks ago

Thanks for info Mario.

hemebond commented 2 weeks ago

Hemebond, two quick questions ...

  1. Anyone figure out why DarkPlaces had this "squashing"?

Quake squashes the bbox of entities that get crushed. In DarkPlaces it then becomes non-solid, while in other Quake engines it still interacts with brush entities.

  1. Are there any known examples of "bad" behaviors or broken things that happen when your "no squashing" is activated? Like are Quake 1.5 or Xonotic or another DarkPlaces specific work negatively impacted?
  1. If you walk into a moving brush you will be dragged along with it, e.g., a door opening will pull you sideways.
  2. Corpses under platforms will pop up onto the platform or get stuck in them. image

Sounds like you've done a great job solving this one!

Just commenting out stuff until it didn't break :-P