Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added cl_show_server_triggers_* cvars (related to plugin PR) #183

Merged
merged 17 commits into from
Dec 31, 2023

Conversation

SmileyAG
Copy link
Contributor

@SmileyAG SmileyAG commented Dec 6, 2023

Explained in YaLTeR/hlkreedz#37

I recently remembered and rebased my old request for plugin a few days ago and at the same time reminded about it to naz in discord, he said that would approve later, so I’m sending a pull request in advance, it’s not necessary to merge it now (since the PR plugin isn't merged yet), but you shouldn’t close it either

@tmp64
Copy link
Contributor

tmp64 commented Dec 8, 2023

Maybe check for gEngfuncs.GetMaxClients() > 1 when hiding singleplayer triggers?

@SmileyAG
Copy link
Contributor Author

SmileyAG commented Dec 8, 2023

Maybe check for gEngfuncs.GetMaxClients() > 1 when hiding singleplayer triggers?

Well, I'd say it might require a separate cvar at that point then, since even when I'm playing at own listenserver with maxplayers 1 and loaded amxmodx+metamod-p+hlkreedz combo, still don't want to see those SP triggers because they wouldn't action normally in deathmatch mode, example:

OpenAG/dlls/triggers.cpp

Lines 1466 to 1475 in 27e7143

void CChangeLevel :: ChangeLevelNow( CBaseEntity *pActivator )
{
edict_t *pentLandmark;
LEVELLIST levels[16];
ASSERT(!FStrEq(m_szMapName, ""));
// Don't work in deathmatch
if ( g_pGameRules->IsDeathmatch() )
return;

@SmileyAG
Copy link
Contributor Author

SmileyAG commented Dec 14, 2023

Please review this code again, because now I have fully changed it to display triggers with TriAPI

Let me first explain why this was done, because if being unenlightened in that question, then you would have a fair reason to ask: why the hell was it needed to go this way?

In short, modern map compilers cut off unused planes due to optimization (such as from triggers), but trigger entities from plugin is still transmitted from the server to the client, which means we can try to take their mins, maxs, origin and draw them separately (e.g. with TriAPI)

Showcase (value 1 = new triggers, value 2 = old triggers):

new_triggers.mp4

@tmp64
Copy link
Contributor

tmp64 commented Dec 14, 2023

What about trigger brushes of forms other than a box? They would be rendered incorrectly

cl_dll/tri.cpp Outdated Show resolved Hide resolved
cl_dll/tri.cpp Outdated Show resolved Hide resolved
cl_dll/tri.cpp Outdated Show resolved Hide resolved
@SmileyAG
Copy link
Contributor Author

What about trigger brushes of forms other than a box? They would be rendered incorrectly

Probably, but have you ever seen a map in which the trigger is not a box? I've looked before through a lot of maps in the bspguy or directly in map sources with J.A.C.K, and I've never seen a brush trigger that wasn't in a box shape

@tmp64
Copy link
Contributor

tmp64 commented Dec 14, 2023

Probably, but have you ever seen a map in which the trigger is not a box? I've looked before through a lot of maps in the bspguy or directly in map sources with J.A.C.K, and I've never seen a brush trigger that wasn't in a box shape

You can try rendering the 0-th hull of the brush entity model. It should be the exact one used for collision detection with point entities.
https://github.com/dreamstalker/rehlds/blob/93f5775ac26240782981f47ee8e052fb53d30877/rehlds/engine/world.cpp#L193

@YaLTeR
Copy link
Owner

YaLTeR commented Dec 14, 2023

Note that I'm not sure if we can use GPL code in OpenAG.

@YaLTeR
Copy link
Owner

YaLTeR commented Dec 14, 2023

I think rendering boxes only is fine if the alternative complicates things a lot.

@tmp64
Copy link
Contributor

tmp64 commented Dec 14, 2023

Note that I'm not sure if we can use GPL code in OpenAG.

It's a reference to collision detection code and it's not intended to be added to OpenAG

cl_dll/cl_util.h Outdated Show resolved Hide resolved
@@ -50,6 +59,12 @@ int CL_DLLEXPORT HUD_AddEntity( int type, struct cl_entity_s *ent, const char *m
break;
}

// show triggers that would be transferred from server-side with specific value in renderfx to differ it from other entities
// update: there is a new implementation of displaying triggers that allows you to display even when planes is stripped due to optimizations in updated map compiler
// so this code will only work if the value 2 is specified in the cvar, but it should not be deleted imo
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but it should not be deleted imo

Why not? Is it preserved server-side too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eeeh, well, absmin/absmax triggers seems to be draw as should for the most of maps, but I had noticed that on some specific map (dyd_axn_plant) in a specific place it draw not the right size as should, so I'd keep that second mode for a reserve case, like if the main mode is would having some issues

Correct draw:

изображение

Wrong draw:

изображение

cl_dll/hud.cpp Show resolved Hide resolved
cl_dll/hudgl.cpp Show resolved Hide resolved
cl_dll/tri.cpp Show resolved Hide resolved
cl_dll/tri.cpp Outdated Show resolved Hide resolved
@@ -16,7 +16,7 @@
#define STUDIO_EVENTS 2

#define MAX_CLIENTS 32
#define MAX_EDICTS 900
#define MAX_EDICTS 2048
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this related?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course! That depending through how many entities we want to iterate to find triggers!

if ((gHUD.m_pShowServerTriggers->value > 0) && (gHUD.m_pShowServerTriggers->value != 2.0f))
	{
		for (int e = 0; e < MAX_EDICTS; ++e)
		{
			cl_entity_t* ent = gEngfuncs.GetEntityByIndex(e);
			if (ent)
			{

According to the investigate from Solokiller, max edicts can be no more than 2047, anything higher than this will have cause crash: ValveSoftware/halflife#1777

Copy link
Contributor Author

@SmileyAG SmileyAG Dec 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also started to think might be it should controllable instead? Like if the map have a not much entities, then you don't want to do extra iterations and it will boost some performance, right? Since that code would run the each frame for updating triggers

Making some cvar like cl_show_server_triggers_entities_limit 2048

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CVar is definitely not the right solution here

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just leave it as is for now, since it doesn't run with showtriggers 0

…function

That done for versatility, since there is not much cases to check validity for
@SmileyAG
Copy link
Contributor Author

SmileyAG commented Dec 30, 2023

@YaLTeR @tmp64 Well I haven't contacted with naz for recent time, but at least exec approved that and updated plugin for all SourceRuns HLKZ servers, so pull requests in those both clients (OpenAG and BugfixedHL-Rebased) can be merged now:

изображение

cl_dll/tri.cpp Outdated Show resolved Hide resolved
cl_dll/entity.cpp Outdated Show resolved Hide resolved
@YaLTeR YaLTeR merged commit 7c35c77 into YaLTeR:master Dec 31, 2023
13 checks passed
@YaLTeR
Copy link
Owner

YaLTeR commented Dec 31, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants