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

Changed visual properties of triggers to make them be shown on the clientside #37

Open
wants to merge 6 commits into
base: unstable
Choose a base branch
from

Conversation

SmileyAG
Copy link

@SmileyAG SmileyAG commented Jul 10, 2023

hl1-triggers

Image explanation

trigger_autosave -> trigger_cdaudio -> trigger_changelevel -> trigger_endsection -> trigger_gravity -> trigger_hurt -> trigger_monsterjump -> trigger_multiple -> trigger_once -> trigger_push -> trigger_teleport -> trigger_transition -> func_ladder -> others

How to draw triggers on client-side

  • Find HUD_AddEntity function (entity.cpp file)
  • Do the next check: if (ent->curstate.renderfx == 241)
  • Add the checks for specific ent->curstate.rendercolor to avoid of draw triggers that related to single-player (_changelevel, _transition, _monsterjump, _endsection, _autosave)
  • Set ent->curstate.renderamt depends of the cvar value (default value should be 120 imo)

cc @fireblizzard

UPD: as soon as this request is accepted, someone needs to make the pull requests up to OpenAG and BugfixedHL-Rebased repos

@fireblizzard
Copy link
Collaborator

Can you check if we can use something like iuser4 instead of checking for the renderfx value of 241?

IIRC iuser3/iuser4 should be good, as in they shouldn't be currently in use for anything, at least in the case of these specific entities. I think for the player entity or something it's already being used in AG for some other purpose, but for these type of entities I think we can use them.

In the client ent->curstate.iuser4 I think it is (https://github.com/YaLTeR/OpenAG/blob/78fac3a9d0d8c033166f13d099d50ca1410e89e4/common/entity_state.h#L114), and in the plugin I think we can set it with the set_pev function and the field pev_iuser4.

@SmileyAG
Copy link
Author

SmileyAG commented Dec 5, 2023

Rebased to latest unstable branch now

After a long time, I decided to remember this request and want to note that renderfx could be changed afterwards on the client side (HUD_AddEntity), so I don't see wrong in use the random value for it (since the values that doesn't determined in CL_FxBlend would be applied for default case, essentially the same as using kRenderFxNone)

Only the first 21 values from renderfx can have any special effect (usually it will be renderamt + kind of offset depending on the type of effect), most of this is set in the CL_FxBlend function from the engine

CL_FxBlend is called after HUD_AddEntity (ClientDLL_AddEntity in the engine), i.e. this means that if you change renderfx on the client, the effect will definitely be different afterwards

And besides, I think iuser3/iuser4 might be utilized by some other plugins (even if most likely this will not be the case for triggers, but still)

…lues here

(since that are constants that are used now for detecting on client-side)
@fireblizzard
Copy link
Collaborator

Thanks man, will try to test tomorrow, but i don't know if i will be doing those PRs myself

@SmileyAG
Copy link
Author

SmileyAG commented Dec 13, 2023

Thanks man, will try to test tomorrow, but i don't know if i will be doing those PRs myself

You mean the PRs for the different clients as OpenAG and BugfixedHL-Rebased to support it? If so, you can leave that task to me, I'm already done the request for OpenAG repo (they are not yet merged, since it waits for your approval to be it merged and used on HLKZ servers)

@fireblizzard
Copy link
Collaborator

I tested and i'm not able to see all of the triggers that are supposed to become visible. Looks like a similar issue to the /winvis one, where sometimes it just won't hide a water body even if it's a func_water, but you load the map some other time and it hides that very entity correctly. We had this issue a lot on 8b1_hellinashop

You can test on the French server (# 9) on AG. When i load dyd_axn_plant i cannot see the trigger_push at the start, but i'm able to see the trigger_multiple entities at the healthbooster. Then i load hl1_bhop_uc1_beta5 and i'm not able to see the trigger_multiple requirements (crowbar and glock pickup volumes/cuboids), nor the start/end trigger_multiples

Do you have any idea of what may be going on? i put a console line both in the plugin where you set the color for the trigger_push and in the client where you set the renderamt with the alpha cvar, and it looks like that specific trigger_push that i can't see in dyd_axn_plant goes correctly through both code flows and i see the lines in the console

@SmileyAG
Copy link
Author

SmileyAG commented Dec 14, 2023

Do you have any idea of what may be going on?

That because modern map compiles is stripping unused planes, so despite that triggers is still transmitted from server to client, in fact they aren't draw
But if you read this carefully, you notice that they are still transmitted to the client, and therefore we still have access to mins, maxs and origin

So, today I implemented TriAPI drawing of triggers as cuboid based on their absmin/absmax on client-side, so, that issue is solved now
You can download the latest version from GitHub Actions of linked pull request in the OpenAG repository to test it

Showcase: YaLTeR/OpenAG#183 (comment)

Also, I decided right now to join on # 9 .fr server as you asked and for some reason only (func_ladder) are displayed, but no triggers (any map), so I’ll just ask: you didn’t break anything in the local version of plugin in an attempt to fix issue?

@fireblizzard
Copy link
Collaborator

That's awesome. Now i have the same problem as you, i can't see the triggers in the server, neither with the latest version nor with the old one, and i swear i could see the ones at the healthbooster in dyd_axn_plant yesterday, but now i'm only able to see them locally. The plugin had no changes, it was a clean pull + compilation. I did it again just to be sure and restarted the server, loaded the maps several times and still cant see them :/

The server has the kz_show_triggers_disable cvar untouched, so it's set to 0. No errors in the log. Some more info:

meta version
Metamod-r v1.3.0.128, API (5:13)
Metamod-r build: 17:47:54 Aug 24 2018
Metamod-r from: https://github.com/theAsmodai/metamod-r/commit/0cf2f70
amxx version
AMX Mod X 1.9.0.5263 (http://www.amxmodx.org)
Authors:
        David "BAILOPAN" Anderson, Pavol "PM OnoTo" Marko
        Felix "SniperBeamer" Geyer, Jonny "Got His Gun" Bergstrom
        Lukasz "SidLuke" Wlasinski, Christian "Basic-Master" Hammacher
        Borja "faluco" Ferrer, Scott "DS" Ehlert
Compiled: Oct 27 2019 16:23:31
Built from: https://github.com/alliedmodders/amxmodx/commit/15a14a0
Build ID: 5263:15a14a0
Core mode: JIT+ASM32
amxx plugins
Currently loaded plugins:
       name                    version     author            file             status
 [  1] Admin Commands          1.9.0.5263  AMXX Dev Team     admincmd.amxx    running
 [  2] Slots Reservation       1.9.0.5263  AMXX Dev Team     adminslots.amxx  running
 [  3] Pause Plugins           1.9.0.5263  AMXX Dev Team     pausecfg.amxx    running
 [  4] Stats Configuration     1.9.0.5263  AMXX Dev Team     statscfg.amxx    running
 [  5] [AMXX] Settings API     1.0         MeRcyLeZZ         amx_settings_ap  debug
 [  6] HL KreedZ Beta          0.51        KORD_12.7, Lev,   hl_kreedz.amxx   debug
 [  7] HLKZ Discord WR Notifi  1.1.1       Th3-822 & naz     hl_kreedz_disco  debug
 [  8] MultiPlayer Bhop        1.2.1-SR    ConnorMcLeod      mpbhop.amxx      running
 [  9] Check fps               1.0         connor            fpslimiter.amxx  running
 [ 10] AFK Kicker              1.1         Cheesy Peteza     afkkicker.amxx   running
 [ 11] Climb Button Maker      0.8         Kr1Zo & naz       cbm.amxx         running
 [ 12] ProKreedz Hook          v2.3        vato loco [GE-S]  hook.amxx        running
 [ 13] Enhanced Map Searching  1.7.1       EJL/JTP10181 & n  searchmaps.amxx  running
 [ 14] AMXBans Core            6.13        YamiKaitou        amxbans_core.am  running
 [ 15] AMXBans Main            6.13        YamiKaitou        amxbans_main.am  running
 [ 16] HLKZ Competitions       0.1.0       naz               hl_kreedz_compe  debug
16 plugins, 16 running

@SmileyAG
Copy link
Author

SmileyAG commented Dec 18, 2023

Hello, I have found time to test and I can definitely say that this is a issue either on the plugin or server-side, but definitely not on the client-side

But here's what's interesting: the plugin still passes rendercolor and renderfx to func_ladder, otherwise it simply wouldn't to be have a blue color
The reason why func_ladder still will be transmitted to client is because in the original server code, during ladder entity initialization, the EF_NODRAW flag is removed: https://github.com/fireblizzard/agmod/blob/bf06e4ffd31c1427784685118820e15552803bcb/dlls/triggers.cpp#L1765
For all other triggers the EF_NODRAW flag is set by default (only if the showtriggers cvar is not enabled, which is not even registered): https://github.com/fireblizzard/agmod/blob/bf06e4ffd31c1427784685118820e15552803bcb/dlls/triggers.cpp#L538
To put it simply, this lets you understand that the original changes only does at trigger initialization, so it will be surely overwritten with our changes in AddToFullPack

I want to ask you one more question: you didn't replace set_pev(ent, pev_effects, pev(ent, pev_effects) & ~EF_NODRAW); with set_es(es, ES_Effects, get_es(es, ES_Effects) & ~EF_NODRAW); in local version of plugin, right?
If that so, then that is the answer to issue, you need to use set_pev to remove EF_NODRAW instead

If it's not, then it’s difficult for me to imagine what it cause that issue then, if that wouldn't resolve then I could make a request to the AG 6.7 repository with identical changes instead (but this time it would be done in the trigger init, and not at AddToFullPack call as in the plugin)

Hello, I have found time to test and I can definitely say that this is a issue either on the plugin or server-side, but definitely not on the client-side

You can even test this yourself by checking if brush entity with specific model name is passed to client in few steps:

  • Open .bsp with newbspguy tool or in Notepad and find what is specified in model keyvalue of needed brush entity (e.g. "model" "*45")
  • Create a test cvar, say debug_test
  • Inside the HUD_AddEntity function, insert the following code:
#include "com_model.h" // Because it is not included in the file, otherwise the compiler will complain for ent->model->name

if (ent && ent->model)
{
	const char *model_debug = "*45";
	if (gHUD.debug_test->value > 0 (!strcmp(ent->model->name, model_debug)))
		gEngfuncs.Con_Printf("entity with %s model passed to client\n", model_debug);
}
  • Now you need walk to that entity so it will be in the player PVS zone, then you type debug_test 1 and check the console

Also, I fixed the crash in code of triggers request for OpenAG, so update the DLL again by downloading it from GitHub Actions

@fireblizzard
Copy link
Collaborator

The plugin in the server is just a git pull + local compile of this branch. No code changes compared to this branch. Now i have put a print of the classname inside the if (!get_pcvar_num(pcvar_kz_show_triggers_disable) in Fw_FmAddToFullPackPost and it's only printing the func_ladder entities. Testing on dyd_axn_plant that has a trigger_push and at the beginning and more triggers at the healthbooster. On a listenserver it prints all of the triggers we want.

I don't have the time to properly trace the path of these entities from the server all the way to the client, i'm not sure why they're not received in Fw_FmAddToFullPackPost. But i have tried a couple things quickly and the following seems to work for both the listenserver and the remote server:

//// New global ////
new g_RenderizableTrigger[MAX_ENTITIES];  // triggers that we may allow the client to see

//// After creating the cvar in plugin_init, to subscribe to changes on the cvar ////
hook_cvar_change(pcvar_kz_show_triggers_disable, "ShowTriggersChange");

//// Around the end of plugin_cfg ////
AdjustTriggersVisibility();

//// New function to be called whenever kz_show_triggers_disable changes ////
public ShowTriggersChange(pcvar, const old_value[], const new_value[])
{
	AdjustTriggersVisibility();
}

//// New function to go through all the entities and cache the rendering status... ////
AdjustTriggersVisibility()
{
	new ent, entCount;

	new disableTriggers = get_pcvar_num(pcvar_kz_show_triggers_disable);

	for (ent = g_MaxPlayers + 1; ent < global_get(glb_maxEntities); ent++)
	{
		if (!pev_valid(ent))
			continue;

		new className[32];
		pev(ent, pev_classname, className, charsmax(className));
	
		if (equali(className, "trigger_", 8) || equali(className, "func_ladder"))
		{
			if (!disableTriggers)
			{
				g_RenderizableTrigger[ent] = true;
				set_pev(ent, pev_effects, pev(ent, pev_effects) & ~EF_NODRAW);
				entCount++;
			}
			else
			{
				g_RenderizableTrigger[ent] = false;
				// This is left here commented out as a reminder that this does not work as intended
				//set_pev(ent, pev_effects, pev(ent, pev_effects) | EF_NODRAW);
			}
		}
		else
			g_RenderizableTrigger[ent] = false;
	}

	server_print("[%s] The current map has %d triggers that can now be turned visible", PLUGIN_TAG, entCount);
}

//// Part of Fw_FmAddToFullPackPost that i changed ////
public Fw_FmAddToFullPackPost(es, e, ent, host, hostflags, player, pSet)
{
	if (!IsConnected(host))
		return FMRES_IGNORED;

	if (!player)
	{
		if (pev_valid(ent))
		{
			static className[32];
			pev(ent, pev_classname, className, charsmax(className));

			if (equali(className, "trigger_", 8) || equali(className, "func_ladder"))
			{
				if (!g_RenderizableTrigger[ent])
				{
					//server_print("invisibilizing %s %d", className, ent);
					set_es(es, ES_Effects, get_es(es, ES_Effects) | EF_NODRAW);
					return FMRES_IGNORED;
				}
				//else
				//	server_print("rendering %s %d", className, ent);

				// Don’t even dare to think about changing that values for render* variables under any circumstances here, everyone understand me clearly?
				// These are now constants, if you need to change them - sure, you can do it on the client side, BUT IN NO CASE ON THE SERVER OR PLUGIN SIDE!

				//set_es(es, ES_Effects, get_es(es, ES_Effects) & ~EF_NODRAW); // That doesn't work! Use set_pev instead!
				//set_pev(ent, pev_effects, pev(ent, pev_effects) & ~EF_NODRAW); // This is now done during plugin_cfg --> AdjustTriggersVisibility
				set_es(es, ES_RenderAmt, 0); // We will set the renderamt value on the clientside at HUD_AddEntity function
				set_es(es, ES_RenderMode, kRenderTransColor);
				set_es(es, ES_RenderFx, 241); // That's how we gonna detect if entity is trigger, since there is no way to get classname straight on clientside AFAIK

				// Assign a separate color to each trigger class so that we can optionally turn off or change their color on the client (e.g. to not showing the single-player triggers as _transition or _changelevel)
				if (equali(className, "func_ladder"))
					set_es(es, ES_RenderColor, { 102, 178, 255 } ); // Sky
				else if (equali(className, "trigger_autosave"))
					set_es(es, ES_RenderColor, { 128, 128, 128 } ); // Grey
				else if (equali(className, "trigger_cdaudio"))
					set_es(es, ES_RenderColor, { 128, 128, 0 } ); // Olive
				else if (equali(className, "trigger_changelevel"))
					set_es(es, ES_RenderColor, { 79, 255, 10 } ); // Bright green
				else if (equali(className, "trigger_endsection"))
					set_es(es, ES_RenderColor, { 150, 75, 0 } ); // Brown
				else if (equali(className, "trigger_gravity"))
					set_es(es, ES_RenderColor, { 70, 130, 180 } ); // Steel blue
				else if (equali(className, "trigger_hurt"))
					set_es(es, ES_RenderColor, { 255, 0, 0 } ); // Red
				else if (equali(className, "trigger_monsterjump"))
					set_es(es, ES_RenderColor, { 238, 154, 77 } ); // Brown Sand
				else if (equali(className, "trigger_multiple"))
					set_es(es, ES_RenderColor, { 0, 0, 255 } ); // Blue
				else if (equali(className, "trigger_once"))
					set_es(es, ES_RenderColor, { 0, 255, 255 } ); // Cyan
				else if (equali(className, "trigger_push"))
					set_es(es, ES_RenderColor, { 255, 255, 0 } ); // Bright yellow
				else if (equali(className, "trigger_teleport"))
					set_es(es, ES_RenderColor, { 81, 147, 49 } ); // Dull green
				else if (equali(className, "trigger_transition"))
					set_es(es, ES_RenderColor, { 203, 103, 212 } ); // Magenta
				else
					set_es(es, ES_RenderColor, { 255, 255, 255 } ); // White
			}
		}

		if (get_bit(g_bit_waterinvis, host) && g_HideableEntity[ent])
		{
			set_es(es, ES_Effects, get_es(es, ES_Effects) | EF_NODRAW);
		}
		return FMRES_IGNORED;
	}

	//// More code here that i haven't touched... ////
}

Now this looks a bit overcomplicated and it can surely be simplified. And one thing i don't really like is that it now all these entities go through Fw_FmAddToFullPackPost, probably because i set them during plugin_cfg to get drawn. So if they weren't doing this before and now they do, that's more load on the server every tick, which i don't know to what extent it matters.

I tested different combinations of changing the serverside and clientside cvars to see that the rendering stops and continues correctly, and also starting the map with the serverside cvar disabled and then enabling it later, just in case this is a thing that only works if set during the plugin configuration, but all the cases worked fine. I tried to set the pev_effects and ES_Effects effects at different places and moments and this is what i found it works so far, maybe you can find a better solution because mine is a bit confusing :)

@SmileyAG
Copy link
Author

SmileyAG commented Dec 23, 2023

Oh sorry for delay, but I don’t think I’ll be refactoring triggers on the plugin-side in near time, so for now we can get with that solution :)

But as soon as you approve it, don’t forget to leave a related comment here or better in an OpenAG request to give the go-ahead for YaLTeR to the merge, and at the same time I’ll also would make a corresponding request for client-side of BugfixedHL-Rebased

@SmileyAG
Copy link
Author

SmileyAG commented Dec 29, 2023

Sorry for the e7cdc1f commit, this is the context for why this happened:

I decided to double-check the triggers initialization code and found that CONTENTS_LADDER is passed to the pev->skin variable in ladder init code: https://github.com/fireblizzard/agmod/blob/bf06e4ffd31c1427784685118820e15552803bcb/dlls/triggers.cpp#L1759

CONTENTS_LADDER is used in pm_shared.c to determine whether the entity is a ladder or not to run the prediction code for it: https://github.com/fireblizzard/agmod/blob/bf06e4ffd31c1427784685118820e15552803bcb/pm_shared/pm_shared.cpp#L2186

And for some reason I did think that if we set EF_NODRAW for it when kz_show_triggers_disable enabled (because EF_NODRAW is unset by default for ladders in HLSDK code), then in pm_shared it will cause the issues with running or predicting the player’s movement on ladders

But I decided to add a debug message to the client-side PM_Ladder code (inside of the CONTENTS_LADDER check), and joined to my local server from second client, and no it didn’t affect the ladder prediction (the check is still passed), so that's why I had to undo that previous commit

TLDR: Ignore the previous commit and also we waiting for merge changes from naz-show_triggers to unstable and uploading it to other SourceRuns HLKZ servers

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.

2 participants