waifu/kemoverse
waifu/kemoverse
3
2
Fork 3
Code Issues 13 Pull requests 4 Projects 1 Releases Packages Wiki Activity

Gacha_response modification #11

Closed
waifu wants to merge 0 commits from dev into master
pull from: dev
merge into: waifu:master
Conversation 1 Commits - Files changed - +1362 -584
waifu commented 2025-05-18 20:24:39 -07:00
Owner
Copy link

Referencing issue #3

  • Modified the bot_app.py process
    • Notifications don't get sent to the parser if they aren't a mention
    • note variables more descriptive
    • Response gets sent directly from bot_app.py
    • Visibility gets checked here too instead of going inside of the function
    • Sleep time lowered to two so testing is faster
      • There may be a way to mark notifications as read using misskey.py
  • Modified parsing.py
    • Takes the client and notification as Input
    • Outputs a list with command,full_user, arguments and note_obj
  • Changed Gacha_response.py to response.py
    • Takes the list coming from parsin.py
    • Outputs response
Referencing issue #3 * Modified the bot_app.py process * Notifications don't get sent to the parser if they aren't a mention * note variables more descriptive * Response gets sent directly from bot_app.py * Visibility gets checked here too instead of going inside of the function * Sleep time lowered to two so testing is faster * There may be a way to mark notifications as read using misskey.py * Modified parsing.py * Takes the client and notification as Input * Outputs a list with command,full_user, arguments and note_obj * Changed Gacha_response.py to response.py * Takes the list coming from parsin.py * Outputs response
waifu added 2 commits 2025-05-18 20:24:40 -07:00
requested review from nai 2025-05-18 20:24:47 -07:00
waifu added the
Refactoring
 label 2025-05-19 05:07:06 -07:00
nai reviewed 2025-05-20 16:57:32 -07:00
bot/response.py Outdated
@ -0,0 +31,4 @@
'''Given a command with arguments, processes the game state and
returns a response'''
command, full_user, arguments, note_obj = parsed_command
nai commented 2025-05-20 16:57:32 -07:00
Collaborator
Copy link

Nice 🎉

Nice 🎉
nai reviewed 2025-05-20 16:58:26 -07:00
bot/parsing.py Outdated
@ -4,3 +3,3 @@
def parse_notification(notification,client):
'''Oarses any notifications received by the bot and sends any commands to
'''Parses any notifications received by the bot and sends any commands to
nai commented 2025-05-20 16:58:26 -07:00
Collaborator
Copy link

this saves the west

this saves the west
nai requested changes 2025-05-20 17:09:10 -07:00
bot/bot_app.py Outdated
@ -47,1 +48,3 @@
note = notification.get("note", {}).get("text", "")
note = notification.get("note", {})
note_text = note.get("text", "")
note_id = note.get("id")
nai commented 2025-05-20 17:09:06 -07:00
Collaborator
Copy link

I love these little things, thank you

I love these little things, thank you
nai commented 2025-05-20 17:09:43 -07:00
Collaborator
Copy link

Idk why it's in "request changes", all good here

Idk why it's in "request changes", all good here
bot/bot_app.py Outdated
@ -50,0 +53,4 @@
# We want the visibility to be related to the type that was received (so if
# people don't want to dump a bunch of notes on home they don't have to)
visibility = notification["note"]["visibility"]
nai commented 2025-05-20 17:08:13 -07:00
Collaborator
Copy link

I understand the intent, but you have to put an unchecked box in your head, that now you bot_app.py is VERY heavily depending on Misskey notification notations and API. If you still want to aggregate all Misskey specifics to one place, this move makes it a bit harder for the future.

Like in chess, with this opening your parse_notification is cleaner now, but bot_app is ""dirtier"" . So think about next moves

I understand the intent, but you have to put an unchecked box in your head, that now you `bot_app.py` is VERY heavily depending on Misskey notification notations and API. If you still want to aggregate all Misskey specifics to one place, this move makes it a bit harder for the future. Like in chess, with this opening your `parse_notification` is cleaner now, but `bot_app` is ""dirtier"" . So think about next moves
nai commented 2025-05-20 17:09:57 -07:00
Collaborator
Copy link

Idk why it's in "request changes", all good here, it's just an observation

Idk why it's in "request changes", all good here, it's just an observation
bot/bot_app.py Outdated
@ -56,0 +70,4 @@
# 🧠 Send to the parser
parsed_command = parse_notification(notification,client)
# Get the response
response = generate_response(parsed_command)
nai commented 2025-05-20 17:05:05 -07:00
Collaborator
Copy link

You are doing neat tuple unpacking at response.py, but here you don't. Would be cool to unify your approach and do the same with response object here! The intention would be a lot clearer

[text, file_ids] = generate_response(parsed_command)

and then

if empty(file_ids): ...

You are doing neat tuple unpacking at [response.py](https://git.waifuism.life/waifu/kemoverse/pulls/11/files#issuecomment-85), but here you don't. Would be cool to unify your approach and do the same with `response` object here! The intention would be a lot clearer `[text, file_ids] = generate_response(parsed_command)` and then `if empty(file_ids): ...`
waifu commented 2025-05-21 20:18:44 -07:00
Author
Owner
Copy link

I think I can't do [text, file_ids] = generate_response(parsed_command) if file_ids don't exist, it will try to unpack two values anyways and throw an error, would it be better in this case to make sure that the output is always [text,file_ids(possibly None value)]?

I think I can't do [text, file_ids] = generate_response(parsed_command) if file_ids don't exist, it will try to unpack two values anyways and throw an error, would it be better in this case to make sure that the output is always [text,file_ids(possibly None value)]?
waifu added 1 commit 2025-05-21 20:00:11 -07:00
VD15 added 4 commits 2025-05-29 00:01:43 -07:00 
Reviewed-on: #36
VD15 added 1 commit 2025-05-29 04:02:30 -07:00
waifu added 9 commits 2025-06-06 22:12:15 -07:00 
Reviewed-on: #46
Reviewed-by: nai <nai@waifuism.life>
waifu added 2 commits 2025-06-07 13:42:42 -07:00 
Reviewed-on: #48
waifu added 4 commits 2025-06-09 20:35:57 -07:00
waifu added 9 commits 2025-06-12 20:34:10 -07:00 
This reverts commit 59915be661.
Reviewed-on: #49
waifu closed this pull request 2025-06-12 20:36:58 -07:00
waifu commented 2025-06-12 20:37:35 -07:00
Author
Owner
Copy link

Moving to a 1.1 pr to make it more official

Moving to a 1.1 pr to make it more official

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
nai
Labels
Clear labels   
Bug

Something's Not Working Right

  
Feature

New Functionality For The Bot

  
Feedback Wanted

Looking for Feedback

  
Refactoring

Codebase Changes

  
Won't Do

Nah Mate.

No labels
Bug
Feature
Feedback Wanted
Refactoring
Won't Do
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: waifu/kemoverse#11
No description provided.
Delete branch "dev"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?