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

Add new account events: onAccountCreate/onAccountRemove & new function getAccountType #3019

Merged
merged 16 commits into from
May 25, 2024

Conversation

TracerDS
Copy link
Contributor

@TracerDS TracerDS commented May 20, 2023

New events:

  • onAccountCreate (takes 1 parameter: account)
  • onAccountRemove (takes 1 parameter: account)

New function:

  • getAccountType

New events:
- `onAccountCreate`
- `onAccountRemove`
@TracerDS TracerDS marked this pull request as ready for review May 20, 2023 20:25
@lopezloo lopezloo added the enhancement New feature or request label May 21, 2023
@CrosRoad95
Copy link
Contributor

looks okey for me

@patrikjuvonen patrikjuvonen changed the title Created new account events: onAccountCreate/onAccountRemove Add new account events: onAccountCreate/onAccountRemove May 21, 2023
@TracerDS TracerDS closed this Jul 4, 2023
@TracerDS TracerDS deleted the 200523_NewAccountEvents branch July 4, 2023 19:44
@TracerDS TracerDS restored the 200523_NewAccountEvents branch July 5, 2023 07:32
@TracerDS TracerDS reopened this Jul 5, 2023
@botder
Copy link
Member

botder commented Jul 24, 2023

What kind of use case do you have for these two events? Also, assuming you've tested these events, can you even process the events for "console accounts" and does it even make sense to trigger that event for guest accounts?

@TracerDS
Copy link
Contributor Author

TracerDS commented Jul 25, 2023

What kind of use case do you have for these two events?

Well, we have some events for creation of elements (onClientProjectileCreation, onClientBrowserCreated), why not have one for creation of accounts?

can you even process the events for "console accounts"

Yes, sir.

For guest accounts, its a debatable thing in my opinion. It may or may not be necessary to fire an event when someone joins but its better to have more than less. or maybe other event (onGuestAccountCreated) might be fired specifically for guest accounts?

Probably not useful but its better to have that than to check if it belongs to console or to a guest
Server/mods/deathmatch/logic/CAccountManager.cpp Outdated Show resolved Hide resolved
Server/mods/deathmatch/logic/CAccountManager.cpp Outdated Show resolved Hide resolved
Server/mods/deathmatch/logic/CAccountManager.cpp Outdated Show resolved Hide resolved
Server/mods/deathmatch/logic/CAccountManager.cpp Outdated Show resolved Hide resolved
Server/mods/deathmatch/logic/CAccountManager.cpp Outdated Show resolved Hide resolved
Server/mods/deathmatch/logic/CAccountManager.cpp Outdated Show resolved Hide resolved
Added const qualifiers, reformatted everything in general
@tederis
Copy link
Member

tederis commented Jul 25, 2023

As you decided to refactor the existing code I would suggest to take a look at CRegistryResultCell. It stores nVal, fVal and pVal all the time but only one can be in use at one time. So its better to use union to save memory and promote true sharing while looping.

@botder
Copy link
Member

botder commented Jul 25, 2023

For guest accounts, its a debatable thing in my opinion. It may or may not be necessary to fire an event when someone joins but its better to have more than less. or maybe other event (onGuestAccountCreated) might be fired specifically for guest accounts?

I can understand having the even trigger for player accounts, but I don't see any value for console or guest accounts - that's just noise.

Copy link

@TMachadoDev TMachadoDev left a comment

Choose a reason for hiding this comment

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

I think its all good, in fact perfect.

@Dutchman101
Copy link
Member

Dutchman101 commented May 25, 2024

This has had enough feedback and eyes, thanks!

@TracerDS please add the 2 events and function to the wiki

@Dutchman101 Dutchman101 merged commit 545f54b into multitheftauto:master May 25, 2024
6 checks passed
@Dutchman101 Dutchman101 changed the title Add new account events: onAccountCreate/onAccountRemove Add new account events: onAccountCreate/onAccountRemove & new function getAccountType May 25, 2024
@TracerDS TracerDS deleted the 200523_NewAccountEvents branch May 26, 2024 09:24
MegadreamsBE pushed a commit to MegadreamsBE/mtasa-blue that referenced this pull request Jun 6, 2024
…to#3019)

New events: onAccountCreate, onAccountRemove
New function: getAccountType

* Refactored CAccountManager & CAccount a little
* Refactored internal SQL logic
@TracerDS TracerDS mentioned this pull request Jun 9, 2024
TheNormalnij pushed a commit that referenced this pull request Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants