ONGEKI Rival Functionality #36

2TT · 2023-09-01T22:53:55Z

2TT commented

2023-09-01 22:53:55 +00:00

Changes

Added Frontend for ONGEKI rival management
Added / Modified put_rival and delete_rival
Fixed handle_get_user_rival_api_request and handle_get_user_rival_data_api_request

# Changes * Added Frontend for ONGEKI rival management * Added / Modified `put_rival` and `delete_rival` * Fixed `handle_get_user_rival_api_request` and `handle_get_user_rival_data_api_request`

2TT added 8 commits 2023-09-01 22:53:56 +00:00

2a52e391d6 Ongeki template start

5cccc9224a geki version display

0d7409906a fixed backend ongeki rival functionality

eaf9e0cf09 removed temp file

1f545aed41 removed print statements

147d7adaaf Frontend for adding rivals, versionless backend function to support

16d801aff5 Rival Delete functionality

d5d2803cc0 forgot a print command, see previous commit

2TT added 1 commit 2023-09-02 06:39:44 +00:00

d584b93ca5 Merge branch 'develop' into develop

2TT changed title from ~~develop~~ to ONGEKI Rival Functionality

2023-09-06 06:30:08 +00:00

Dniel97 commented

2023-09-08 15:34:17 +00:00

Thanks for the PR!

I just checked the code and I see some problems with it.

The first issue I noticed is in the file frontend.py L46, if you have an empty rival_list then rival_info is never set and lead to a crash because the variable is unset.

The second question I have is the addition of a new function get_profile_data_ignore_version() frontend.py L38. You added the function but only check for the profile with the version number 7 (bright MEMORY) later, you could either directly use the get_profile(aime_id, version=7) function or even better get the latest profile version or give the choice which profile version should be loaded, bright (6) or bright MEMORY (7). Also the function get_profile_data_ignore_version() just gets a single profile and that one could even be older than version 7.

I hope I explained the issues I have with the submitted code properly and I hope you can fix them so they can be merged :)

Thanks for the PR! I just checked the code and I see some problems with it. The first issue I noticed is in the file [frontend.py L46](https://gitea.tendokyu.moe/Hay1tsme/artemis/src/commit/d584b93ca5d5ed6d8b2d80af9273e753c45c22b0/titles/ongeki/frontend.py#L46), if you have an empty `rival_list` then `rival_info` is never set and lead to a crash because the variable is unset. The second question I have is the addition of a new function `get_profile_data_ignore_version()` [frontend.py L38](https://gitea.tendokyu.moe/Hay1tsme/artemis/src/commit/d584b93ca5d5ed6d8b2d80af9273e753c45c22b0/titles/ongeki/frontend.py#L38). You added the function but only check for the profile with the version number 7 (bright MEMORY) later, you could either directly use the `get_profile(aime_id, version=7)` function or even better get the latest profile version or give the choice which profile version should be loaded, `bright (6)` or `bright MEMORY (7)`. Also the function `get_profile_data_ignore_version()` just gets a single profile and that one could even be older than version 7. I hope I explained the issues I have with the submitted code properly and I hope you can fix them so they can be merged :)

2TT added 1 commit 2023-09-08 21:55:32 +00:00

ed46ea33e3 Added version select in session, removed get_profile_data_ignore_version, renamed scripts file

2TT commented

2023-09-08 21:59:47 +00:00

Added a version select (saved for session)
rival_info is now defined even with an empty rival list
removed get_profile_data_ignore_version()

* Added a version select (saved for session) * `rival_info` is now defined even with an empty rival list * removed `get_profile_data_ignore_version()`

Midorica commented

2023-09-20 14:39:06 +00:00

Hello 2TT,

I might give it a try this weekend just to be sure everything works properly before approving the merge.

At a first glance, everything appears to be good and I also see the small fix you did regarding the typo for one of the request which I totally missed

Thank you

Hello 2TT, I might give it a try this weekend just to be sure everything works properly before approving the merge. At a first glance, everything appears to be good and I also see the small fix you did regarding the typo for one of the request which I totally missed Thank you

Midorica commented

2023-09-23 16:07:14 +00:00

Hello 2TT,

I just found some time to test the code and everything is working

The PR will now be approved

Thank you!

Hello 2TT, I just found some time to test the code and everything is working The PR will now be approved Thank you!

Midorica merged commit 91791813dc into develop

2023-09-23 16:08:16 +00:00

Midorica referenced this issue from a commit

2023-09-23 16:08:18 +00:00

Merge pull request 'ONGEKI Rival Functionality' (#36) from 2TT/artemis:develop into develop