engineer-man / piston-bot

I Run Code bot on Discord
https://emkc.org/run
MIT License
242 stars 36 forks source link

Support user app installation context #80

Open Masterjoona opened 2 months ago

Masterjoona commented 2 months ago

Hello! I added the ability to use it as an user installable app! closes #79 Some other minor changes such as annotating functions

If you find issues, then let me know of course.

https://github.com/user-attachments/assets/3f43114d-0977-461d-9c85-5fb8642120b5

For this pr to be functional you have to toggle the installation context at https://discord.dev/

And a nit from me, I suggest that there should be a .vscode/settings.json file (for whatever reason its in .gitignore currently?) since having the code formatting settings in the editor would enforce them more.

Thats why you might notice some random changes because I originally used ruff instead of autopep8. I recommend getting a linter/formatter such as ruff, since there is a lot of weird styling (like mix-matching different quotes, some code is formatted/intended differently to other)

brtwrst commented 2 months ago

I do appreciate the work you did here. It's a lot to parse with all the formatting and typehint changes. From what I've seen so far I like the changes and in principle, I am not opposed to the functionalities you have added. Because I have not looked at this codebase in years, it's going probably to be a slow process.

Before I decide to dive into this any more we have to clarify some things with input from @realtux

Modals:

As I have always understood it, this bots primary purpose was always "visual teaching" - meaning it is supposed to be used on a server by someone who wants to show something to someone else. (or similar)

For that it's arguably necessary for everyone to see the input easily.

We looked at modals when they came out but back then seeing the input code was not very visually friendly.

Has this changed? I assume you still need an extra click and if the source code is verbose, it's still hard to parse.

Right click menu activation

When activating something with the right click menu, the input and output might become separated in the chat which would be harder to parse for viewers.

User app

I'm not sure if Piston/The Bot are meant as "personal code runner" for anyone. I am aware that you can currently whisper the bot already but I'm not sure if that is used very much and currently it's just "tolerated".

User apps are kind of meant to be used by a user alone are they not? Also I assume there is a way (Like there is for bots) that other users can easily add the app to their account if they see someone else using it?

Resource usage

I am not certain how this will impact the bot's resource usage and as far as I know, the current resource usage is "too much" already.

Again thank you for your work. Lets figure out how to proceed.

Masterjoona commented 2 months ago

Modals:

For that it's arguably necessary for everyone to see the input easily.

Yes, good point, that's is why I left the original commands as is. I guess we can output the source code with the output?

Right click menu activation

When activating something with the right click menu, the input and output might become separated in the chat which would be harder to parse for viewers.

Ah good catch! I can add a link to the message ot was used on.

User app

User apps are kind of meant to be used by a user alone are they not?

Partly, for example someone might use a wiki command to look up something and want to share it too. But we can add a command option to send as an ephemeral message to user only, if thats something you would like?

Also I assume there is a way (Like there is for bots) that other users can easily add the app to their account if they see someone else using it?

Yes they can click app and click the add app sT2jGa0dx6 G5QPjbYBtl

Resource usage

I am not certain how this will impact the bot's resource usage and as far as I know, the current resource usage is "too much" already.

Sorry this a problem I can't don't have a solution for lol but I feel like it won't add much usage, at least not initially since people won't know of the update.

realtux commented 2 months ago

thinking strictly about user experience, this is probably an upgrade, and it's pretty neat work. however, thinking about the purpose of piston-bot to begin with, as brtwrst pointed out, it's probably a downgrade for having an additional click to see the source code. additionally, it would appear edit would no longer be a thing. you might not be aware but the bot will re-evaluate the code on a message edit. this is a quite useful feature.

Masterjoona commented 2 months ago

Edit will still exist for current commands but not for these, no. I can't think of a good solution for these ones

brtwrst commented 2 months ago

will /del work when using the modals? like when you have a typo in the code and it returns an error. are you (and the channel) stuck with the bot message or can you delete it?

Masterjoona commented 2 months ago

Del doesnt work because i forgot it existed but users can delete the message yes

brtwrst commented 2 months ago

Well I'm not opposed to this. Hopefully shouldn't break any existing functionality. Printing the source code when using modals could be an option, but how many characters can you enter in the modal? If it's a lot then you'd have to send the source code split up in multiple messages, which sucks.

Masterjoona commented 2 months ago

The max (and default) is 4000 but we can set it to whatever we want under that, that is a one way to limit the output length

brtwrst commented 2 months ago

You could just send the source code as a text attachment if it is too long for a message.

brtwrst commented 2 months ago

OK so why does the commit "Support user app installation context" also

The meat of "supporting user app installation content" as i see it is

The rest just makes it hard to parse.

Can you split this big commit and move the other refactoring stuff into it's own commit? I'd also appreciate it if you could take out the typehints. I did find like 2 places where it's almost useful, but I can live without them there and I'd prefer not littering the code with things like ctx: commands.Context and guild: Guild

Also how will /error and /error 0 behave when an error happens in a "User App" instead of a Guild channel or DM channel? Could you simulate that and post some screenshots

Masterjoona commented 2 months ago

I guess I can revert the type stuff I took out the runner to its own file because the run.py file was quite long and i didnt want to the add user commands also there Im not sure what you mean by that error stuff? 😅

Masterjoona commented 2 months ago

Okay I have reverted all formatting changes and all of the types except for the user commands as those require them

brtwrst commented 2 months ago

Ok the only thing remaining is to prevent it from crashing if a file is uploaded / run that has more than ~2000 characters. Either truncate the "This is your input" message or attach the input file in that case.