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

Discord webhook #875

Merged
merged 20 commits into from
Jul 25, 2023
Merged

Discord webhook #875

merged 20 commits into from
Jul 25, 2023

Conversation

ewof
Copy link
Contributor

@ewof ewof commented Jul 20, 2023

Description:

Users can now have siege war notifications in their discords
image

it uses the localization so works on all supported languages


New Nodes/Commands/ConfigOptions:

DISCORD_WEBHOOK (comment)
DISCORD_WEBHOOK_ENABLED (boolean)
DISCORD_WEBHOOK_URL (string)
DISCORD_WEBHOOK_NOTICIATION_SESSION_START (boolean)
DISCORD_WEBHOOK_NOTIFICATION_SESSION_END (boolean)
DISCORD_WEBHOOK_NOTIFICATION_SIEGECAMP_START (boolean)
DISCORD_WEBHOOK_NOTIFICATION_SIEGE_START (boolean)
DISCORD_WEBHOOK_NOTIFICATION_SIEGE_END (boolean)
DISCORD_WEBHOOK_NOTIFICATION_SIEGE_REMOVE (boolean)


Relevant Issue ticket:


  • I have tested this pull request for defects on a server.

By making this pull request, I represent that I have the right to waive copyright and related rights to my contribution, and agree that all copyright and related rights in my contributions are waived, and I acknowledge that the TownyAdvanced organization has the copyright to use and modify my contribution under the SiegeWar License for perpetuity.

Copy link
Member

@LlmDl LlmDl left a comment

Choose a reason for hiding this comment

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

Took a first pass on this.

Might be worth considering using the lib that EventWar uses, there's a lots of stuff in the DiscordWebhook class that doesn't get used by SW at all.

@Goosius1 Goosius1 added the enhancement New feature or request label Jul 21, 2023
@Goosius1 Goosius1 added this to the 2.8.0 milestone Jul 21, 2023
@ewof ewof requested a review from LlmDl July 21, 2023 14:14
@ewof
Copy link
Contributor Author

ewof commented Jul 21, 2023

also from what i was told in the discord this is the same webhook class that eventwar uses unless you've changed it since getting the class from peti way back when

ewof added 5 commits July 21, 2023 16:08
…. make the config notification toggles work. add siege remove notification
… ending it, add defending town name to remove notification, fix siege start notification check
@ewof
Copy link
Contributor Author

ewof commented Jul 24, 2023

all comments addressed, ready for re-review

@Goosius1
Copy link
Collaborator

Hi @LlmDl I was talking to Ewof earlier, and he has addressed all the comments he is aware of. Pinging here to remind ourselves where we are at with this PR before we forget.

@ewof ewof requested a review from LlmDl July 24, 2023 17:11
@ewof
Copy link
Contributor Author

ewof commented Jul 24, 2023

i pushed another commit called reviews idk y it doesn't show here on the pr but it's there

@LlmDl
Copy link
Member

LlmDl commented Jul 24, 2023

I dont see the commit here.

@ewof
Copy link
Contributor Author

ewof commented Jul 24, 2023

ewof@de19bb0

ewof added 2 commits July 24, 2023 16:24
(trying to make the other commit to appear idk y it's not)
@ewof ewof requested a review from LlmDl July 24, 2023 21:16
Copy link
Member

@LlmDl LlmDl left a comment

Choose a reason for hiding this comment

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

It took a little bit but I am happy with this.

@LlmDl
Copy link
Member

LlmDl commented Jul 24, 2023

I will leave this to @Goosius1 to merge at his leisure.

@Goosius1
Copy link
Collaborator

Great thanks Llama!. I'll now do a secondary review.

Copy link
Collaborator

@Goosius1 Goosius1 left a comment

Choose a reason for hiding this comment

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

Review complete. 1 comment.

@ewof ewof requested a review from Goosius1 July 25, 2023 14:49
Copy link
Collaborator

@Goosius1 Goosius1 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Well done!!!!

@Goosius1 Goosius1 merged commit b41b93f into TownyAdvanced:master Jul 25, 2023
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.

None yet

3 participants