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 Discord Logs #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added Discord Logs #2

wants to merge 1 commit into from

Conversation

ImClin
Copy link

@ImClin ImClin commented Nov 10, 2023

Added discord logs using embed message using steamID, steamname, and points.
image

@@ -1,7 +1,7 @@
ConfigShared = {}
ConfigShared.devmod = false -- Keep the UI on by default, usefull when tweaking UI
ConfigShared.UseDefaultUI = true -- Set this to false if you want to use your own UI

ConfigShared.DiscordLog = 'WEBHOOK HERE'
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add an toggle for this, as not everyone would like this feature

@@ -0,0 +1,20 @@
-- server.lua
RegisterServerEvent('drift:logToDiscord')
AddEventHandler('drift:logToDiscord', function(points)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a nit pick, but I prefer RegisterNetEvent, and also fusion the Register and the Handler

RegisterNetEvent('drift:logToDiscord', function(points) ....

@@ -36,6 +36,9 @@ function Modules.DriftCounter.GetCurrentAngle()
end
end

function Modules.DriftCounter.SendDriftDataToServer(points)
Copy link
Contributor

Choose a reason for hiding this comment

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

Using a config option, check if logs are enabled or not before sending the event

@@ -0,0 +1,20 @@
-- server.lua
RegisterServerEvent('drift:logToDiscord')
AddEventHandler('drift:logToDiscord', function(points)
Copy link
Contributor

Choose a reason for hiding this comment

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

Also not a fan of the points arg not being processed. That mean a cheater could abuse the event, send whatever text he want and spam the discord with it. Maybe filter the arg to only allow numbers, that would at least prevent cheaters from sending whatever they want

Copy link
Contributor

@Rubylium Rubylium left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, added few comments before accepting the PR.
Thanks for the feature

@ImClin
Copy link
Author

ImClin commented Mar 25, 2024

Will do once i have time :)

["title"] = "**Drift Points Logged**",
["description"] = "Player: " .. playerName .. "\nSteam License: " .. playerIdentifier .. "\nPoints: " .. points,
["footer"] = {
["text"] = "Drift Logging System By Clin",
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand you want to leave your mark, but ... eh, at least give the option for the user to change this in the config

AddEventHandler('drift:logToDiscord', function(points)
local discordWebhook = ConfigShared.DiscordLog -- Replace with your webhook URL
local playerName = GetPlayerName(source)
local playerIdentifier = GetPlayerIdentifier(source, 0) -- 0 for license identifier
Copy link
Contributor

Choose a reason for hiding this comment

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

Be carefull with this as depending if the user has steam open or not, the Identifier could change, giving either the steamID, or License

if ConfigShared.UseDefaultUI then
Modules.DriftCounter.FadeOutHud()
end
TriggerEvent(ConfigShared.DriftFinishedEvent, Modules.DriftCounter.CurrentPoints)
Modules.DriftCounter.SendDriftDataToServer(Modules.DriftCounter.CurrentPoints) -- Add this line here
Copy link
Contributor

Choose a reason for hiding this comment

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

"Add this line here" ?

Copy link
Contributor

@Rubylium Rubylium left a comment

Choose a reason for hiding this comment

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

New change I noticed

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