Conversation
39481ca to
cde0914
Compare
Shows a quake live style damage plum on damage when set to 1.
cde0914 to
06f671d
Compare
There was a problem hiding this comment.
I'm a little concerned with performance, but otherwise looks sensible in my amateur eyes.
But the performance, besides RAM usage, is not affected when the option is disabled, and is disabled by default, so it's not a big blocker?
I didn't test this yet though.
| plum = G_TempEntity( origin, EV_DAMAGEPLUM ); | ||
| // only send this temp entity to the attacker | ||
| plum->r.svFlags |= SVF_SINGLECLIENT; | ||
| plum->r.singleClient = attacker->s.number; | ||
| // | ||
| plum->s.otherEntityNum = attacker->s.number; | ||
| plum->s.time = damage; |
There was a problem hiding this comment.
I see that you followed the example of cg_scorePlum, but I wonder if we really need a server event for this? Can't the client do this not far from where the "hit" sound is played, i.e. CG_TransitionPlayerState?
baseq3a/code/cgame/cg_playerstate.c
Lines 483 to 527 in aa8dac0
This would also allow to use the option even if the server doesn't support it.
Maybe I'm micro-optimizing here, but I think that we should expect the "damage" events to take up a significant portion of the traffic?
There was a problem hiding this comment.
They don’t seem to be appreciably noisy in my testing, but that’s helped by aggregating damage during server frames and sending just one.
My initial try was to do exactly what you said re: estimating the damage client side. I ran into issues with things like splash damage, though, because the client doesn’t know the split of damage per player hit.
code/game/g_combat.c
Outdated
| trap_GetUserinfo( attacker->s.number, userinfo, sizeof( userinfo ) ); | ||
| damagePlumsEnabled = atoi( Info_ValueForKey( userinfo, "cg_damagePlums" ) ); |
There was a problem hiding this comment.
Is this not expensive? Maybe this should be stored on a new property on clientPersistant_t, and updated like the rest of userinfo, in ClientUserinfoChanged?
Lines 656 to 657 in aef7f82
There was a problem hiding this comment.
I'm flexible on approach here if we decide we'd like to implement the feature. I did the work for another repo and wanted to see if there was interest here.
|
Just wondering: could enabling this be considered an unfair advantage over who plays with default settings? Just wondering whether it might be the case to allow server admins to disallow its usage (or even to force it to everyone?), via g_ cvar or dmflag? |
|
@WofWca took your suggestion. wdyt? As for the idea of disallowing it, I'm not sure the damage numbers are likely to be a competitive advantage much beyond the damage sounds, which aren't server-toggled? |
44edfa9 to
695d925
Compare
There was a problem hiding this comment.
To my untrained eye of a stranger on the internet, looks good!
I have tested the feature for a while. Some feedback:
- In Quake Live, the size of the numbers is always the same regardless of distance.
I think this is nice. Can we achieve the same?
Does the game even support this? - At close distance it's hard to see the number.
I think the origin should be moved a little down,
closer to the center of the player.
Or maybe also the "raising" of the number should be tuned down? - Maybe the font needs to be made smaller?
- The aggregation seems like a bad thing sometimes?
e.g. when shooting with the lightning gun, you sometimes get a plum of 16
instead of 2 plums of 8.
This throws you off a little.
But overall I think this is about good to merge.
Just take a look at whether you'd want to address my smaller comments.
The rest can be addressed in follow-up MRs.
Edit: for those lazy, here is the build of the mod with this MR's changes (just rename to pak8a.pk3):
pak8a.pk3
damage-plums-1.mp4
damage-plums-2.mp4
code/cgame/cg_localents.c
Outdated
| len = VectorLength( delta ); | ||
| if ( len < 20 ) { |
There was a problem hiding this comment.
| len = VectorLength( delta ); | |
| if ( len < 20 ) { | |
| len = VectorLengthSquared( delta ); | |
| if ( len < 20*20 ) { |
VectorLengthSquared is cheaper, is there a reason you changed this?
There was a problem hiding this comment.
Don’t recall now. Will switch it back though.
code/cgame/cg_localents.c
Outdated
| if (damage >= 50) { | ||
| re->shaderRGBA[0] = 0xff; | ||
| re->shaderRGBA[1] = 0x00; | ||
| re->shaderRGBA[2] = 0x00; | ||
| } else if (damage >= 25) { | ||
| re->shaderRGBA[0] = 0xff; | ||
| re->shaderRGBA[1] = 0x80; | ||
| re->shaderRGBA[2] = 0x00; | ||
| } else if (damage >= 10) { | ||
| re->shaderRGBA[0] = 0xff; | ||
| re->shaderRGBA[1] = 0xff; | ||
| re->shaderRGBA[2] = 0x00; | ||
| } else { | ||
| re->shaderRGBA[0] = 0xff; | ||
| re->shaderRGBA[1] = 0xff; | ||
| re->shaderRGBA[2] = 0xff; | ||
| } |
There was a problem hiding this comment.
Are the thresholds based on anything? They seem to be different in Quake Live.
I don't think that we should differentiate the damage numbers much as long as it's below 30, or at least 20? Cause plasmagun is 20.
Either way, should probably just copy things from Quake Live.
There was a problem hiding this comment.
Do you have the actual Quake Live thresholds? I played QL very little. Went with what felt right because I couldn’t find official numbers and because Q3 weapon damage is tuned differently anyway.
There was a problem hiding this comment.
Well, TBH they have two modes of cg_damagePlumsColorStyle: "Damage Colors" and "One Color Fade". With "one color fade" it's just between white and red between 0 and 100. But for "Damage Colors" I just tested it (with handicap and direct rocket launcher hits):
- <= 25: blue (could be white in our case)
- <= 50 yellow
- <= 75 orange
- > 75 red
Q3 weapon damage is tuned differently anyway
true, but the difference is not that big I'd say.
There was a problem hiding this comment.
I’m open to adjustments. I wanted to see a difference between things like bullets and plasma and the 25 threshold makes most damage blue by this calculation. Maybe that’s fine, since splash damage is the big variable anyway?
There was a problem hiding this comment.
Maybe that’s fine, since splash damage is the big variable anyway?
I agree. Also shotgun
I think that we don't need to differentiate between plasma, lightning gun, and bullets, because it's anyways obvious what the damage is.
There was a problem hiding this comment.
So, I personally would take Quake Live's scale. It's also intuitive that it's linear.
But I won't insist.
There was a problem hiding this comment.
Another thought: with the current implementation, there is basically no differentiation between damage of 50 and 100. Which is a big difference when using rocket launcher and shotgun. You'd want to know if a 110 HP enemy has 20 HP left, or is it 60.
There was a problem hiding this comment.
Aaaand another point: to be consistent with hit sounds:
baseq3a/code/cgame/cg_playerstate.c
Lines 298 to 302 in aa8dac0
| CG_AddDamagePlum | ||
| =================== | ||
| */ | ||
| void CG_AddDamagePlum( localEntity_t *le ) { |
There was a problem hiding this comment.
There really is a lot of common code between CG_AddScorePlum and this. Maybe some could be factored out?
Although maybe not, the original code is also full of copy-paste.
There was a problem hiding this comment.
That was my thinking. It’s weird but also follows the style of the original
It doesn’t support it directly, but we can calculate a consistent angular width based on distance to set the size of the sprite. I do the same for the crosshair sprite in Q3VR/ioq3quest.
Can look into this. More likely move origin down a bit. The arc felt pretty close to QL in my initial testing.
Can adjust if standardizing size. Size was chosen for distance visibility before. Might need to make it configurable via cvar, as I mainly care about visibility in VR and only submitted here because I want the server-side support more consistently. 😉
that does sound like it could throw you off. I’ve never seen it. It works based on server frames so I imagine a lower setting on the server could also make this more likely. |
OK, then maybe that could be for another MR.
see my second video, it happens there once, right at the beginning. IIRC it was at |
Yep, saw it on your video. Meant I never saw it in my own gameplay. But I run servers at 40 instead of 20 and I suspect that is why. |
Also, adjust to match colors and set a consistent size. Spawn entity at about half height. Overall, should look closer to Quake Live appearance.
|
@WofWca Incorporated feedback. Looks a good bit better to me. |
|
Yes, I think this looks better! For comparison, Quake Live (don't look at the bottom right corner though). Indeed it looks like that the font family is the reason that the numbers are more readable in Quake Live. And here is a build: pak8a.pk3. |
Remove unused variables. Co-authored-by: WofWca <wofwca@protonmail.com>
|
Another thing I noticed about this: old game can't play back demos recorded on the new game (this MR). It errors out with "unknown event" as soon as there is a damage plum in the demo playback. IDK if this is important. |
|
Basically, you introduced new net-code changes that are breaking by adding new events or changes to anything playerstate/entitystate. |
|
Yeah, you can't really get backward-compatible client playback from an event addition, but playback of old demos will work fine. Could conceivably prevent recording of the damage plum event, I suppose, but that would happen in the engine or by triggering plums via server commands vs events. This doesn't seem worth the squeeze. The server won't send the events if the player doesn't enable them, so the player still has control over recording demos compatible with old clients if this matters to them. |
| DECLARE_EVENT( EV_TAUNT_PATROL ) | ||
| DECLARE_EVENT( EV_TAUNT_PATROL ), | ||
|
|
||
| DECLARE_EVENT( EV_DAMAGEPLUM ) // damage plum |
There was a problem hiding this comment.
Hey, I've come up with a way to keep network compatibility!
This will display damage plums as score plums in older clients.
0001-fix-don-t-introduce-new-event-reuse-EV_SCOREPLUM.patch
patch text
From 0cefcfcdc83db1ad2f9c263362ca704dca69cd40 Mon Sep 17 00:00:00 2001
From: WofWca <wofwca@protonmail.com>
Date: Sun, 1 Feb 2026 18:52:10 +0400
Subject: [PATCH] fix: don't introduce new event, reuse EV_SCOREPLUM
This fixes inability to play back demos that were recorded
in the new version on a client that doesn't know
about the `EV_DAMAGEPLUM` event. See
https://github.com/ec-/baseq3a/pull/56#issuecomment-3772877175.
This will display damage plums as score plums in older clients.
---
code/cgame/cg_event.c | 10 +++++-----
code/game/bg_events.h | 4 +---
code/game/bg_public.h | 1 +
code/game/g_combat.c | 3 ++-
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/code/cgame/cg_event.c b/code/cgame/cg_event.c
index 15390dcb..fe33a047 100644
--- a/code/cgame/cg_event.c
+++ b/code/cgame/cg_event.c
@@ -960,11 +960,11 @@ void CG_EntityEvent( centity_t *cent, vec3_t position, int entityNum ) {
#endif
case EV_SCOREPLUM:
- CG_ScorePlum( cent->currentState.otherEntityNum, cent->lerpOrigin, cent->currentState.time );
- break;
-
- case EV_DAMAGEPLUM:
- CG_DamagePlum( cent->lerpOrigin, cent->currentState.time );
+ if (cent->currentState.eventParm & SCOREPLUM_IS_DAMAGEPLUM) {
+ CG_DamagePlum( cent->lerpOrigin, cent->currentState.time );
+ } else {
+ CG_ScorePlum( cent->currentState.otherEntityNum, cent->lerpOrigin, cent->currentState.time );
+ }
break;
//
diff --git a/code/game/bg_events.h b/code/game/bg_events.h
index 27d74e09..b9e0b2ce 100644
--- a/code/game/bg_events.h
+++ b/code/game/bg_events.h
@@ -108,9 +108,7 @@ DECLARE_EVENT( EV_TAUNT_NO ),
DECLARE_EVENT( EV_TAUNT_FOLLOWME ),
DECLARE_EVENT( EV_TAUNT_GETFLAG ),
DECLARE_EVENT( EV_TAUNT_GUARDBASE ),
-DECLARE_EVENT( EV_TAUNT_PATROL ),
-
-DECLARE_EVENT( EV_DAMAGEPLUM ) // damage plum
+DECLARE_EVENT( EV_TAUNT_PATROL )
#ifdef EVENT_ENUMS
, DECLARE_EVENT( EV_MAX )
diff --git a/code/game/bg_public.h b/code/game/bg_public.h
index c5374159..af02efe6 100644
--- a/code/game/bg_public.h
+++ b/code/game/bg_public.h
@@ -355,6 +355,7 @@ typedef enum {
#undef EVENT_ENUMS
} entity_event_t;
+#define SCOREPLUM_IS_DAMAGEPLUM 0x01
typedef enum {
GTS_RED_CAPTURE,
diff --git a/code/game/g_combat.c b/code/game/g_combat.c
index 4639789a..9156bc84 100644
--- a/code/game/g_combat.c
+++ b/code/game/g_combat.c
@@ -36,13 +36,14 @@ void DamagePlum( gentity_t *attacker, vec3_t origin, int damage ) {
return;
}
- plum = G_TempEntity( origin, EV_DAMAGEPLUM );
+ plum = G_TempEntity( origin, EV_SCOREPLUM );
// only send this temp entity to the attacker
plum->r.svFlags |= SVF_SINGLECLIENT;
plum->r.singleClient = attacker->s.number;
//
plum->s.otherEntityNum = attacker->s.number;
plum->s.time = damage;
+ plum->s.eventParm = SCOREPLUM_IS_DAMAGEPLUM;
}
/*
--
2.41.0
Maybe one should consider hiding them completely, e.g. by providing an out-of-range client so that the if (client != cg.predictedPlayerState.clientNum) return check is triggered, but I think it's OK to show them together with regular score plums.
There was a problem hiding this comment.
Solid idea. I've stopped pushing over here but implemented in https://github.com/ernie/trinity (which has adopted the quake live style font, as well)




Shows a quake live style damage plum on damage when set to 1.