Various things #5963
Various things #5963
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to f7861fe in 26.856203 seconds
More details
- Looked at
221lines of code in5files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_tZO7W7L5DFMFtmHG
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| await userStatsBankUpdate(user, 'random_event_completions_bank', new Bank().add(event.id)); | ||
| messages.push(`Did ${event.name} random event and got ${loot}`); | ||
| return { |
There was a problem hiding this comment.
The function triggerRandomEvent should not return an object with itemsToAddWithCL directly. Instead, it should handle the item transactions internally and return nothing, or it should return the items to be added and let the caller handle the transaction. This change ensures better separation of concerns and consistency with other parts of the codebase.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 7a4fc1b in 42.402017 seconds
More details
- Looked at
130lines of code in4files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_83R66692TLsTA978
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -156,16 +156,6 @@ client.on('interactionCreate', async interaction => { | |||
| return; | |||
There was a problem hiding this comment.
Removing the bot's readiness and uptime check before processing interactions can lead to processing interactions when the bot is not fully operational, potentially causing errors or unexpected behavior. Consider restoring this check to ensure stability.
| @@ -187,19 +187,19 @@ const cache = new LRUCache<string, number>({ max: 500 }); | |||
| const doesntGetRandomEvent: activity_type_enum[] = [activity_type_enum.TombsOfAmascut]; | |||
|
|
|||
| export async function triggerRandomEvent(user: MUser, type: activity_type_enum, duration: number, messages: string[]) { | |||
There was a problem hiding this comment.
The change to always return an object from triggerRandomEvent could lead to issues if other parts of the code rely on the return value to determine if an event was triggered. Consider reverting this change or ensuring that all dependent code handles the new return type correctly.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on b1d6f5b in 25.457803 seconds
More details
- Looked at
49lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_zJkdagsossBZd1df
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -156,10 +153,13 @@ client.on('interactionCreate', async interaction => { | |||
| return; | |||
| } | |||
|
|
|||
| if (!client.isReady() || !client.uptime || client.uptime < Time.Second * 10) { | |||
| if ( | |||
There was a problem hiding this comment.
The change to notify users and guilds when they are blacklisted could potentially alert malicious users that they are being actively blocked, which might not be desirable. Consider the implications of this change on the security and effectiveness of the blacklist feature.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on f21dab4 in 26.332001 seconds
More details
- Looked at
51lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_CSyKwLG9Hx2V5aYi
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| import { minionKCommand } from '../../src/mahoji/commands/k'; | ||
| import { createTestUser, mockClient, mockMathRandom } from './util'; | ||
|
|
||
| test('Random Events', async () => { |
There was a problem hiding this comment.
The test case for 'Random Events' should include assertions to verify the actual triggering and effects of the random events, not just the outcomes like kill counts or bank updates.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on b37a096 in 20.824294 seconds
More details
- Looked at
28lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_q9s0NHAPAfmcmNIz
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
tests/integration/preStartup.test.ts
Outdated
| import { preStartup } from '../../src/lib/preStartup'; | ||
| import { mockClient } from './util'; | ||
|
|
||
| test('Random Events', async () => { |
There was a problem hiding this comment.
The test name 'Random Events' does not accurately describe the test's purpose. Consider renaming it to reflect that it tests the preStartup function.
| test('Random Events', async () => { | |
| test('preStartup Initialization', async () => { |
No description provided.