feature: landmark for trigger_teleport

[khanghugo]

Something from CSS. Something like this s1lentq/ReGameDLL_CS#950

I have to hijack the "message" field because trigger_teleportation does not have its own key value function.

I don't really hope this thing is merged but I want it stay around in case anyone wanting to test landmark teleportation in GoldSrc. It is a neat feature, very simple and effective.

Here is the map to test, along with its source
arte_confused.zip

[khanghugo]

The only reason why I say that "I don't really hope this thing is merged" because the implementation is very constrained. I cannot really think of a way to make maps taking advantage of this to just work. That means mappers must be aware of BXT to add value into "message" field or somebody else has to edit the map with bspguy or something. Also simply hijacking the "message" field is not very nice (attitude).

[SmileyAG]

That means mappers must be aware of BXT to add value into "message" field or somebody else has to edit the map with bspguy or something.

I suppose DispatchKeyValue function can be hooked from exported table (funcs.pfnKeyValue) for hijacking keyvalues:

BunnymodXT/HLSDK/engine/eiface.h

Line 424 in a0cc469

void (*pfnKeyValue) ( edict_t *pentKeyvalue, KeyValueData *pkvd );

Then perform all needed conditions, if result is positive then received edict and its keyvalue we can store all in our custom container at the same index (e.g. std::vector with emplace_back)

BunnymodXT/HLSDK/engine/eiface.h

Lines 297 to 304 in a0cc469

	// Passed to pfnKeyValue
	typedef struct KeyValueData_s
	{
	char *szClassName; // in: entity classname
	char *szKeyName; // in: name of key
	char *szValue; // in: value of key
	long fHandled; // out: DLL sets to true if key-value pair was understood
	} KeyValueData;

In our function we can iterate over that container and check whether the szKeyName matches with what you expect and whether the edict from the same index in the container matches the one edict you are checking for
If all matches, then extract data from szValue and apply it in your code

In the end, clean the container every time the server is shutdown (ServerDeactivate function should be ideal for this task, it is also exported by using funcs.pfnServerDeactivate):

BunnymodXT/HLSDK/engine/eiface.h

Lines 446 to 448 in a0cc469

	#ifndef HLSDK10_BUILD
	void (*pfnServerDeactivate) ( void );
	#endif

[khanghugo]

Thanks for the suggestion, I think this should be better.

[khanghugo]

Apparently this does not work on Windows atm. Not sure why.

[SmileyAG]

const Vector VEC_HULL_MIN(-16, -16, -36);
const Vector VEC_DUCK_HULL_MIN(-16, -16, -18);
auto hull_offset = is_duck ? VEC_DUCK_HULL_MIN : VEC_HULL_MIN;

pev->origin[2] += hull_offset[2];

https://github.com/ValveSoftware/halflife/blob/c7240b965743a53a29491dd49320c88eecf6257b/dlls/util.h#L414-L424

[khanghugo]

So many repetition of this HULL values, maybe future refactor?

[SmileyAG]

So many repetition of this HULL values, maybe future refactor?

Surely.

I know that now in the code these HULL values are declared several times in scope of different functions, but with a future refactor all this will be declared once in some header.

So let it be like this for now.

[SmileyAG]

void TriggerTpKeepsMomentumRestore(entvars_t *pev, Vector prev_vel, Vector prev_view, Vector prev_angles, Vector prev_basevelocity)

[khanghugo]

This is rebase of another other commit, maybe ill change it there instead

[SmileyAG]

ServerDLL::GetInstance().pEngfuncs->pfnAngleVectors(vecAngles, vecForward, nullptr, nullptr);

[khanghugo]

Rebase of another commit

[SmileyAG]

Apparently this does not work on Windows atm. Not sure why.

The reason why it does not work on Windows is because the hook was not declared in the MemUtils::Intercept and MemUtils::RemoveInterception blocks

BunnymodXT/BunnymodXT/modules/ServerDLL.cpp

Line 128 in 76aefb2

MemUtils::Intercept(moduleName,

BunnymodXT/BunnymodXT/modules/ServerDLL.cpp

Line 183 in 76aefb2

MemUtils::RemoveInterception(m_Name,

[SmileyAG]

Ugh. You know, clearing in SV_SpawnServer is generally not bad, because it is called when a new map starts, changelevels and similar events, and it is called before the engine calls DispatchKeyValue

ServerDeactivate (funcs.pfnServerDeactivate from DLL_FUNCTIONS table) is also called in those events and also before DispatchKeyValue, but the difference is that it also covers other events such as disconnect
At that point clearing in ServerDeactivate looks a bit safer, because for example if you are disconnected and you are in the menu, you know that the data will already be cleared, and in the case of SV_SpawnServer you need load map to clear this old data

So personally, I would advise hooking ServerDeactivate from the server-side (fortunately it doesn’t require patterns) and including a comment at the beginning of its function about the difference in data cleanup compared to SV_SpawnServer

You may also need to hook ServerActivate to track its state, because HLSDK does this to avoid unnecessary calls to ServerDeactivate, take a look at the highlighted lines: https://github.com/ValveSoftware/halflife/blob/c7240b965743a53a29491dd49320c88eecf6257b/dlls/client.cpp#L685-L708
ServerActivate could be hooked from exported server table too, so not need to worries about patterns

[khanghugo]

What do you mean by tracking ServerActivate state? Tracking g_serveractive? So there needs to be either another pattern for that static value or BXT own static value for g_serveractive? Seems like a lot of extra work. I think clearing data on load is fine unless you are adamant about it then I can make some changes for that.

[SmileyAG]

If you read the highlighted lines carefully, you will understand that you don’t need to intercept that g_serveractive variable at all. Let me show you a concept of how this can be done without patterns.

// Global variable
bool g_serveractive = false;

// Find "DLL_FUNCTIONS funcs;" in the server module for the declaration location
ORIG_ServerActivate = funcs.pfnServerActivate;
ORIG_ServerDeactivate = funcs.pfnServerDeactivate;

HOOK_DEF_0(ServerDLL, void, __cdecl, ServerDeactivate)
{
	// https://github.com/ValveSoftware/halflife/blob/c7240b965743a53a29491dd49320c88eecf6257b/dlls/client.cpp#L685-L708
	if (g_serveractive)
	{
		g_serveractive = false;

		// Peform any shutdown operations here...
	}

	ORIG_ServerDeactivate();
}

HOOK_DEF_3(ServerDLL, void, __cdecl, ServerActivate, edict_t*, pEdictList, int, edictCount, int, clientMax)
{
	g_serveractive = true;
	ORIG_ServerActivate(pEdictList, edictCount, clientMax);
}

[SmileyAG]

on load is fine unless you are adamant about it then I can make some changes for that.

Well I'd fine with it in general because I don't think there would a cases when you want to access that data while being not on the server, and even so there seems a validation checks is done, so I'd like to say fine, but ServerDeactivate would be a nicer option in fact, maybe it should be done with future refactoring then

[khanghugo]

I totally understood what you mean. I just wonder if it is worth it. Do you think it is good to pursue this?

[khanghugo]

Github going slow. I agree it should be in the future refactor. This PR is another rebase of another PR so I am not very confident to make some changes here.

[YaLTeR]

That's a lot of string comparisons that happen all the time (?), especially for an optional feature.

[khanghugo]

You made me paranoid so I debug print it. This is only called when a new server is started. Every time a new server is started, a lot of string comparisons like this happen to parse entities in BSP. This one is just one extra entity available in the game.

[YaLTeR]

Otherwise good to go ig

[YaLTeR]

Thanks