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

Clientprefs plugin #1828

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

Conversation

Sikarii
Copy link
Contributor

@Sikarii Sikarii commented Aug 25, 2022

This is a work in progress

Converts the existing Clientprefs extension completely to a SourceMod plugin.
Looking to get feedback on its current state and additionally if you have any ideas on the considerations below.

Considerations

Tabs versus spaces

  • What is actually used for the SM codebase, I've seen a good mix of them.

Unified auth ids

  • How do you achieve this in plugins, I don't see an API for it, like in extensions.

Migration strategy

Implemented APIs

Natives

  • RegClientCookie (and Cookie constructor)
  • FindClientCookie (and Cookie.Find)
  • SetClientCookie (and Cookie.Set)
  • GetClientCookie (and Cookie.Get)
  • SetAuthIdCookie (and Cookie.SetByAuthId)
  • AreClientCookiesCached
  • GetCookieIterator
  • ReadCookieIterator
  • GetCookieAccess (and Cookie.AccessLevel getter)
  • GetClientCookieTime (and Cookie.GetClientTime)
  • SetCookiePrefabMenu (and Cookie.SetPrefabMenu)
  • SetCookieMenuItem
  • ShowCookieMenu

Forwards

  • OnClientCookiesCached

Commands

  • sm_cookies
  • sm_settings

@JoinedSenses
Copy link
Contributor

JoinedSenses commented Aug 25, 2022

Tabs versus spaces
What is actually used for the SM codebase, I've seen a good mix of them.

Tabs for left indent, spaces for everything else. Most everything on sp side already does this, and as you can see in your changes, everything had tabs but has been changed to spaces.

@nosoop
Copy link
Contributor

nosoop commented Aug 25, 2022

Tabs versus spaces

For existing files I'd keep things as-is to keep the commit diff tidy.

Migration strategy

In my opinion, it's critical that existing compiled plugins continue to work, whether that means preserving the extension as a shim or keeping the extension as-is and making clientprefs-sp completely separate.

@KyleSanderson
Copy link
Member

In my opinion, it's critical that existing compiled plugins continue to work, whether that means preserving the extension as a shim or keeping the extension as-is and making clientprefs-sp completely separate.

Agreed. I apologize this hasn't received any feedback publicly - this is a great addition and very much so welcomed. I believe when I was speaking with someone else about this the best path forward is the shim / migration strategy, where if the extension is requested we request the plugin instead. The very light (not even) debate is we do this in the ext, or core.

I know this is marked as a draft, and there's one single item remaining on your checklist. What's the latest on it? is there anything I/we can do to help make this a reality?

@Sikarii
Copy link
Contributor Author

Sikarii commented Oct 7, 2022

In my opinion, it's critical that existing compiled plugins continue to work, whether that means preserving the extension as a shim or keeping the extension as-is and making clientprefs-sp completely separate.

Agreed. I apologize this hasn't received any feedback publicly - this is a great addition and very much so welcomed. I believe when I was speaking with someone else about this the best path forward is the shim / migration strategy, where if the extension is requested we request the plugin instead. The very light (not even) debate is we do this in the ext, or core.

I know this is marked as a draft, and there's one single item remaining on your checklist. What's the latest on it? is there anything I/we can do to help make this a reality?

Hey, thanks for replying, I haven't really worked on this for some time now as I've picked up other things.
I did remember SetCookieMenuItem needing some refactoring in the code I've done for SetCookiePrefabMenu.

That being said, I hope to pick this up again soon, I'm not sure what you could do to make this a reality unfortunately.
If this gets out of hand, I would be comfortable giving write access to whoever to my fork, if it gets there, that is.

@sapphonie
Copy link
Contributor

What's blocking this?

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.

5 participants