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 cookie methodmap (clientprefs) #1012

Merged
merged 13 commits into from May 31, 2019

Conversation

@JoinedSenses
Copy link
Contributor

JoinedSenses commented May 12, 2019

This PR adds a methodmap for clientprefs cookies without breaking changes, considering that the cookie Handle is not always the first param for clientprefs functions.

@JoinedSenses

This comment has been minimized.

Copy link
Contributor Author

JoinedSenses commented May 13, 2019

If there is a better solution for handling the methods where the Handle is not the first param, please let me know.

@psychonic

This comment has been minimized.

Copy link
Member

psychonic commented May 14, 2019

If there is a better solution for handling the methods where the Handle is not the first param, please let me know.

I think for other similar cases, we did the same shim but on the C++ side to give us more flexibility for future.

@JoinedSenses

This comment has been minimized.

Copy link
Contributor Author

JoinedSenses commented May 14, 2019

Happen to have any examples? I spent an hour or two digging for similar cases, but couldnt find anything, although good chance I overlooked it.

By doing it on C++ side, how would it be handled? Creating a new native callback specifically for the methodmap?

@psychonic

This comment has been minimized.

Copy link
Member

psychonic commented May 14, 2019

Yeah, you'd create a native that just either calls the other native (similar to what you did), or one that calls the same internal function that the other native wraps (with the difference in parameters).

f020b56 is an example

@JoinedSenses

This comment has been minimized.

Copy link
Contributor Author

JoinedSenses commented May 29, 2019

Merge conflict existing due to moving typedef to top of inc, allowing it to be defined for the methodmap.

JoinedSenses added 12 commits May 12, 2019
This PR adds a methodmap for clientprefs cookies without breaking changes, considering that the cookie Handle is not always the first param for clientprefs functions.
I implemented the switching to the c++ side of things and made the methodmap functions natives.
@Headline Headline force-pushed the JoinedSenses:patch-clientprefs branch from 62e77ef to 5fc1b39 May 30, 2019
Copy link
Member

Headline left a comment

Just one little comment, I'll be happy to merge after that change

Nice work! 🎉

static cell_t Cookie_Set(IPluginContext *pContext, const cell_t *params)
{
// This version takes (handle, client, value). The original is (client, handle, value).
cell_t new_params[4] = {

This comment has been minimized.

Copy link
@Headline

Headline May 30, 2019

Member

can you mark this const?

this along with the others

This comment has been minimized.

Copy link
@JoinedSenses

JoinedSenses May 30, 2019

Author Contributor

Should this then be made const as well, the commit change I used for reference? https://github.com/alliedmodders/sourcemod/blob/master/core/smn_keyvalues.cpp#L1107

This comment has been minimized.

Copy link
@Headline

Headline May 30, 2019

Member

Yes please!

This comment has been minimized.

Copy link
@JoinedSenses

JoinedSenses May 30, 2019

Author Contributor

Addressed 60e2a30

@Headline Headline removed the WIP label May 30, 2019
@JoinedSenses

This comment has been minimized.

Copy link
Contributor Author

JoinedSenses commented May 31, 2019

Tested with:

#include <sourcemod>
#include <clientprefs>

Cookie g_hTestCookie;

public void OnPluginStart() {
	RegAdminCmd("sm_testcookie", cmdTestCookie, ADMFLAG_ROOT);

	g_hTestCookie = new Cookie("TestCookie", "", CookieAccess_Private);
}

public Action cmdTestCookie(int client, int args) {
	if (!client) {
		return Plugin_Handled;
	}

	Cookie testcookie = Cookie.Find("TestCookie");
	PrintToChat(client, "%sFound", testcookie == null ? "Not " : "");
	delete testcookie;

	g_hTestCookie.Set(client, "TestValue");
	CreateTimer(5.0, timerCookie, GetClientUserId(client));
	return Plugin_Handled;
}

Action timerCookie(Handle timer, int userid) {
	int client = GetClientOfUserId(userid);
	if (!client) {
		return Plugin_Continue;
	}

	char buffer[10];
	g_hTestCookie.Get(client, buffer, sizeof(buffer));

	PrintToChat(client, "Result: %s", buffer);
	PrintToChat(client, "Time: %i", g_hTestCookie.GetClientTime(client));
	PrintToChat(client, "Level: %i", g_hTestCookie.AccessLevel);
	return Plugin_Continue;
}

Results:

Found
Result: TestValue
Time: 1559265624
Level: 2
Copy link
Member

Headline left a comment

Thanks!

@Headline Headline merged commit 352f078 into alliedmodders:master May 31, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@JoinedSenses JoinedSenses deleted the JoinedSenses:patch-clientprefs branch May 31, 2019
Headline added a commit that referenced this pull request May 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.