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 Discord bot and initial commands #190

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

Conversation

johnpooch
Copy link
Collaborator

No description provided.

@@ -30,5 +36,21 @@ func main() {
router := mux.NewRouter()
routes.Setup(router)
http.Handle("/", router)

apiImpl := api.CreateApi()
session, err := discordgo.New("Bot " + os.Getenv("DISCORD_BOT_TOKEN"))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@zond I'll need to add a DISCORD_BOT_TOKEN value to GitHub secrets or something. Can you help me with that?

Copy link
Owner

Choose a reason for hiding this comment

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

Currently all the secrets needed are stored in App Engine Datastore, see for example https://github.com/zond/diplicity/blob/master/auth/sendgrid.go#L50 where it fetches a Sendgrid key used to send emails via Sendgrid (which may not be functional anymore, people have reported that email deliveries don't work).

Copy link
Owner

Choose a reason for hiding this comment

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

When running locally, I typically have just uploaded the secret using e.g. https://github.com/zond/diplicity/blob/master/game/handler.go#L664.

type Api struct {
}

func (a *Api) SourceProvinces(userId, channelId string) ([]Province, error) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note, the methods here which get the provinces/order types are just returning static data. Will get correct data in a later PR

ChatLanguageISO639_1: "en",
GameMasterEnabled: false,
RequireGameMasterInvitation: false,
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note, for now all values in the newly created game are fixed. We can gradually add more customizability.

return game.CreateGame(nil, request)
}

func CreateApi() *Api {
Copy link
Owner

Choose a reason for hiding this comment

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

Are you sure you need this? Why not just use &api.Api{} instead of api.CreateApi()?

@zond
Copy link
Owner

zond commented Jun 5, 2024

I have a very strong suspicion that using discordgo won't work well in the long run.

The reason is that if I understand https://github.com/bwmarrin/discordgo/blob/v0.28.1/wsapi.go#L50 correctly it connects to Discord using a WebSocket.

App Engine doesn't play well with WebSockets - it is a fully HTTP request based environment. Any API call to internal App Engine things like datastore, memcache, async tasks etc, need a HTTP request to produce a proper App Engine context (like you artificially do in CreateAuthenticatedRequest). However, doing it artificially is a hack that you'll probably regret at some point.

I understand it works for you in a dev_appserver.py setup, but before you invest too much in discordgo you should probably test it in a production App Engine deployment. I suspect that e.g. datastore operations won't work with the home-made authenticated requests. And even if it works, it would be a bit hacky and might break without warning due to internal changes. Maybe.

What might work, to make sure that every action initiated from the Discord side (messages to the bot causing things to happen) work in the App Engine world (by having an up-to-date HTTP request to create an up-to-date App Engine context), is use https://discord.com/developers/docs/quick-start/getting-started#adding-an-interaction-endpoint-url.

I'm not sure if this changes the available features of Discord apps, I guess that needs some research.

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